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

Use direction in error reporting during decode #374

Merged

Conversation

snawaz
Copy link
Contributor

@snawaz snawaz commented Jun 18, 2020

It is not a bug-fix or a new feature as such. Just being more explicit and loud in reporting error using the communication direction.

Motivation

We encountered this error message today:

(Status { code: Internal, message: "Unexpected compression flag: 123" }

It's not immediately obvious as to whether this error-scenario was encountered while sending the request or while receiving the response. I think adding that information in the message would be highly helpful to figure out the actual source causing the error.

Solution

It's pretty simple. I used self.direction to detect the error-scenario (as in, sending or receiving?) and created an error message accordingly.

Please let me know if this little improvement is acceptable in this form, or I missed out anything important. I'd like to make changes as required.

@snawaz
Copy link
Contributor Author

snawaz commented Jun 18, 2020

CI / cargo-deny check FAILED for no obvious reason. I think re-run should work? I don't see any re-run option though.

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

This is a great idea! Thanks and sorry for the delay!

@LucioFranco LucioFranco merged commit d68dd36 into hyperium:master Jul 6, 2020
@snawaz snawaz deleted the use-direction-in-error-reporting branch July 6, 2020 20:53
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