Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sdk 2246 go retrieve session #282
Sdk 2246 go retrieve session #282
Changes from 11 commits
0a7667c
8825126
82b3fc9
b326008
c30c85b
d0f8123
f0be59e
f6de789
dc8aeb7
3d92450
391b925
49a758c
bdda8e4
d011b02
6f5d220
8b2a670
ade2267
9fe3f6c
8c88429
4a9ff75
ca50060
787a94b
198f704
f86f324
fdce2b6
95eb7c5
3ddcb2c
51f50dc
2591663
565204d
0fa3553
3321f3e
c7d96fc
c4c3290
004d711
edf368c
9d43a02
d99947b
1c74f09
aad8d97
75fe18e
6566ede
fba5a14
0df76b0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 72 in digital_identity_client.go
GitHub Actions / Sonar Scan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the way the errors are returned changed so now they will be:
{ "id": "52f6a70a61191b6ad772e3c2bb875e5a", "status": 401, "error": "INVALID_REQUEST_SIGNATURE", "message": "Invalid request signature" }
so we will have to handle it differently here: https://github.com/getyoti/yoti-go-sdk/blob/master/requests/request.go#L19
as it's not the same yotierror format.
Share V2 links to this https://lampkicking.atlassian.net/wiki/spaces/CON/pages/443252751/Error+Handling+System
So it's worth checking with the other SDK teams to ask for more info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add a success test case for this too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add extensive testing as in client_test.go and also add the test for Retrieving a session
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we need to update this
requests
package to support the error codes connect send back, some of these are really useful and we should differentiate between them and give the RP an exported error var they can switch on and handle.e.g. from looking at this
404
UNKNOWN_SESSION
is invalid session id which would be useful for a RP to know when integrating with sdk, as opposed to theShareSessionHTTPErrorMessages
where404
is mapped toApplication was not found
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mehmet-yoti are you still working on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I am doing some changes on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't see the changes for this from the newest commits @mehmet-yoti - feel free to ping me on slack if you wanna chat about it abit quicker
Check failure on line 61 in digitalidentity/service_test.go
GitHub Actions / Sonar Scan
Check failure on line 72 in digitalidentity/service_test.go
GitHub Actions / Sonar Scan
Check failure on line 83 in digitalidentity/service_test.go
GitHub Actions / Sonar Scan
Check failure on line 114 in digitalidentity/service_test.go
GitHub Actions / Sonar Scan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be a pointer