surf::Exception does not implement std::error::Error

[dtolnay]

This is an unfriendly choice for errors returned by a library because it makes them not work with ?.

async fn repro() -> Result<(), failure::Error> {
    let _ = surf::get("https://www.rust-lang.org").await?; // doesn't work
    Ok(())
}

async fn repro() -> anyhow::Result<()> {
    let _ = surf::get("https://www.rust-lang.org").await?; // doesn't work
    Ok(())
}

Application-focused error types like failure::Error are built on impl<T: std::error::Error> From<T> which is why it matters that library errors implement std::error::Error.

[yoshuawuyts]

@dtolnay thanks for pointing this out; I agree this is not right, and we should fix this. Thanks for reporting, and sorry this spilled over into your issue tracker!

[matklad]

@yoshuawuyts this might want to be marked with 2.0 milestone.

[richard-uk1]

From anyhow::Error docs

Error works a lot like Box, but with these differences:

• Error requires that the error is Send, Sync, and 'static.
• Error guarantees that a backtrace is available, even if the underlying error type does not provide one.
• Error is represented as a narrow pointer — exactly one word in size instead of two.

• The first one of these is fixed by using Box<dyn std::error::Error + Send + Sync> (I'm informed boxed errors are 'static unless they have a lifetime parameter).
• anyhow::Error creates a backtrace where the error is first converted into an anyhow::Error, I'm not sure that adds much in this use case over not having one.
• anyhow::Error is a thin pointer — so you add one indirection to save one usize width. Again I don't think this matters much in this context: your Result<T> will be sized by T (unless it is less than 2 usizes), and unless you've only got a tiny amount of memory (in which case you can't use alloc, and therefore this crate) the nature of making http requests means that you're not going to create enough to make memory usage an issue.

All in all just use Box<dyn std::error::Error + Send + Sync> which is also 'static. 1 less dependency and does the job of a type-erased error.

I made an RFC for adding the type alias to std.

[dtolnay]

@derekdreery Box<dyn Error + Send + Sync> does not implement std::error::Error so it does not address this issue at all.

[matklad]

I'm informed boxed errors are 'static unless they have a lifetime parameter

The precise rule here is that lifetime elision works for trait object (dyn Trait), and elided lifetime is 'static, unless the trait object is behind an &[mut] 'a, in which case the lifetime is 'a. That is, boxed errors are not special, a DST dyn Error is also equivalent to dyn Error + 'static.

All in all just use Box<dyn std::error::Error + Send + Sync> which is also 'static. 1 less dependency and does the job of a type-erased error.

The general problem here is that any two type-erased errors are not interoperable with each other, be it Box<dyn Error>, anyhow::Error or failure::Error.

In the ideal world, anyhow::Error would contain From<Box<dyn Error>> for anyhow::Error impl, but that is incoherent, because upstream crate (std) could add impl Error for Box<dyn Error>. The irony is that std can't actually add this imp, because that would make the two From impls for Box<dyn Error> overlap.

It looks like, if a library wants to use dynamic errors internally, it has to do some manual wrapping into struct Error { dyn_error: Box<dyn std::error::Error> } at the boundary, or it has to force some specific dynamic error type onto the user.

[richard-uk1]

@dtolnay you're right I'm thinking about it the wrong way round - it's the outer error (anyhow/failure) that could be Box<dyn Error +...>. The solution here is just to implement std::error::Error some way that makes sense for Exception.

[Fishrock123]

I believe this was effectively solved by moving to http_types::Error in c8366fb. cc @yoshuawuyts