Feedback from a new user.

[dpc]

I have decided to give snafu a shot. I'll provide unsolicited feedback in this thread, and keep adding stuff as I bump into them. Feel free to close & mute or even tell me to go away.

I landed at https://docs.rs/snafu/0.4.1/snafu/index.html , looked at the example. Seemed straightforward. I went into coding head-on.

First bump was:

error[E0271]: type mismatch resolving `<Redis as snafu::IntoError<Error>>::Source == redis::types::RedisError`
  --> src/main.rs:22:58
   |
22 |     let redis = redis::Client::open("redis://127.0.0.1").context( Redis );
   |                                                          ^^^^^^^ expected struct `snafu::NoneError`, found struct `redis::types::RedisError`
   |
   = note: expected type `snafu::NoneError`
              found type `redis::types::RedisError`

"What does this thing want?" open does not return Option. I looked back and forth, scratched my head a lot, and finally figure out that source is special, and I have to define it as a field in my enum.

So far it was:

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

type Result<T, E = Error> = std::result::Result<T, E>;

and nowhere was it told (or I haven't noticed) that source is special. Now I get it, and it makes sense, but it's a bump and googling didn't help when I tried.

[dpc]

It's not clear to me how to "organize" my errors. One big error? Many? I've seen somewhere on reddit that I'm supposed to define new Error in each module or something, and aggregate them in the top one?

[dpc]

Hmm... For some errors I'd like to ignore the source altogether, because I don't want to tie the error type with the type of underlying issue.... hmmmm... I can't do it with context...

[dpc]

(just to be clearn - I'm not neccesarily looking for anwsers, just describing my thought process)

Is there any way to turn an error into snafu-error automatically / programmaticaly (Into) like in failure (I used failure before, so I tend to think in terms of it). I wish I could just ? and snafu error conversion would happen programmaitcally. Typing .context everywhere is kind of annoying ... donno ... maybe I'm just thinking incorrectly about the whole approach here. But many times underlying error is 1:1 with the outer error defined by my module...

I wish there was more examples. I know I'm am a person that can figure a lot, just by looking at examples. Looking through long documentation documents is not my thing.

[dpc]

Maybe https://docs.rs/snafu/0.4.1/snafu/trait.IntoError.html ? Hard to tell.

[dpc]

I've decided to start morning with reading any existing discussion from reddit/Rust fora about snafu. Turned out, I've been looking exactly for the pattern described here: https://www.reddit.com/r/rust/comments/bgy80y/any_experiences_with_snafu/elp5m3g/

[dpc]

I got at least enough understanding to help myself with:

trait ResultExt {
    type T;
    fn erased(self)
        -> std::result::Result<Self::T, Box<std::error::Error + 'static + Send + Sync>>;
}

impl<T, E> ResultExt for std::result::Result<T, E>
where
    E: std::error::Error + 'static + Send + Sync,
{
    type T = T;
    fn erased(self) -> std::result::Result<T, Box<dyn std::error::Error + 'static + Send + Sync>> {
        self.map_err(|e| Box::new(e) as Box<_>)
    }
}

so now I can:

#[derive(Debug, Snafu)]
pub enum Error {
    DeserializeRequest {
        source: Box<dyn std::error::Error + Send + Sync>,
    },
    IO {
        source: Box<dyn std::error::Error + Send + Sync>,
    }
}

and then just:

        serde_json::from_str(&s)
            .erased()
            .context(DeserializeRequest)?

[dpc]

Hmmm... I really like the ability to manually add context, but on the other hand using snafu so far is really tedious and noisy, often in places where I have nothing to add to the context. I really miss the ability to use "any-error" as source without having to use manual type conversion method. I also would appreciate some "inside context" where I could declare the context beforehand, and execute bunch of code (closure) inside it.

[dpc]

I guess I basically want some of the auto-type conversions from failure with ability to handily define an error context as in snafu.

[dpc]

Hmmm... I was actually able to automate the conversion by just implementing

impl std::convert::From<redis::RedisError> for super::Error {
   fn from(e: redis::RedisError) -> Self {
       super::Error::IO {
           source: Box::new(e),
       }
   }
}

I guess this is OK as a workaround for me right now, but it's just a chore right now. I'd rather have it derived somehow.

[shepmaster]

I'll provide unsolicited feedback in this thread, and keep adding stuff as I bump into them. Feel free to close & mute or even tell me to go away.

Thank you very much! Getting feedback from that first impression can only happen once, so I'm happy you captured it.

source is special, and I have to define it as a field in my enum

It's interesting because it's both special and not special. You have to include it in your enum just like any other field (practically, we can't modify a struct in a derive macro anyway). However, we use the field name source automatically to decide if there's an underlying error.

It's not clear to me how to "organize" my errors. One big error? Many?

My default is each module should have an Error type (mentioned in the philosophy section of the user guide).

It's certainly possible to have one project-wide error and that likely makes sense in some cases; I do think this is a case-by-case decision.

For some errors I'd like to ignore the source altogether, because I don't want to tie the error type with the type of underlying issue

#70

I wish I could just ? and snafu error conversion would happen programmaitcally.

#78

Typing .context everywhere is kind of annoying ... donno ... maybe I'm just thinking incorrectly about the whole approach here. But many times underlying error is 1:1 with the outer error defined by my module...

This may be true, but a big reason for this library is to accrete valuable context to provide to the user as the error bubbles outward. If you never have extra context to add, this library may not be the best fit for you.

    serde_json::from_str(&s)
        .erased()
        .context(DeserializeRequest)?

automate the conversion by just implementing

This can be handled directly by SNAFU (guide: Transforming the source):

use snafu::{ResultExt, Snafu};

#[derive(Debug, Snafu)]
pub enum Error {
    DeserializeRequest {
        #[snafu(source(from(serde_json::Error, Box::new)))]
        source: Box<dyn std::error::Error + Send + Sync>,
    },
}

fn main() -> Result<(), Error> {
    let input = r#""hello""#;
    let _s: String = serde_json::from_str(input).context(DeserializeRequest)?;

    Ok(())
}

I also would appreciate some "inside context" where I could declare the context beforehand, and execute bunch of code (closure) inside it.

I don't understand what you mean here, could you provide a code snippet of how it would look?

I don't want to tie the error type with the type of underlying issue

Why is that? Are you exposing this error type in your API?

[dpc]

https://docs.rs/snafu/0.4.1/snafu/guide/attributes/index.html#transforming-the-source could use examples of what changes in the code handling errors or something. Otherwise it's not immediately clear what have actually changed.

[dpc]

BTW. After a while, I wrote some stuff about context on internals forum: https://internals.rust-lang.org/t/thoughts-on-error-context-in-error-handling-libraries/10349/8 . If I ever have any more thoughts, I might add more comments here.

Thank you for reading through this and all the great work!