-
Notifications
You must be signed in to change notification settings - Fork 61
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
Embedding a whatever broken on nightly with unstable-provider-api
#373
Comments
I just tried building another project of mine against the latest nightly with Adding
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 #[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 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:
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 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:
|
Note that the provided Lines 1462 to 1474 in 00bba4b
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...
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. |
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,
}
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 |
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.
Only reason I needed it was that it wouldn't print backtrace otherwise. I don't want to use unstable features.
Not my intention, I'm not a native English speaker, though I was also a bit annoyed myself when I wrote this.
Exactly! |
The points made so far seem to suggest that you would be better served with
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);
}
} |
If you wanted this in stable today, I'd leverage 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()
}
[dependencies]
snafu = { version = "0.7.4", features = ["rust_1_61", "backtraces-impl-std"] } |
Unless there's something I'm missing, to have that work without the provider API, the |
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 thereport
macro.However enabling that breaks embedding a
Whatever
in my custom error type:Results in:
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.
The text was updated successfully, but these errors were encountered: