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

Can I use context for any error type that implements a trait without providing an exhaustive list of all possible source error types? #123

Open
fan-tom opened this issue Jul 23, 2019 · 13 comments
Labels
found in the field A user of SNAFU found this when trying to use it support request Help requested on using the project

Comments

@fan-tom
Copy link
Contributor

fan-tom commented Jul 23, 2019

I need to have Box<dyn std::error::Error> as source in some of my error variants. Can I achieve that without manual boxing of error before passing it to error selector?

@shepmaster shepmaster changed the title Allow trait as source Can I use Box<dyn std::error::Error> without manually boxing it before passing it to an error selector? Jul 23, 2019
@shepmaster shepmaster added found in the field A user of SNAFU found this when trying to use it support request Help requested on using the project labels Jul 23, 2019
@shepmaster
Copy link
Owner

You should be able to using source(from). Can you check the relevant section of the guide and see if that solves your problem?

@shepmaster
Copy link
Owner

/cc #104

@shepmaster
Copy link
Owner

If that section of the guide helps you, I'd appreciate any feedback on making it easier to discover. All the documentation I could write will be useless if no one finds it.

@fan-tom
Copy link
Contributor Author

fan-tom commented Jul 23, 2019

You should be able to using source(from). Can you check the relevant section of the guide and see if that solves your problem?

Yes, I have read that section, but it mentions concrete type as source to transformation, while I need to be able to pass any type that satisfies concrete trait, such as std::error::Error and get it boxed automatically.
Now I can achive that using explicit conversation

    #[derive(Snafu)]
    pub enum CoalesceIoError
    {
        FileOpenErrror { source:  std::io::Error},
        OtherError { source: Box<dyn std::error::Error> },
    }
    
    fn send_file_over_network(file: tokio::fs::File) -> impl Future<Item=(), Error=network::Error> {
    // ...
    }

    fn usage(path: PathBuf) {
        tokio::fs::File::open(&path).context(FileOpenError{})
        .and_then(|file| {
             send_file_over_network(file)
                 .map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
                 .context(OtherError{})
          })
         // other stuff
    }

What I want to be able to do is something like

send_file_over_network(file)
                 // no need to map explicitly
                 .context(OtherError{})

@shepmaster
Copy link
Owner

This is a version of your code that actually compiles. When requesting help from people, I would encourage you to provide these examples yourself; there are many more people asking for help than there are to provide it (just me for SNAFU!). Creating and providing these makes the process of helping much faster, which I'm sure you'd appreciate:

use tokio::prelude::*; // 0.1.22
use snafu::{Snafu, futures01::FutureExt}; // 0.4.2
use std::path::PathBuf;

mod network {
    use snafu::Snafu;

    #[derive(Debug, Snafu)]
    pub enum Error {}
}

#[derive(Debug, Snafu)]
pub enum CoalesceIoError {
    FileOpenError { source: std::io::Error },
    OtherError { source: Box<dyn std::error::Error> },
}

fn send_file_over_network(_file: tokio::fs::File) -> impl Future<Item = (), Error = network::Error> {
    future::ok(())
}

fn usage(path: PathBuf) {
    tokio::fs::File::open(path)
        .context(FileOpenError {})
        .and_then(|file| {
            send_file_over_network(file)
                .map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
                .context(OtherError {})
        });
    // other stuff
}

fn main() {}

Starting from this, source(from) appears to allow the code you want:

#[derive(Debug, Snafu)]
pub enum CoalesceIoError {
    FileOpenError { source: std::io::Error },
    OtherError {
        #[snafu(source(from(network::Error, Box::new)))]
        source: Box<dyn std::error::Error>,
    },
}

fn usage(path: PathBuf) {
    tokio::fs::File::open(path)
        .context(FileOpenError)
        .and_then(|file| {
            send_file_over_network(file)
                .context(OtherError)
        });
}

If this isn't what you are asking, can you try constructing a more representative example?

Other things to note:

  • You don't need the {} for a context selector without extra fields.

@fan-tom
Copy link
Contributor Author

fan-tom commented Jul 23, 2019

When requesting help from people, I would encourage you to provide these examples yourself

Sorry for that, I thought that my wishes are clear, but it seems that they aren't.
What I want to be able to do is to not provide exhaustive list of all possible source error types here

#[snafu(source(from(network::Error, Box::new)))]

I just want to say that any type implementing specified trait is appropriate. Imagine that I want to separate errors in such a way that I don't want to distinguish sources, so many different source errors are possible causes of my single domain error. The only thing I want from them is to be representable as trait objects (std::error::Error or Display or whatever else) to be able to, for example, print them. Imagine that in my example with file there are many other send_file_over_network-like functions, each of them returns different error type, and I want to distribute them between small number of my domain errors. Or maybe I want to distinguish one type of errors from all others (for example in actix framework I want to distinguish MailboxError from inner errors of returned responses, so I introduced

    #[derive(Snafu)]
    pub enum CoalesceMailboxError
    {
        MailboxErrorOccurred { source: MailboxError },
        DomainError { source: Box<dyn std::error::Error> },
    }

But with that type I cannot simply use plain context method and must manually box them by mapping before, because inner error may be anything, I just know that it is std::error::Error)

@shepmaster shepmaster changed the title Can I use Box<dyn std::error::Error> without manually boxing it before passing it to an error selector? Can I use context for any error type that implements a trait without providing an exhaustive list of all possible source error types? Jul 24, 2019
@shepmaster
Copy link
Owner

I want to separate errors in such a way that I don't want to distinguish sources, so many different source errors are possible causes of my single domain error

I think this is a key point. Many similar libraries map one underlying error (e.g. std::io::Error) to one "domain" error variant (e.g. mycrate::Error::Io). Call this 1->1. One key differentiator that SNAFU brings is stating that this isn't usually the best idea. Instead, one underlying type might map to multiple domain errors (e.g. mycrate::Error::LoadConfig and mycrate::Error::SaveConfig). Call this 1->N.

Your request sounds like the counterpart: N->1.

With the current tools that SNAFU provides, I don't see a direct path. The main problem is that context requires that the selector implements IntoError which in turn uses an associated type for the underlying error. If I recall correctly, we need the associated type as opposed to a generic in order to be able to do proper inference.

Effectively, you'd like to be able to write this:

impl<E> IntoError<Error> for Selector
where
    E: Into<Box<dyn std::error::Error>>,
{
    type Source = E;
    
    fn into_error(self, source: Self::Source) -> Error {
        unimplemented!()
    }
}

...but...

error[E0207]: the type parameter `E` is not constrained by the impl trait, self type, or predicates
 --> src/lib.rs:4:6
  |
4 | impl<E> IntoError<Error> for Selector
  |      ^ unconstrained type parameter

Instead, you might be able to just get by with your own extension trait (untested):

trait LooseResultExt {
    fn loose_context<C, E2>(self, context: C) -> Result<T, E2>
    where
        C: IntoError<E2, Source = E>,
        E2: Error + ErrorCompat;
}

impl<T, E> LooseResultExt for Result<T, E> {
    fn loose_context<C, E2>(self, context: C) -> Result<T, E2>
    where
        E: std::error::Error,
        C: IntoError<E2, Source = Box<dyn std::error::Error>>,
        E2: Error + ErrorCompat,
    {
        self.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
            .context(context)
    }
}

@fan-tom
Copy link
Contributor Author

fan-tom commented Jul 24, 2019

N->1, yes, exactly what I mean. Thanks, I'll try extention trait. I think in SNAFU it may be possible to do that by form #[snafu(from(dyn std::error::Error, Box::new))] and then something like Box::new(<error>) as Box<dyn std::error::Error> where currently it is simply Box::new(<error>)

@shepmaster
Copy link
Owner

it may be possible to do that by form #[snafu(from(dyn std::error::Error, Box::new))]

I don't understand what this implementation would look like; would it be possible for you to provide some code demonstrating it working?

@Ploppz
Copy link

Ploppz commented Aug 18, 2019

I came here because I have the same problem.
Just to give an idea why this feature may be desirable:

Several places in my application, I have to deserialize JSON. One example is in a request with reqwest: https://docs.rs/reqwest/0.9.19/reqwest/async/struct.Response.html#method.json . Another example: https://docs.rs/serde_json/1.0.40/serde_json/de/fn.from_str.html
I thought maybe I could have one Error variant DeserializeJson, but that seems impossible right now because of the different source Error types. Anyway perhaps it is neutral or even positive to separate the error variants based on the source error after all; I'm not sure.

@shepmaster
Copy link
Owner

I thought maybe I could have one Error variant DeserializeJson, but that seems impossible right now because of the different source Error types

@Ploppz there have been mentions in passing of supporting multiple from attributes:

enum Error {
    JsonDeserialization { 
        #[snafu(from(SerdeError, Box::new))]
        #[snafu(from(ReqwestError, Box::new))]
        source: Box<dyn Error>,
    }
}

No one has taken the time to open an issue requesting such a feature yet.

Anyway perhaps it is neutral or even positive to separate the error variants based on the source error after all

SNAFU would encourage you to separate the variants based on what you are doing. I don't know the domain of your application, but instead of having a DeserializeJson, I think you want something like

enum Error {
    UserRecordMissing { source: ReqwestError },
    ConfigurationFileMissing { source: SerdeError },
}

You could also combine the concepts, if both of your operations could use either method to get JSON:

enum DeserializeJsonError {
    Reqwest { source: ReqwestError },
    Serde { source: SerdeError },
}

enum Error {
    UserRecordMissing { source: DeserializeJsonError },
    ConfigurationFileMissing { source: DeserializeJsonError },
}

@io12
Copy link

io12 commented Nov 10, 2019

This seems to work. I don't think it's possible to resolve this without changing the trait or adding another trait.

trait IntoError<S, E>
where
    S: Error + ErrorCompat,
    E: Error + ErrorCompat,
{
    fn into_error(self, source: S) -> E;
}

impl<S> IntoError<S, MyError> for MySelector
where
    S: Error + ErrorCompat,
{
    fn into_error(self, _source: S) -> MyError {
        unimplemented!()
    }
}

@carols10cents
Copy link

carols10cents commented Jan 25, 2021

Adding another use case: when the type of the source error is a trait's associated type. It'd be nice to not need the .map_err(|e| Box::new(e) as _), but I don't think there's anything that we could put in #[snafu(source(from(???, Box::new)))] that would work:

use snafu::{ResultExt, Snafu};
use std::sync::Arc;
use std::fmt::Debug;

#[derive(Debug, Snafu)]
pub enum Error {
    #[snafu(display("Partition error creating snapshot: {}", source))]
    PartitionError {
        // what to put in here? Ideally without introducing generics into this Error type
        // #[snafu(source(from(???, Box::new)))]
        source: Box<dyn std::error::Error + Send + Sync>,
    },

}

#[derive(Debug)]
pub struct Snapshot<T>
where
    T: Send + Sync + 'static + PartitionChunk,
{
    partition: Arc<T>,
}

impl<T> Snapshot<T>
where
    T: Send + Sync + 'static + PartitionChunk,
{
    pub fn run(&self) -> Result<(), Error> {
        self.partition
            .id()
            // would like to be able to remove this:
            .map_err(|e| Box::new(e) as _)
            .context(PartitionError)?;
        Ok(())
    }
}

pub trait PartitionChunk: Debug + Send + Sync {
    type Error: std::error::Error + Send + Sync + 'static;

    fn id(&self) -> Result<u32, Self::Error>;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
found in the field A user of SNAFU found this when trying to use it support request Help requested on using the project
Projects
None yet
Development

No branches or pull requests

5 participants