-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add http routing errors handler #1517
Add http routing errors handler #1517
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
@googlebot I fixed it. |
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.
This looks great! Could I just ask that you add a little blurb to the migration guide (https://github.com/grpc-ecosystem/grpc-gateway/blob/v2/docs/_docs/v2-migration.md) and the error handling documentation (https://github.com/grpc-ecosystem/grpc-gateway/blob/v2/docs/_docs/customizingyourgateway.md#error-handler) to tell users about this new handler? Thanks!
@johanbrandhorst For some reason google bot doesn't believe me I have a CLA, is it requires some time to get synced? |
@googlebot I fixed it. |
The issue with the CLA bot is that your commits are made from a different email (or account) than the account you've used here. You can see it in the commit log. Could you see if you can push some new commits with the correct email? |
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.
Some small wording corrections, thanks for the docs additions!
ea115bc
to
d56d652
Compare
@johanbrandhorst thanks for suggestions! applied |
Co-authored-by: Johan Brandhorst <[email protected]>
3cbe584
to
68d7647
Compare
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Thanks for your contribution! |
This pull request adds routing error handler to address issue #1513
gRPC Gateway v2 had new error handling infrastructure added.
As a side effect of unified error handling all errors now handled as gRPC errors by standardised handler ErrorHandlerFunc.
As it works very well for gRPC errors, for cases when routing errors happen generalised error handler making some assumptions and can change expected http behaviour:
To fix this PR adds additional error handler: routing error handler. Routing error handler is only called for errors related to routing problems:
By default with no routing handler installed current behaviour will be preserved and errors will be converted to gRPC errors with following default error handler call.