Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

How to impl a trait for failure::Error? #104

Open
flosse opened this issue Dec 7, 2017 · 10 comments
Open

How to impl a trait for failure::Error? #104

flosse opened this issue Dec 7, 2017 · 10 comments

Comments

@flosse
Copy link

flosse commented Dec 7, 2017

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?

@withoutboats
Copy link
Contributor

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.

@flosse
Copy link
Author

flosse commented Dec 8, 2017

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 :-\

@withoutboats
Copy link
Contributor

withoutboats commented Dec 8, 2017

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.

@flosse
Copy link
Author

flosse commented Dec 8, 2017

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`

@withoutboats
Copy link
Contributor

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

@withoutboats
Copy link
Contributor

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!

@flosse
Copy link
Author

flosse commented Dec 10, 2017

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).

@tismith
Copy link

tismith commented Jun 1, 2018

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.

@kpcyrd
Copy link

kpcyrd commented Jun 21, 2018

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

@RX4NT9UP
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants