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

feat(types): Add gRPC Richer Error Model support #1060

Closed
wants to merge 5 commits into from

Conversation

flemosr
Copy link
Contributor

@flemosr flemosr commented Aug 18, 2022

Motivation

The gRPC richer error model is quite useful to send additional feedback to clients, and is supported by many gRPC libraries in other languages.

Solution

This PR adds richer error model support to tonic-types, as per #1034. Example servers and clients were also added.

@flemosr
Copy link
Contributor Author

flemosr commented Aug 18, 2022

Note: there are links at the bottom of the added README.md that are supposed to point to the crate docs. They would have to be updated to point to the correct released version of tonic-types.

@LucioFranco
Copy link
Member

Thank you for this! This is quite a large PR and will likely take me a while to review. Do you think you could reduce this into multiple PRs that we can merge independently?

@flemosr
Copy link
Contributor Author

flemosr commented Aug 23, 2022

Hi! Happy to contribute. I think the new code can be more or less evenly splitted between the functionallity corresponding to each standard error message type (BadRequest, RetryInfo, etc). I could add only the functionallity corresponding to BadRequest on the first PR, and add support for the other error types in subsequent PR's. Would this approach work better?

@LucioFranco
Copy link
Member

@flemosr yeah that would be great! Thanks

@flemosr
Copy link
Contributor Author

flemosr commented Aug 26, 2022

Hi! The first PR is still quite big, but most of it is the new proto file, docs and tests. The following ones can be smaller

@flemosr flemosr closed this Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants