Consider how to support tuple variants

[shepmaster]

People ask about tuple variants:

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

There are a few reasons I've avoided adding it this far.

Identifying source errors / backtraces

Right now, we tell what the underlying error is by using the name source (mirroring std::Error::source). #10 proposes extending this, so using the attribute from there, you'd be required to annotate the appropriate field:

#[derive(Debug, Snafu)]
enum Error {
    Something(#[snafu(source)] AnotherError, i32, #[snafu(backtrace)] Backtrace),
}

Context selectors would also be order-dependent

Since context selectors mirror the corresponding variant, they would become tuple structs. The automatically-handled fields would be removed, which means that the parameters of the selector are different:

#[derive(Debug, Snafu)]
enum Error {
    Something(Backtrace, String, AnotherError, i32),
}

fn example() {
    something().context(Something("name", 42))
}

Accessing the fields in Display has no standard syntax

Right now, we bind the fields in the variant to the given names, but tuples don't have valid names to use. We'd have to make up some syntax (like failure) or create some variable:

#[derive(Debug, Snafu)]
enum Error {
    // Bind them to variables with some prefix (here `_`)
    #[snafu(display("The {} is {}: {}", _1, _3, _2))]
    // Create another tuple with some fixed name (here `props`)
    #[snafu(display("The {} is {}: {}", props.1, props.3, props.2))]
    Something(Backtrace, String, AnotherError, i32),
}

---

None of the problems are insurmountable, but is this a pattern that we want to even encourage? What are the benefits of supporting tuple variants?

[pythoneer]

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.

[oblique]

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.

[pythoneer]

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

[l4l]

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.

[shepmaster]

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

[oblique]

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

[shepmaster]

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.

[shepmaster]

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