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

Have anyhow::Error implement std::error::Error? #25

Closed
swallez opened this issue Oct 19, 2019 · 4 comments · Fixed by #28
Closed

Have anyhow::Error implement std::error::Error? #25

swallez opened this issue Oct 19, 2019 · 4 comments · Fixed by #28

Comments

@swallez
Copy link

swallez commented Oct 19, 2019

anyhow::Error does not implement std::error::Error, which leads to confusion when using anyhow with code that expects a Result<_, std::error::Error>.

I noticed that the same applies to failure::Error in the failure crate.

What is the reason for that? What prevents anyhow::Error from implementing std::error::Error?

... and thanks for all your awesome Rust crates!

@dtolnay
Copy link
Owner

dtolnay commented Oct 19, 2019

That impl would be incoherent with the existing pair of impls impl From<T> for T (libcore) and impl<E> From<E> for anyhow::Error where E: std::error::Error + Send + Sync + 'static (? support).

error[E0119]: conflicting implementations of trait `std::convert::From<anyhow::Error>` for type `anyhow::Error`:
   --> src/error.rs:313:1
    |
313 | / impl<E> From<E> for Error
314 | | where
315 | |     E: std::error::Error + Send + Sync + 'static,
316 | | {
...   |
320 | |     }
321 | | }
    | |_^
    |
    = note: conflicting implementation in crate `core`:
            - impl<T> std::convert::From<T> for T;

Can you give a minimal example of code that would want this impl to exist?

@swallez
Copy link
Author

swallez commented Oct 19, 2019

There's a (short) story behind that. A new rustacean colleague had written something like this, with a function panicking with expect, calling it in main along with other functions returning standard errors:

use anyhow::Context;

fn with_expect() {
    let foo = std::env::var("FOO")
        .expect("Env var FOO must be set to provide the foo");

    let data = std::fs::read(foo) // Result<_, io::Error>
        .expect("Failed to read data");

    println!("{:?}", data);
}

fn do_something() -> std::io::Result<()> {
    Ok(())
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    with_expect();
    do_something()?;
    Ok(())
}

I explained that anyhow::Context would allow to provide the context he added with expect without panicking. So he ended up with this, which doesn't compile because anyhow::Error cannot be converted to a stdlib error.

fn with_anyhow() -> anyhow::Result<()> {
    let foo = std::env::var("FOO")
        .context("Env var FOO must be set to provide the foo")?;

    let data = std::fs::read(foo)
        .context("Failed to read data")?;

    println!("{:?}", data);
    Ok(())
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    with_anyhow()?;
    do_something()?;
    Ok(())
}

The solution of course is to change the return type of main to Result<(), anyhow::Result> (or even anyhow::Result<()> but being a new rustacean he couldn't find why anyhow which is supposed to help in error handling was causing these compilation errors and was confused to no end.

There was an implicit assumption that an anyhow::Error was also a std::error::Error or at least implemented Into<std::error::Error> so that '?' would work with a Box<dyn std::error::Error> return type.

Error handling is still a hard thing for people new to Rust because of lack of standardization around std::error::Error caused by the fact that Result doesn't mandate its error part to implement it. I guess it's to support no_std but newcomers easily assume that Result and std::error::Error go hand in hand and can be confused to no end by that.

@dtolnay
Copy link
Owner

dtolnay commented Oct 19, 2019

Ah that's somewhat different, that looks like a missing From impl not a missing Error impl. I released 1.0.16 with the extra impl to make your second code sample work.

@swallez
Copy link
Author

swallez commented Oct 20, 2019

Awesome! Indeed this was actually more about having aFrom than a missing impl. Thanks a lot!

Repository owner locked and limited conversation to collaborators Jul 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants