shepmaster / snafu

Easily assign underlying errors into domain-specific errors while adding context
https://docs.rs/snafu/
Apache License 2.0
1.45k stars 61 forks source link

Consider providing a bail!() macro #192

Closed lucab closed 3 years ago

lucab commented 5 years ago

I haven't seen any previous discussion on this, so I'm opening this ticket mostly to see what is @shepmaster's stance on this.

Coming from past experiences with error_chain and failure, I got used to the bail!() macros they both provide (with slightly different semantics) for quickly generating+returning errors:

Have you maybe already considered that in the past for snafu? Do you have additional thoughts to share?

shepmaster commented 5 years ago

Does SNAFU vs. Failure / "Strings as errors" cover this topic?

It's unclear exactly how such a macro would work with SNAFU, and what kind of overlap it would have with ensure!. Could you expand with a more complete example of how you would use bail! with SNAFU?

lucab commented 5 years ago

Does SNAFU vs. Failure / "Strings as errors" cover this topic?

Yes, I think the common usage of bail!() is very similar to what is described there. And I understand that string-errors are not the main usecase for snafu, but they are still fairly common (especially for developers coming from other languages).

It's unclear exactly how such a macro would work with SNAFU, and what kind of overlap it would have with ensure!. Could you expand with a more complete example of how you would use bail! with SNAFU?

At a basic level, it could be seen as a special case for ensure!(false, ...), I agree.

With a bit more space to roam, I think (but I haven't experimented enough yet) that there are two common UX cases that could be covered by some macro:

error_chain has an always-generated String variant (see generated example) which is used by bail!(). I don't think the "always" part was a particular good idea, but I think some quick way of turning a string into an error variant (and Err-return it) would be appreciated.

To be honest, I'm not advocating to add this to the library as it isn't a fundamental issue IMHO. But I saw this low-hanging syntactic sugar gap and was wondering whether it was omitted on purpose or a potential improvement.

shepmaster commented 5 years ago
  • quickly returning an error type: this case is only a shortcut, similar to ensure, and the user has to construct the error content.

Is this much better than the current

return ContextSelector {/* ... */}.fail();
// bail!(ContextSelector {/* ... */});

I don't think the "always" part was a particular good idea, but I think some quick way of turning a string into an error variant (and Err-return it) would be appreciated.

One solution I could see would be something like:

#[derive(Debug, Snafu)]
#[snafu(create_other_error(SomeName))]
enum Error {
    // Optionally other variants
}

This would expand to

enum Error {
    SomeName(String),
}

impl HasOtherError for Error {
    fn construct(s: String) -> Self {
        Error::SomeName(s)
    }
}

And the core library has

trait HasOtherError {
    // ...
}

// Some implementation of the macro that allows
fn foo() -> Result<(), Error> {
    bail!("wow {}", 42);
}

One big "problem" with this is that the current derive-based implementation cannot do this, as it involves rewriting the enum itself. We'd have to switch to a full-powered procedural macro, which means it'd actually look something like:

#[derive(Debug)]
#[Snafu]
#[snafu(create_other_error(SomeName))]
enum Error { /* ... */ }

Switching to a procedural macro is something I've toyed around with (couldn't do before due to supporting older Rust versions) as using derive to create the context selectors is unexpected by many users.

shepmaster commented 4 years ago

More spitballing:

enum Error {
    #[snafu(other)]
    Other { message: String }
}

bail!("oh noes {}", 42);

becomes

enum Error {
    Other { message: String }
}

impl snafu::FromString for Error {
    fn from_string(message: String) -> Self {
        Error::Other { message: String }
    }
}

return Err(snafu::FromString::from_string(format!("oh noes {}", 42))).map_err(Into::into);
shepmaster commented 3 years ago

OK, I'm working towards something. Here's an example of what it looks like now:

use snafu::{whatever, ResultExt};

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

fn subtract_numbers(a: u32, b: u32) -> Result<u32> {
    if a > b {
        Ok(a - b)
    } else {
        whatever!("Can't subtract {} - {}", a, b)
    }
}

fn complicated_math(a: u32, b: u32) -> Result<u32> {
    let val = subtract_numbers(a, b).whatever_context("Can't do the math")?;
    Ok(val * 2)
}

In addition, you can add the "whatever" support to your own error types in addition to any more structured errors:

#[derive(Debug, Snafu)]
enum Error {
    #[snafu(whatever, display("{}", message))]
    Whatever {
        #[snafu(source(from(Box<dyn std::error::Error>, Some)))] // FIXME
        source: Option<Box<dyn std::error::Error>>,
        message: String,
        backtrace: Backtrace,
    },
}

Would anyone be up for some beta testing / doc reading? If so, check out the branch.