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

Enable mapping h2 errors to correct Status codes. #394

Closed
LucioFranco opened this issue Jul 10, 2020 · 4 comments
Closed

Enable mapping h2 errors to correct Status codes. #394

LucioFranco opened this issue Jul 10, 2020 · 4 comments
Labels
A-tonic C-enhancement Category: New feature or request E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.
Milestone

Comments

@LucioFranco
Copy link
Member

LucioFranco commented Jul 10, 2020

We currently have this code https://github.com/hyperium/tonic/blob/master/tonic/src/status.rs#L344 which maps h2::Error into proper Code's. We should test and ensure that if the user enables the transport feature we can downcast the errors into h2 variants and mapping them to correct status codes.

@LucioFranco LucioFranco added C-enhancement Category: New feature or request A-tonic labels Jul 10, 2020
@LucioFranco LucioFranco added E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. labels Jul 27, 2020
@LucioFranco LucioFranco added this to the 0.4 milestone Nov 27, 2020
@SkamDart
Copy link
Contributor

Happy to take this over but after reading through the code, it looks as if maybe this has already been done with this feature and test?

Unless I'm misunderstanding this and you were looking to feature gate something like, pub enum Http2Error based on this spec under transport?

@LucioFranco
Copy link
Member Author

Last I remembered I don't think its hooked up would be good to test. We don't need to add a http2error we just need to ensure when we get an h2 error its translated into the correct status code.

@davidpdrsn
Copy link
Member

Is this related to #612?

@davidpdrsn
Copy link
Member

It isn't. But it was fixed in #509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tonic C-enhancement Category: New feature or request E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

No branches or pull requests

3 participants