Error causes can't be downcast

[brson]

Originally reported here: https://users.rust-lang.org/t/cant-to-introspect-error-cause/6536/2

Because cause doesn't return an Error bound by 'static you can't downcast it.

This seems like a significant oversight. Of course you want to be able to downcast that object.

[brson]

cc @Stebalien

[nikomatsakis]

cc me

[nikomatsakis]

This is definitely a bug in the trait as written. OTOH, it's a hard one to fix. Just adding the 'static bound would probably break tons of code.

I did have one thought -- though it probably won't work -- which is adding Any as a supertrait to Error. Seems worth experimenting with, though I guess there must be some people that implement Error for types that are not 'static.

[nikomatsakis]

Another possibility might be deprecating the existing method and adding a new one, I guess (with a default). But it'll be a drag to deal with, particularly since there is no way to "bridge" from the old implementation.

[aturon]

cc me

[aturon]

I think what happened here is that we had Error: Any initially (which implies 'static), but that was replaced with Reflect. Sadly, it seems there weren't any tests of the downcasting functionality...

It's definitely worth a crater run with adding a 'static bound, to see how widespread the problem is.

[Stebalien]

So, I'm worried about generic errors that return values to the programmer (e.g. PoisonError). Such a change would break std. However, as Reflect is unstable, generic errors defined without the help of unstable features will bound their generic parameter with T: Any, not T: Reflect. This implies T: 'static.

Unfortunately, this logic does not apply to errors that return values of non-generic types that just happen to not have static lifetimes.

[alexcrichton]

Possible routes to solve:

• Replace Reflect with Any
• Add 'static to the trait object returned by Error
• Add cause2 which cause calls by default
• Add Error2, an entirely new trait

We'll be using crater here to figure out what to possibly do here.

[alexcrichton]

I don't think we can replace the Reflect bound with Any, too many regressions: https://gist.github.com/alexcrichton/ed5c565611351adbf2a34687b2972cc7

I also don't think we can add 'static to the trait object returned by Error as it'll break anyone who's previously written fn cause. I think at this point we're moving into the territory of "need a whole new error trait" or "need a whole new method"

[antoyo]

This issue happens even when not using cause.
For instance:

use std::error::Error;
use std::fmt::{self, Display, Formatter};

#[derive(Debug)]
struct MyError { }

impl Error for MyError {
    fn description(&self) -> &str {
        "description"
    }
}

impl Display for MyError {
    fn fmt(&self, formatter: &mut Formatter) -> Result<(), fmt::Error> {
        write!(formatter, "description")
    }
}

fn main() {
    let error: Box<Error> = From::from(Box::new(MyError {}));
    let downcasted = error.downcast_ref::<MyError>();
    println!("{:?}", downcasted); // WRONG: prints None.

    let error: Box<Error> = Box::new(MyError {});
    let downcasted = error.downcast_ref::<MyError>();
    println!("{:?}", downcasted); // OK: prints Some(MyError).
}

[Stebalien]

@antoyo That's because you're double boxing. The correct code is:

use std::error::Error;
use std::fmt::{self, Display, Formatter};

#[derive(Debug)]
struct MyError { }

impl Error for MyError {
    fn description(&self) -> &str {
    "description"
}
}

impl Display for MyError {
    fn fmt(&self, formatter: &mut Formatter) -> Result<(), fmt::Error> {
        write!(formatter, "description")
    }
}

fn main() {
    let error: Box<Error> = From::from(Box::new(MyError {}));
    let downcasted = error.downcast_ref::<Box<MyError>>(); // Downcast to `Box<MyError>`
    println!("{:?}", downcasted); 

    let error: Box<Error> = Box::new(MyError {});
    let downcasted = error.downcast_ref::<MyError>();
    println!("{:?}", downcasted); // OK: prints Some(MyError).
}

However, this is extremely confusing and magical.

[nikomatsakis]

@alexcrichton yes, I was afraid it would come to that. I wonder if we could add a new method and deprecate the old one somehow...or something.

[amluto]

Would it make more sense to fix down casting instead? After reading about the issue, it seems like downcasting really should work for any type unless that type has invariant lifetime parameters. The implementation would need to squash the covariant parameters to their smallest possible lifetimes (i.e. that of the Error itself) and contravariant lifetime to the largest ('static). Is there any reason this couldn't work, short of implementation issues? (One implementation issue: you'd want a marker trait DowncastableFrom<'a> implemented by all types that have their lifetimes set right to be downcast to from an object of lifetime <'a>.)

This wouldn't even need a constraint like Any has. I see no problem with letting literally all types implement a trait that did this -- there would just be a handful of types (those with invariant lifetime parameters) that you couldn't downcast to, and those types would give nice errors because they're not DowncastableFrom.

[sfackler]

I don't think those lifetime adjustments can be expressed in the type system.

[amluto]

Why not? This compiles, and I think that, with appropriate compiler support (TypeId improvements to make it implementable and the appropriate automatically generated implementations) it would work. Is there a reason I'm missing that it wouldn't work?

/// This is automatically implemented for all types T for which downcasting
/// a type-erased reference of lifetime `'source` to `&'source T` is sound.
/// For example, a simple struct with a lifetime parameter `'a` containing
/// references of lifetime `'a` is `DowncastableFrom<'a>`.
///
/// NB: I don't think this is the same thing as `T: 'a`, but
/// I haven't thought about it too hard.
pub trait DowncastableFrom<'source> {}

pub trait ImprovedAny {
    fn downcast<'a, T>(&'a self) -> &'a T where T: DowncastableFrom<'a>;
}

[seanmonstar]

Bumped into this trying to make reqwest's error type a struct instead of an enum, with the idea that people could downcast the cause. The idea seemed like the perfect way to allow backwards compatible changes of additional internal variants, while still allowing users to inspect what happened.

[jluner]

Just hit this trying to bring error-chain into cargo. cargo wants to decide whether to show an error in the chain to the user based on the nature of the error.

[rustonaut]

I'm not sure if my observation is right but isn't it impossible to return a &Error which does not comply with 'static from a error which does comply with 'static?
Mainly if I want return a &Error of a not static type from a member function the only way I can think of is by returning a reference to something contained in self(/the original Error)r but if I do so the self can not comply with 'static therefor &Error returned by a Error+'static can be assumed to be 'static, or?

I might have missed something crucial but assuming I didn't couldn't we add a downcast_cause function to any Error+'static e.g.:

impl Error+'static {
    fn downcast_cause<T: Error+'static>(&self) -> Option<&T> {
        self.cause()
            .map(|err| { mem::transmute::<&Error, &(Error+'static)>(err) })
            .and_then( |err| err.downcast_ref::<T>() )
    }
}

I just had this idea and probably missed some points like that you still have to have something like cause_static/cause2 if you want the cause of a cause, or that you probably need a extension
trait because else thinks might break due to name collision. Anyway it only makes sense if
my hypothesis hold, which I'm not sure it will ;=)

PS:
also downcast_ref_cause and downcast_mut_cause would be needed so maybe just stick with adding a cause_static or similar as it is needed anyway.

[nikomatsakis]

I'm not sure if my observation is right but isn't it impossible to return a &Error which does not comply with 'static from a error which does comply with 'static?

Interesting thought. Something like this might indeed work.

[dtolnay]

Thankfully the failure crate fixes this in its cause method.

extern crate failure;
use failure::Fail;

fn cause_is_io_error(error: &Fail) -> bool {
    match error.cause() {
        Some(cause) => cause.downcast_ref::<std::io::Error>().is_some(),
        None => false,
    }
}

[dtolnay]

I am closing this issue because I don't believe it is actionable as is (beyond sitting on the Rust 2.0 wishlist). Any solution we implement in the Error trait or elsewhere will need to go through the RFC process.

@dathinab or anyone else who would be interested in spearheading the RFC for this -- it would be great to see an RFC with the downcast_cause / cause_static approach fleshed out some more along with a discussion of backward compatibility implications and an explanation of what libraries would need to do to be able to take advantage of the new functionality.