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(error): preserve InvalidUri details #3044

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Nov 4, 2022

This branch changes the representation of the Parse::Uri error kind to preserve information provided by the http crate's uri::InvalidUri and uri::InvalidUriParts errors. A new InvalidUri enum is added that holds one of those two error types, or a custom message (since hyper currently returns Parse::Uri errors that didn't come from an inner http error in some cases). The new enum has a custom Display and Debug implementation to reduce repetition of the string "invalid URI" in its formatted output.

This is not stored as the error's cause currently in order to avoid exposing the http crate's error types in the public API. However, when http 1.0 is released, we can simplify this code significantly by storing the error as a cause and exposing it in the source chain.

Closes #3043

This branch changes the representation of the `Parse::Uri` error kind to
preserve information provided by the `http` crate's `uri::InvalidUri`
and `uri::InvalidUriParts` errors. A new `InvalidUri` enum is added that
holds one of those two error types, or a custom message (since `hyper`
currently returns `Parse::Uri` errors that didn't come from an inner
`http` error in some cases). The new enum has a custom `Display` and
`Debug` implementation to reduce repetition of the string "invalid URI"
in its formatted output.

This is _not_ stored as the error's `cause` currently in order to avoid
exposing the `http` crate's error types in the public API. However, when
`http` 1.0 is released, we can simplify this code significantly by
storing the error as a cause and exposing it in the source chain.

Closes #3043
@hawkw hawkw force-pushed the eliza/invalid-uri branch from 08622d5 to c3e4bb3 Compare November 4, 2022 17:58
@seanmonstar
Copy link
Member

This is not stored as the error's cause currently in order to avoid exposing the http crate's error types in the public API.

hyper already exposes http publicly, and http will need to be marked 1.0 before hyper. If it does make things easier, than we can just do that.

@hawkw
Copy link
Contributor Author

hawkw commented Nov 7, 2022

@seanmonstar In that case, it might be nicer to expose http's errors as a source and downcast_ref target!

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.

URI parse errors should preserve information from the HTTP crate
2 participants