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

Display implementation for Error is unergonomic #27

Open
netvl opened this issue May 5, 2018 · 0 comments
Open

Display implementation for Error is unergonomic #27

netvl opened this issue May 5, 2018 · 0 comments

Comments

@netvl
Copy link

netvl commented May 5, 2018

I found that there is no simple way to get a detailed message of an error without direct matching on Error variants, or without using Debug, because the Display impl for Error ignores the message:

impl fmt::Display for Error {
    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
        write!(fmt, "HTTP/2 Error: {}", self.description())
    }
}

impl StdError for Error {
    fn description(&self) -> &str {
        match *self {
            ...
            Error::Other(_) => "An unknown error",
        }
    }
    ....
}

This interferes with generic error handling using libraries like failure - you cannot easily present a generic error to the user if it contains httpbis::error::Error. For example, suppose I have code like this (using rust-grpc):

fn start_server(config: Config) -> Result<(), failure::Error> {
    let mut server_builder = grpc::ServerBuilder::new_plain();
    server_builder.http.set_addr(&config.address).context("Failed to set address")?;
    ...
}

set_addr returns httpbis::error::Error, and it may fail if e.g. hostname in the address resolves to many addresses. If I use this code and I try to print the error chain when the address is incorrect, I get this:

% ./target/debug/iprlist server
[E] [iprlist] Failed to set address
[E] [iprlist]   HTTP/2 Error: An unknown error

which is completely opaque and user-unfriendly. And there is no simple way to show a more detailed error message, because the httpbis error is wrapped into failure::Context and failure::Error, and it would require downcasting and matching on all httpbis::error::Error enum variants to get a proper error message. Moreover, as far as I can see, this is not limited to the Error::Other variant - variants like Error::IoError are also displayed without any details.

In my opinion, the Display impl should display the underlying error, if there is any, instead of forwarding to description().

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

1 participant