Consider how to support tuple variants about snafu HOT 8 OPEN

Given that you have to annotate the source and backtrace in the tuple struct, i feel like you're not gaining very much (assuming you want to use a tuple struct to save some keystrokes). And as shown in your example – it could be very confusing in the order dependent usage with context selectors. I don't feel like this is an overall win to the library. I think its only purpose here is to ease the transition from Failure? If that is the only real usecase, a prominent explanation (migration guide?) could be enough the make transition easier.

from snafu.

use dot instead of underscore

That's interesting! Of course it's written by dtolnay, which explains all the fancy custom parsing going on there. That's the big problem of such a solution: .0 isn't a valid expression.

fn main() {
    println!("{}", .0);
}

error: float literals must have an integer part
 --> src/main.rs:2:20
  |
2 |     println!("{}", .0);
  |                    ^^ help: must have an integer part: `0.0`

That means that thiserror has some code that attempts to parse the format expression arguments. If it finds anything that matches, it ... does something with it. I'm not yet sure what it's rewritten to. There's probably a hidden variable (such as __foo) that's been bound to the values of the tuple, but I don't obviously see that code.

The big downsides I see of such a path is that all of that parsing has to be custom, which seems complicated seeing as how any valid expression can be put in the format. There's also a worry in the back of my head that the Rust compiler will decide to accept .0 as a valid floating point literal as that error message is much higher than I'd expect.

from snafu.

My current thinking is to make tuple variants effectively equivalent to this:

#[derive(Debug, Snafu)]
enum Error {
    #[snafu(context(false))]
    OneWay { source: AnError },

    SameThing(AnError),
}

That is, a tuple variant can have exactly 1 element which must be a source error. It functions the same as a context-less variant and would be available to the Display implementation as source. This will work hand-in-hand with the struct error ability (#250 and #199).

from snafu.

I'm using failure but I want to migrate to snafu and I find tuples very useful then I want to handle the error with a match of an it let.

In the following example assume that ErrorB and ErrorC are from different crates:

use snafu::ResultExt;
use snafu::Snafu;

#[derive(Debug, Snafu)]
pub enum ErrorA {
    #[snafu(display("ErrorA1"))]
    ErrorA1,

    #[snafu(display("ErrorA::B: {}", source))]
    B { source: ErrorB },
}

#[derive(Debug, Snafu)]
pub enum ErrorB {
    #[snafu(display("ErrorB1"))]
    ErrorB1,

    #[snafu(display("ErrorB::C: {}", source))]
    C { source: ErrorC },
}

#[derive(Debug, Snafu)]
pub enum ErrorC {
    #[snafu(display("ErrorC1"))]
    ErrorC1,
}

fn run() -> Result<(), ErrorA> {
    Err(ErrorC::ErrorC1).context(C).context(B)?
}

fn main() {
    match run() {
        Ok(()) => {
            // all fine
        }
        Err(ErrorA::B {
            source: ErrorB::C {
                source: ErrorC::ErrorC1,
            },
        }) => {
            // do something special
        }
        Err(e) => {
            // display error and exit
            println!("{}", e);
            std::process::exit(1);
        }
    }
}

With failure the same match will be written as:

    match run() {
        Ok(()) => {
            // all fine
        }
        Err(ErrorA::B(ErrorB::C(ErrorC::ErrorC1))) => {
            // do something special
        }
        Err(e) => {
            // display error and exit
            println!("{}", e);
            std::process::exit(1);
        }
    }

IMO with tuples you can write cleaner code in this case.

from snafu.

@oblique thanks for your insights! I don't really see how this is "cleaner" (whatever this means). You write less, yes – but i don't think that's all there is to "cleaner code". I see your usecase and the demand to write less but i don't think it outweighs the problems that this would introduce e.g with context selectors.

from snafu.

That was the first issue I've stumbled in before I even try to use snafu, and here's my first thoughts. I have not found any explicit notice that tuple-structs are unsupported in user's guide. Before it is implemented (or decided to not) I think it's better be documented with link to that issue.

Some existing external types might be a tuple struct. E.g. std::mpsc::SendError, thus for #snafu(display(..)) it is pretty awkward to get inner value, since you need to manually write a getter like this (or am I missing something?):

fn send_error_inner<T>(err: &mpsc::SendError<T>) -> &T {
    &err.0
}

#[derive(snafu::Snafu)]
pub enum Error {
    #[snafu(display("err: {}", send_error_inner(&source)))]
    SomeError { source: mpsc::SendError<Command> },
    ...
}

The other thing - tuple-struct support might be a helpful for migration. So rewriting existing error types might need less to change for getting a somewhat result. Thus in my opinion it is better be added, but with a notice of "undesired usage". For example that might be just a line in guide, or even a warning (hopefully with suppressing).

As an implementation, I prefer accessing fields in failure-like style (e.g. _1, _2, ...). Introducing a special keyword seems obscure and might conflict with other identifiers.

from snafu.

I have not found any explicit notice that tuple-structs are unsupported in user's guide

There are plenty of things that any given library doesn't support; listing all of them seems counterproductive. Where would you suggest putting this information in the guide?

Some existing external types might be a tuple struct

Accessing a tuple inside of the enum variant works fine:

#[derive(Debug, snafu::Snafu)]
pub enum Error {
    #[snafu(display("err: {:?}", source.0))]
    SomeError { source: std::sync::mpsc::SendError<std::process::Command> },
}

tuple-struct support might be a helpful for migration

Perhaps, which is a reason that this has remained open, but is blocked on figuring out enough pieces to be useful without making the rest of the library worse.

I prefer accessing fields in failure-like style (e.g. _1, _2, ...). Introducing a special keyword seems obscure and might conflict with other identifiers.

Using underscores is already a conflict:

const _2: i32 = 42;

#[derive(Debug, snafu::Snafu)]
pub enum Error {
    #[snafu(display("example: {} {}", _1, _2))]
    SomeError { _1: i32 },
}

from snafu.

You could use dot instead of underscore, the same way thiserror does.

from snafu.