-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat: add gnap error response schema to spec #519
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: e505fc3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for openpayments-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
6d06c97
to
01d8f9b
Compare
openapi/auth-server.yaml
Outdated
- user_denied | ||
- request_denied | ||
- unknown_interaction | ||
- too_fast |
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 too_fast
shouldn't be indented, because in the generated file "unknown_interaction - too_fast"
should be two separated values.
openapi/auth-server.yaml
Outdated
@@ -532,6 +588,26 @@ components: | |||
- debitAmount | |||
- required: | |||
- receiveAmount | |||
gnap-error: |
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.
what do you think of having separate objects per error, and then using specific ones per-response?
e.g. we wouldn't have too_fast
in a normal grant request, just for grant continuation.
Maybe that is too specific, but curious to hear your opinion
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 considered that, but it felt at times overly granular to have a schema for an error that was only referenced once - too_fast
is a pretty good example of that, as it's only thrown by the grant continuation endpoint.
In hindsight though, I think it would be more useful to use the specific ones per-response. Biggest reason to me being that these routes may throw the same http code for different reasons, in particular 401 could be thrown by the signature middleware or the actual business logic...
@njlie given we verify the Open Payments API responses in the Open Payments client, this actually might be breaking if we are returning incorrect errors in the Rafiki AS, particularly if an integrator is on an old version of Rafiki. Is that correct? |
Changes proposed in this pull request
gnap-error
schema to the spec, and references them in the error responses of the auth server endpoint specs.Context
Closes #518.