Skip to content
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

chore(auth): return objects during GNAP errors #2779

Merged
merged 3 commits into from
Jun 25, 2024
Merged

Conversation

mkurapov
Copy link
Contributor

@mkurapov mkurapov commented Jun 21, 2024

Changes proposed in this pull request

Updates how errors are displayed when making requests to the AS, to be more in-line with the spec. This improves error handling in general as it allows for parsing error codes, and making use of the description.

Before:
Screenshot 2024-06-21 at 20 01 08

After:
Screenshot 2024-06-21 at 20 00 43

This PR also adds some if-checks to the OP post requests scripts in bruno, in order not to prevent any errors from being hidden, since when post requests scripts would fail to parse response body information, the script error would override the actual response error.

Context

General OP error handling #1905

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@github-actions github-actions bot added type: tests Testing related type: source Changes business logic pkg: auth Changes in the GNAP auth package. labels Jun 21, 2024
Copy link

netlify bot commented Jun 21, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit 4464dd1
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/6675c81e9265c50008119dfc

@@ -57,10 +57,19 @@ export async function gnapServerErrorMiddleware(
'Received error when handling Open Payments GNAP request'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main changes here: don't throw error, but return status & body. Follows error handling koa docs: https://github.com/koajs/koa/blob/master/docs/error-handling.md

Comment on lines +85 to +87
ctx.body = {
error: { description: err.message }
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a GNAP error per se, just a validation error, for which we just add the description

Comment on lines +85 to +87
ctx.body = {
error: { description: err.message }
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a GNAP error per se, just a validation error, for which we just add the description

@mkurapov mkurapov marked this pull request as ready for review June 24, 2024 07:51
Copy link
Collaborator

@koekiebox koekiebox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@njlie njlie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mkurapov mkurapov merged commit 0a6c72a into main Jun 25, 2024
42 checks passed
@mkurapov mkurapov deleted the 1905/mk/gnap-responses branch June 25, 2024 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: auth Changes in the GNAP auth package. type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants