Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider providing a bail!() macro #192

Closed
lucab opened this issue Nov 8, 2019 · 5 comments · Fixed by #287
Closed

Consider providing a bail!() macro #192

lucab opened this issue Nov 8, 2019 · 5 comments · Fixed by #287
Labels
enhancement New feature or request feedback requested User feedback desired on a design decision help wanted Extra attention is needed

Comments

@lucab
Copy link
Contributor

lucab commented Nov 8, 2019

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?

@lucab lucab changed the title consider providing a bail!() macro Consider providing a bail!() macro Nov 8, 2019
@shepmaster
Copy link
Owner

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
Copy link
Contributor Author

lucab commented Nov 8, 2019

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:

  • quickly returning an error type: this case is only a shortcut, similar to ensure, and the user has to construct the error content.
  • an easy way to return a string-error: this additionally involves a bit of .into (or similar type-juggling) and a compatible variant (e.g. the Any one from the failure example).

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
Copy link
Owner

  • 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
Copy link
Owner

shepmaster commented Oct 20, 2020

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);
  • How would we handle a cause?
  • Could we handle static strings without allocation?

@shepmaster shepmaster pinned this issue Nov 5, 2020
@shepmaster
Copy link
Owner

shepmaster commented Nov 19, 2020

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.

@shepmaster shepmaster added enhancement New feature or request feedback requested User feedback desired on a design decision help wanted Extra attention is needed labels Nov 19, 2020
@shepmaster shepmaster linked a pull request May 4, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feedback requested User feedback desired on a design decision help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants