Embedding a whatever broken on nightly with `unstable-provider-api`

[VorpalBlade]

I'm trying to get backtraces to work. Apparently I need unstable-provider-api (if I understand the docs correctly) for this to work with the report macro.

However enabling that breaks embedding a Whatever in my custom error type:

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

Results in:

error[E0599]: the method `provide` exists for reference `&Option<Box<dyn Error>>`, but its trait bounds were not satisfied
   --> src/errors.rs:5:17
    |
5   | #[derive(Debug, Snafu)]
    |                 ^^^^^ method cannot be called on `&Option<Box<dyn Error>>` due to unsatisfied trait bounds
    |
   ::: /home/arvid/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:563:1
    |
563 | pub enum Option<T> {
    | ------------------ doesn't satisfy `Option<Box<dyn snafu::Error>>: snafu::Error`
    |
    = note: the following trait bounds were not satisfied:
            `Option<Box<dyn snafu::Error>>: snafu::Error`
            which is required by `&Option<Box<dyn snafu::Error>>: snafu::Error`
    = note: this error originates in the derive macro `Snafu` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> src/errors.rs:49:9
    |
5   | #[derive(Debug, Snafu)]
    |                 ----- arguments to this method are incorrect
...
49  |         source: Option<Box<dyn std::error::Error>>,
    |         ^^^^^^ expected `&Box<dyn Error>`, found `&Option<Box<dyn Error>>`
    |
    = note: expected reference `&Box<dyn snafu::Error>`
               found reference `&Option<Box<(dyn snafu::Error + 'static)>>`
note: method defined here
   --> /home/arvid/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/any.rs:953:12
    |
953 |     pub fn provide_ref<T: ?Sized + 'static>(&mut self, value: &'a T) -> &mut Self {
    |            ^^^^^^^^^^^

Do I need a different whatever definition? If so, this should be in the docs. I find this whole crate to be worse documented than thiserror or anyhow. The only reason I wanted to use snafu (on stable to begin with) was to get backtraces output when errors bubble up out of main. The docs for this needs improvement.

[Enet4]

I just tried building another project of mine against the latest nightly with unstable-provider-api and it also failed in deriving Snafu for an error enum with a whatever variant. Likewise, it expected the Optional<_> of a source field to be constrained against Error.

Adding -Z macro-backtrace to the rustc flags also shows this:

pub fn snafu_derive(input: TokenStream) -> TokenStream {
    | ------------------------------------------------------ in this expansion of `#[derive(Snafu)]`
    |

I was close to calling this a bug in the current provider API implementation, but then I checked the docs again and saw that it expects users to annotate members which are optionally provided with provide(opt).

    #[snafu(source(from(Box<dyn std::error::Error>, Some)))]
    #[snafu(provide(opt))]
    source: Option<Box<dyn std::error::Error>>,

This might work then, although I wonder if future versions of snafu couldn't infer provide(opt) in this case, at least from source fields. It is a bit surprising for an additional feature to introduce a breaking change like this.

---

The remaining confusion might be attributed to the fact that snafu is more complex than thiserror and anyhow combined, but concrete suggestions for improvement should be rather welcomed!

[VorpalBlade]

The remaining confusion might be attributed to the fact that snafu is more complex than thiserror and anyhow combined, but concrete suggestions for improvement should be rather welcomed!

Indeed! I think that the issue is one of documentation. Most users will not read the docs end to end, but try to find what is relevant to what they are trying to accomplish. I went to the quickstart and guide sections primarily to begin with.

Quoting the quickstart:

You can combine the power of the whatever! macro with an enum error type. This is great if you started out with Whatever and are moving to a custom error type: [code removed] You may wish to make the type Send and/or Sync, allowing your error type to be used in multithreaded programs, by changing dyn std::error::Error to dyn std::error::Error + Send + Sync.

And https://docs.rs/snafu/latest/snafu/derive.Snafu.html#controlling-stringly-typed-errors says:

        // Having a `source` is optional, but if it is present, it must
        // have this specific attribute and type:
        #[snafu(source(from(Box<dyn std::error::Error>, Some)))]
        source: Option<Box<dyn std::error::Error>>,

Notice the key phrase "have this specific attribute and type". Which I indeed double checked that I did!

Neither of these pages, nor the section on the provider-api flag at https://docs.rs/snafu/latest/snafu/guide/feature_flags/index.html mentions provide(opt) must be added to the whatever variant. It also only affected the whatever variant, not the other enum variants for me.

---

In general I feel the docs are a bit badly organised. It is kind of hard to put my finger on exactly what the issue is:

• Perhaps it is not crosslinked enough between relevant topics.
• Perhaps it is that the guide docs could be more comprehensive.
• Perhaps it is that the rather large amount of feature flags that have effects across the whole crate and interact with each other (as opposed to something like the nix crate that has a lot of features flags, but all they do are enable specific modules).
• Perhaps it is a mix of all of these.

[shepmaster]

Note that the provided Whatever does something different by disabling the default providing and chaining the provider API to the source trait object:

https://github.com/shepmaster/snafu/blob/00bba4b4243a340cdcafc7a8478f9e26b860692d/src/lib.rs#L1462-L1474

---

Potentially one very-forward-looking solution would be to finally migrate from a derive macro to a procedural macro. Derive macros are not capable of modifying the code they are attached to, but procedural macros can. In that case, you could have something like the made-up syntax

#[snafu]
struct Error {
    // Rewrites the `Whatever` variant to add all of the fields and attributes
    // that the library suggests should be put on the `Whatever`.
    #[snafu(whatever)]
    Whatever,
}

---

Recall that you are using the intersection of advanced and unstable features.

Combining stringly-typed errors with structured errors is something that I expect most people will not want to do it (or for long). I'd be happy (but surprised) to learn that isn't the case. I assume that people that like stringly-typed errors tend to stick to anyhow. I assume it's when deciding to migrate towards structured errors that the combination of the stringly-typed and structured errors makes the most sense.

The provider API (and SNAFU's interaction with it) is unstable; it only makes so much sense to spend X time up front documenting and polishing features that might never actually materialize (although I expect the provider API to stabilize someday). Part of the reason that unstable things exist in the first place is to gather feedback and improve ergonomics. To that end, I echo @Enet4's points that I'm all ears for concrete suggestions for improvement.

---

Unfortunately, far too many interactions I have with folks regarding documentation basically boils down to "why doesn't the first page of documentation cover exactly my case and no others?". For example, you likely don't mind that the quickstart didn't discuss what you need to do to get the library to work on Rust 1.34 while targeting a no_std environment. Then from the past there's...

• group A who thinks all the documentation should be on one page and group B who thinks they should be separate
• group C who thinks that the documentation should redundantly state facts everywhere they are used and group C who thinks that one piece of information should exist once and exactly once
• group D who thinks that the documentation should focus on examples and group E who thinks it should focus on concepts
• etc.

Quite simply, I can't do all of that by myself (in the cases where it's not inherently impossible to meet both goals).

I have to admit I was rather dreading responding to this issue at all after reading it this morning. Your original tone really came across poorly and quite frankly made me angry. I'm sorry that the code / documentation quality has impacted you so negatively that you felt that this was the most appropriate way to get your point across, but it still makes me feel bad. Library maintainers are humans as well.

[shepmaster]

although I wonder if future versions of snafu couldn't infer provide(opt) in this case, at least from source fields.

I've generally tried to avoid this kind of inference because it's pretty brittle. Macros can only work with the literal strings of code, so they've no idea how to deal with things like:

type Foo = std::option::Option;
struct Option;

struct Error {
    thing_a: Foo<String>,
    thing_b: Option,
}

It is a bit surprising for an additional feature to introduce a breaking change like this.

That is indeed unfortunate, and unexpected. To clarify, you are saying you had something like

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

then added the unstable-provider-api feature flag and the code stopped compiling?

[VorpalBlade]

Recall that you are using the intersection of advanced and unstable features.

To be frank, I did not especially want to do any of this. All I wanted was backtraces on errors that escaped main(). I'm making a systemd service that got weird errors during boot only (worked fine while it was started on an already running system). I needed to see what those errors were.

As such I read the docs and tried to get the minimal working "thing" to solve my problem. All the while being in a bit of a hurry.

The provider API (and SNAFU's interaction with it) is unstable;

Only reason I needed it was that it wouldn't print backtrace otherwise. I don't want to use unstable features.

I have to admit I was rather dreading responding to this issue at all after reading it this morning. Your original tone really came across poorly and quite frankly made me angry.

Not my intention, I'm not a native English speaker, though I was also a bit annoyed myself when I wrote this.

That is indeed unfortunate, and unexpected. To clarify, you are saying you had something like [SNIP] then added the unstable-provider-api feature flag and the code stopped compiling?

Exactly!

[Enet4]

All I wanted was backtraces on errors that escaped main().

The points made so far seem to suggest that you would be better served with anyhow or eyre. At least the latter also has a report API of its own.

Only reason I needed it was that it wouldn't print backtrace otherwise. I don't want to use unstable features.

I think it should be possible for snafu Reports to print a backtrace if available, but it's probably only implemented through the unstable provider API at the moment. Unless there's something I'm overlooking here, this is something that could be tracked and resolved in a future version without waiting for the provider API to stabilize.

Alternatively, a custom error reporting function would do the trick, as I have done frequently in the past.

fn report<E: 'static>(err: &E)
where
    E: std::error::Error,
    E: snafu::ErrorCompat,
    E: Send + Sync,
{
    eprintln!("[ERROR] {}", err);
    if let Some(source) = err.source() {
        eprintln!();
        eprintln!("Caused by:");
        for (i, e) in std::iter::successors(Some(source), |e| e.source()).enumerate() {
            eprintln!("   {}: {}", i, e);
        }
    }

    if let Some(backtrace) = ErrorCompat::backtrace(err) {
        eprintln!("Backtrace:");
        eprintln!("{}", backtrace);
    }
}

[shepmaster]

If you wanted this in stable today, I'd leverage Report and your knowledge of the error type. A fully-worked example:

use snafu::prelude::*;

#[derive(Debug, Snafu)]
enum Error {
    ItWasABadDay { backtrace: snafu::Backtrace },
}

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

fn main() {
    if let Err(e) = inner_main() {
        let report = snafu::Report::from_error(&e);
        eprintln!("{report}");
        if let Some(bt) = snafu::ErrorCompat::backtrace(&e) {
            eprintln!("{bt}");
        }
    }
}

fn inner_main() -> Result<()> {
    could_fail()?;
    Ok(())
}

fn could_fail() -> Result<i32> {
    ItWasABadDaySnafu.fail()
}

ItWasABadDay

   0: std::backtrace_rs::backtrace::libunwind::trace
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1: std::backtrace_rs::backtrace::trace_unsynchronized
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2: std::backtrace::Backtrace::create
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/backtrace.rs:332:13
   3: <std::backtrace::Backtrace as snafu::GenerateImplicitData>::generate
             at /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/snafu-0.7.4/src/lib.rs:1252:9
   4: bt::ItWasABadDaySnafu::build
             at ./src/main.rs:3:17
   5: bt::ItWasABadDaySnafu::fail
             at ./src/main.rs:3:17
   6: bt::could_fail
             at ./src/main.rs:26:5
   7: bt::inner_main
             at ./src/main.rs:21:5
   8: bt::main
             at ./src/main.rs:11:21
   9: core::ops::function::FnOnce::call_once
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/ops/function.rs:507:5
  10: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/sys_common/backtrace.rs:121:18
  11: std::rt::lang_start::{{closure}}
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/rt.rs:166:18
  12: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/ops/function.rs:606:13
  13: std::panicking::try::do_call
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panicking.rs:483:40
  14: std::panicking::try
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panicking.rs:447:19
  15: std::panic::catch_unwind
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panic.rs:137:14
  16: std::rt::lang_start_internal::{{closure}}
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/rt.rs:148:48
  17: std::panicking::try::do_call
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panicking.rs:483:40
  18: std::panicking::try
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panicking.rs:447:19
  19: std::panic::catch_unwind
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panic.rs:137:14
  20: std::rt::lang_start_internal
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/rt.rs:148:20
  21: std::rt::lang_start
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/rt.rs:165:17
  22: _main

[dependencies]
snafu = { version = "0.7.4", features = ["rust_1_61", "backtraces-impl-std"] }

[shepmaster]

I think it should be possible for snafu Reports to print a backtrace if available, but it's probably only implemented through the unstable provider API at the moment. Unless there's something I'm overlooking here, this is something that could be tracked and resolved in a future version without waiting for the provider API to stabilize.

Unless there's something I'm missing, to have that work without the provider API, the Report type (and the #[report] macro) would need to require that the error passed in implement snafu::ErrorCompat in addition to std::error::Error. Effectively that means that you'd only be able to use it with SNAFU-created errors, and even then you wouldn't be able to leverage the ExitCode support.