snafu icon indicating copy to clipboard operation
snafu copied to clipboard

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

Open fan-tom opened this issue 5 years ago • 13 comments

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?

fan-tom avatar Jul 23 '19 15:07 fan-tom

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 avatar Jul 23 '19 16:07 shepmaster

/cc #104

shepmaster avatar Jul 23 '19 16:07 shepmaster

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.

shepmaster avatar Jul 23 '19 16:07 shepmaster

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{})

fan-tom avatar Jul 23 '19 18:07 fan-tom

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.

shepmaster avatar Jul 23 '19 18:07 shepmaster

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)

fan-tom avatar Jul 23 '19 20:07 fan-tom

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

shepmaster avatar Jul 24 '19 01:07 shepmaster

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

fan-tom avatar Jul 24 '19 06:07 fan-tom

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?

shepmaster avatar Aug 12 '19 13:08 shepmaster

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.

Ploppz avatar Aug 18 '19 12:08 Ploppz

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 },
}

shepmaster avatar Aug 19 '19 15:08 shepmaster

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!()
    }
}

io12 avatar Nov 10 '19 08:11 io12

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>;
}

carols10cents avatar Jan 25 '21 20:01 carols10cents