hyper: Error type redesign

I came here via #1652. I think that methods is_* on an Error struct are very unergonomic and a mistake in general.

• Code that needs to handle many scenarios will be a long if/else if chain.
• For contributors, it is very easy to forget to add is_* methods when the internal enum is extended, leading to holes in the API.
• There is no help for an API user to implement error handling, because the is_* methods convey no information about which classes of errors there are (also see previous point). The is_* methods might be implemented for a small subset, they might overlap, there is no way to know! Contrast this with enums where you know everything is mutually exclusive, and you know (at least for the current version in use) that the set is exhaustive.
• It is difficult to estimate what level of granularity a user of the library might need to recover from certain errors. The is_* methods might be too granular or not granular enough for their use cases.

If the number of internal errors is large and there is a concern for frequent breaking changes, I am greatly in favour of a two-stage classification. Use one enum for the general error category, and other enums for more granular, specific errors. At the very least the general category should be made public so an API consumer can match on it.

I’ve recently been having a hard time trying to find a way to get to the actual cause of an error that Hyper gives me. For example, when I receive an error that looks like this:

Error(
    Hyper(
        Error(
            Io,
            Os {
                code: 10054,
                kind: ConnectionReset,
                message: "An existing connection was forcibly closed by the remote host.",
            },
        ),
    ),
    "{url goes here}",
)

I would like to be able to, somehow, extract the io::Error to match against io::ErrorKind::ConnectionReset, since that’s a specific case I’d like to deal with. I don’t think this is possible with the current Error type, or at least, I haven’t been able to figure out how to do it.

I was referring to the possibility of a Clippy lint, or compiler warning in future, which would appear after a minor update in which new variants had been added (see the “Unresolved questions” section).

Unless something’s changed since the RFC, #[non_exhaustive] generates a hard error, not a warning if you try to exhaustively match against of an enum outside of the crate it’s defined in.

Regarding just a naming aspect in this design. Chrono has a similar use of the same names for both a variant and nested sub-type. There at least, it can be challenging to get the use imports right without naming ambiguity. If there are natural names that avoid same-names, that can be friendlier, e.g:

enum Connect {
    OnResolve(Resolve),
    OnHandshake(Handshake),
}

A backtrace is neat (but expensive?). I’m not sure I’d want to expose any enum at all with regards to the Error or ErrorKind, because of the inability to grow it. I’d reconsider when and if Rust ever grows some form of private variants (or however is used to prevent exhaustive matches).

It’s worth thinking about the things some would do with an Error from hyper.

• Just log it. So it needs fmt::Debug and fmt::Display.
• Try to send a proper Response depending on the error that occurred? Like, return 431 when there was a Error::TooManyHeaders, perhaps a 414 if there were Error::TooLarge, and a 400 with other parse errors.
• Maybe check for certain io::Errors, to allow an application to adapt, such as if the error involves too many file descriptors open, or something.

Anything else?

I’m thinking of having methods on the type to allow inspecting the type of error it was, since it’s always possible to add more methods. Like, error.is_uri_too_long() or whatever.