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

Create a hyper::error::Code type #2845

Open
seanmonstar opened this issue May 20, 2022 · 6 comments
Open

Create a hyper::error::Code type #2845

seanmonstar opened this issue May 20, 2022 · 6 comments
Labels
A-error Area: error handling B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.

Comments

@seanmonstar
Copy link
Member

seanmonstar commented May 20, 2022

Create a new opaque struct Code in hyper::error, that contains constants of errors that could happen in hyper.

pub struct Code(Code_);

enum Code_ {
    Canceled,
    // etc
}

impl Code {
    pub const CANCELED: Code = Code(Code_::Canceled);
}

Should it be an opaque struct, or a non-exhaustive enum?

Proposal welcome

A motivated individual that would like to be able to inspect errors, and send HTTP 2/3 errors, can take this on and propose how exactly to add this. Note the following for whatever would be accepted:

  • Conservative: we can add things, but we can't take anything away. Err on minimal additions to start.
  • Opaque: the feature shouldn't necessarily mirror exactly what is used internally. We may decide to change how things work internally, and that shouldn't affect the public API.
  • Extensible: as we internally refine how to detect different errors, adding relevant details to the public API should be logical.
  • Purposeful: Keep the purpose in mind: to be able to react to certain kinds of errors. Not all errors are that interesting. There is little value in matching on every single possible error variant. Most useful actions are usually: can I retry this? if not, how do I make it pretty to show to a user?
  • Some background reading: https://seanmonstar.com/blog/pattern-matching-and-backwards-compatibility/
@seanmonstar seanmonstar added C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful. A-error Area: error handling labels May 20, 2022
@seanmonstar seanmonstar added this to the 1.0 milestone May 20, 2022
@dswij
Copy link
Member

dswij commented Jun 29, 2022

Since this is related to hyperium/hyper-util#3, figured I'll take a look at this.

One thing I'm not too sure of is: how exactly does this Code differ from hyper::error::Kind?

@seanmonstar
Copy link
Member Author

It's not required to make progress on hyperium/hyper-util#3, we can figure out the relationship afterwards. But to the question: the major difference is that Code is a public thing. The internal Kind is meant to be a cheaper way to assign specific error information, vs allocating a string that might not ever be printed. I expect Kind to probably hold more specific information than Code might expose.

@GlenDC
Copy link
Contributor

GlenDC commented Aug 15, 2023

I think I do right by responding to this issue instead of making an ew one, but if not sorry for hijacking and I'll make a new one none the less, @seanmonstar .

I think this is the kind of thing I need and wouldn't mind contributing one of these weeks, just would need a bit of guidance.

Currently the error reporting of Hyper is pretty limited as exposed by the current Error struct https://docs.rs/hyper/1.0.0-rc.4/hyper/struct.Error.html (seems unchanged from 0.14).

In general it is fine, but if you want to be able to handle specific kinds of errors you're often now forced to downcast_ref, perhaps even a chain of such checks, which becomes very painful very quickly. E.g. you might be looking for a specific kind of IO Error (which can happen in different ways), or some specific h2 issue (e.g. invalid frame size), etc etc.. Right now checking these kind of things so you can handle failures in a proper manner (so instead of just logging and accept failure, that you can actually correct your mistake on the fly, or retry, or w/e else.

For this I would argue against another opaque type.. I mean the type can be opaque in its implementation (so if you prefer a struct over an Enum, I'm fine with that), but the other approach is fine as well. Most important however is that any kind of error can just be accessed just as easily, while now some seem more exposed then others.

Is this the kind of use case you had in mind for this issue, or am I completely in the wrong issue?

@seanmonstar seanmonstar removed this from the 1.0 milestone Aug 31, 2023
@kszlim
Copy link

kszlim commented Feb 10, 2024

Curious if there has been any progress made on this front? I similarly to #2711 want access to the Kind::Io. It's a little tricky writing opinionated wrappers with hyper/reqwest due to the inability to introspect into the Kind of an error.

@seanmonstar seanmonstar added the B-rfc Blocked: More comments would be useful in determine next steps. label Feb 14, 2024
@seanmonstar
Copy link
Member Author

I've updated the top issue comment, indicating a proposal is welcome and some values to consider.

@acfoltzer
Copy link
Member

@seanmonstar We've had some success with a revised set of "send" errors on the Fastly Compute platform that are roughly based on the proxy-status RFC, though of course extended with cases that cover non-proxy points of view.

The tentative success here has led my colleagues to use a similar structure in the error-code struct of the vendor-agnostic wasi-http spec.

The major gap here is that our work has so far focused on errors that arise during the initial exchange of request/response headers, rather than errors that can arise during subsequent reads of message contents.

I'm interested to take folks's temperature on this kind of approach. I know that basing things on an enum gets into the above-mentioned issues with pattern matching and compatibility, but my feeling is that stability concerns are mitigated by tying things (if unofficially and inexactly) to open standards. If it seems promising, I'll write up a more detailed proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error Area: error handling B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.
Projects
No open projects
Status: Todo
Development

No branches or pull requests

5 participants