failure icon indicating copy to clipboard operation
failure copied to clipboard

How to impl a trait for failure::Error?

Open flosse opened this issue 7 years ago • 10 comments

I trying to migrate from quick_error to failure with the following code. This was the original error enum:

quick_error!{
    #[derive(Debug)]
    pub enum AppError {
        Business(err: BusinessError){
            from()
            cause(err)
            description(err.description())
        }
        R2d2(err: r2d2::Error){
            from()
        }
        // ...
        Other(err: Box<error::Error + Send + Sync>){
            from()
            description(err.description())
            from(err: io::Error) -> (Box::new(err))
        }
    }
}

that is used within a rocket module:

type Result<T> = result::Result<Json<T>, AppError>;

#[get("/count/tags")]
fn get_count_tags(db: State<DbPool>) -> Result<usize> {
    let tags = usecase::get_tag_ids(&*db.get()?)?;
    Ok(Json(tags.len()))
}

And here I was able to impl a trait for my custom AppError:

impl<'r> Responder<'r> for AppError {
    fn respond_to(self, _: &rocket::Request) -> result::Result<Response<'r>, Status> {
        Err(match self {
            AppError::Business(ref err) => {
              // ...
            }
            _ => Status::InternalServerError,
        })
    }
}

With failure I don't need the wrapping AppError anymore (👍) so now the Result looks like this:

type Result<T> = result::Result<Json<T>, failure::Error>;

And everything is fine but of course I can't implement a trait for the external failure::Error :( How would you solve this? Keep using the wrapper AppError that holds all possible error types? Or use a struct WrappedError(failure::Error) and impl the std::error::Error manually?

flosse avatar Dec 07 '17 11:12 flosse

Yes, if you need to implement a trait for it, you'll need to use a newtype around it. :-\

If you do that, I recommend adding impl<F: failure::Fail> From<F> for AppError. This makes ? work automatically, just like it does for failure::Error.

withoutboats avatar Dec 07 '17 19:12 withoutboats

Hm... it's a bit complicated :( Here is my wrapper and result type:

#[derive(Debug)]
enum AppError<'a>{
    Fail(&'a failure::Error),
    Unknown
}

type Result<'a, T> = result::Result<Json<T>, AppError<'a>>;

with

impl<'a, F: failure::Fail> From<F> for AppError<'a> {
     fn from(f: F) -> AppError<'a> {
        if let Some(err) = f.downcast_ref::<failure::Error>() {
            AppError::Fail(err)
        }  else{
            AppError::Unknown
        }
    }
}

but this leads to

let tags = usecase::get_tag_ids(&*db.get()?)?;
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::error::Error` is not implemented for `failure::Error`

And implementing the std::error::Error trait does of course not work for the external failure::Error :-\

flosse avatar Dec 08 '17 01:12 flosse

The best way to convert your error type to Error is with from(), not downcast_ref. You should do this:

struct AppError(failure::Error);

impl<F: failure::Fail> From<F> for AppError {
    fn from(f: F) -> AppError { AppError(failure::Error::from(f)) }
}

You should also implement the conversion for failure::Error:

impl From<failure::Error> for AppError {
   fn from(e: failure::Error) -> AppError { AppError(e) }
}

Together, these will let you ? anything that implements Fail, anything that implements std::error::Error, as well as failure::Error.

withoutboats avatar Dec 08 '17 01:12 withoutboats

I'm sorry for bothering you:

/ impl<F: failure::Fail> From<F> for AppError {
|     fn from(f: F) -> AppError { AppError(failure::Error::from(f)) }
| }
|_- first implementation here
 
/ impl From<failure::Error> for AppError {
|    fn from(e: failure::Error) -> AppError { AppError(e) }
| }
|_^ conflicting implementation for `infrastructure::web::AppError`

flosse avatar Dec 08 '17 10:12 flosse

Ugh that's very unfortunate! I'm sorry, I thought it would work, but coherence has bitten us pretty badly here.

withoutboats avatar Dec 08 '17 19:12 withoutboats

I've got a better solution!

If you want to abstract over both F: Fail and failure::Error, you should use the bound T: Into<failure::Error>, which will be implemented for all fail types as well as the error type.

In your case:

impl<T: Into<failure::Error>> From<T> for AppError {
    fn from(t: T) -> AppError { AppError(t.into()) }
}

This should work for you!

withoutboats avatar Dec 09 '17 03:12 withoutboats

Ok, your last hint solved most issues but now I still have problems to match my ErrorKind variants :( Out of the box failure seems not to support it but I thought Error and ErrorKind could solve it. Unfortunately I cant setup a working example (see #107).

flosse avatar Dec 10 '17 16:12 flosse

Slightly off topic, but this last hint has let me refactor how my newtype impl's were structured in https://github.com/tismith/exitfailure - I was running into the same coherence issues as @flosse and using Into<failure::Error> worked a treat.

tismith avatar Jun 01 '18 00:06 tismith

Any chance we get failure::Error to implement std::error::Error on its own?

kpcyrd avatar Jun 21 '18 07:06 kpcyrd

After 3 days of trying to coalesce errors between tokio, thrussh, and failure I'm pretty much ready to give up in frustration. It seems like if the story for failure is backwards compatibility then not implementing std::error::Error is a pretty big hole. Thanks.

RX4NT9UP avatar Oct 17 '18 18:10 RX4NT9UP