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

Expose error kind enum for matching? #188

Closed
choubacha opened this issue Mar 12, 2018 · 9 comments
Closed

Expose error kind enum for matching? #188

choubacha opened this issue Mar 12, 2018 · 9 comments

Comments

@choubacha
Copy link
Contributor

Currently, the inner kind of the error type is unexposed which makes mapping into an SDKs error types.

https://github.com/hyperium/http/blob/master/src/error.rs#L25-L35

This works well for displaying the description but if I want to map the result to a different error to make it consistent I cannot. For example, if I'm using both hyper and http (this may work better in the future) I cannot match a Uri parse error in hyper to that of http with ease and combine them into a single BadUri enum. In fact, it's deeper than that in that the inner types for the Uri errors actually point to another struct and error kind.

I also acknowledge that I'm still new to rust and this means that I might be missing a design pattern that I should use.

@seanmonstar
Copy link
Member

I don't think we would expose the ErrorKind enum publicly, because it prevents adding new variants, or changing them around, and thus makes improving the Error internal design a lot harder.

If you had access to the ErrorKind, what kind of code would you hope to write? Maybe we can find a solution.

@choubacha
Copy link
Contributor Author

Hmm. I think that since individual functions appear to each implement their own error kinds (and those structs are public) I can properly map them. However, I'm currently implementing both a From the InvalidUri and the Error types:

https://github.com/kbacha/stellar-sdk/pull/39/files#diff-045ef8f78ba0431706a905bb35c78758R56

This enables me to map specifically for invalid errors but also catch-all when a builder replies with the generic error. Perhaps it works out the box anyhow?

@seanmonstar
Copy link
Member

You could change from using the builders to doing the conversions yourself, thus knowing exactly what operation the error came from.

@choubacha
Copy link
Contributor Author

Ah, that could work. And that's what I'm doing with the URI in this case.

@carllerche
Copy link
Collaborator

Is this issue resolved?

@ohbadiah
Copy link

I find myself with the same question @kbacha had, but I don't understand the solution @seanmonstar offered. I too am new to Rust, which may be the issue.

I'm trying to upgrade a library from hyper 0.10 to hyper 0.12. The library has a domain-specific Error enum, some variants of which correspond to variants of ErrorKind. I'd prefer to map the ErrorKind variants to my existing variants, rather than creating a catch-all for all instances of http::Error.

What's the best way to accomplish that?

@C4K3
Copy link

C4K3 commented Feb 25, 2021

Now that we have non_exhaustive (added in stable 1.40.0) this is possibly something to reconsider. Does this crate specify a minimum supported version less than 1.40?

@tgross35
Copy link

tgross35 commented Feb 5, 2024

MSRV is currently called out as 1.49, so this should be possible at this point. Could this issue be reopened? edit: #382 is open, that's probably a better place

I think this would just be:

  1. Delete the current Error
  2. Rename ErrorKind to Error
  3. Mark this non_exhaustive

This should be a nonbreaking change IIUC. The debug impl could be adjusted or changed to a derive as well.

@catshow
Copy link

catshow commented Mar 11, 2024

Please re-open this issue, to expose uri:InvalidUri(ErrorKind). We should now be able to use the non exhaustive annotation. It should be non-breaking because the previous API was private.

My use case is: I am trying to port the reqwest proxy implementation for hyper, and switch it to use uri instead of url, and there are tons of test cases that check the kind of error.

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

No branches or pull requests

7 participants