-
Notifications
You must be signed in to change notification settings - Fork 190
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
Improve HTTP header errors #3779
Conversation
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.
strongly recommend adding some tests of invalid headers and ensuring that you get errors that help to trace back to the source.
{ | ||
Err(HttpError::header_was_not_a_string(e)) | ||
Err(HttpError::not_utf8(not_utf8.as_bytes())) |
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.
seems like including the actual header here (including the key!) would be more helpful
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.
We can't always include the key because some header-related methods focus solely on the value. For cases where the key is available, I've added it to the error message.
A new generated diff is ready to view.
A new doc preview is ready to view. |
I still don't fully understand where the error is coming from. I'd like to release what I have because I expect it to help and then I'll work on tracking down the source afterwards. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
This improves error messaging for HTTP header related errors.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.