hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.42k stars 1.59k forks source link

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

Open seanmonstar opened 2 years ago

seanmonstar commented 2 years ago

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:

dswij commented 2 years ago

Since this is related to https://github.com/hyperium/hyper-util/pull/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 commented 2 years ago

It's not required to make progress on https://github.com/hyperium/hyper-util/pull/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 commented 1 year ago

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?

kszlim commented 7 months ago

Curious if there has been any progress made on this front? I similarly to https://github.com/hyperium/hyper/pull/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 commented 7 months ago

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

acfoltzer commented 7 months ago

@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.