-
Notifications
You must be signed in to change notification settings - Fork 84
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
enable error backtraces #212
Conversation
I'm not entirely sure how to conditionally test this, but on a nightly toolchain, given this file in http-types/examples/backtrace.rs: fn main() {
dbg!(http_types::format_err!("error").backtrace());
}
[examples/backtrace.rs:2] http_types::format_err!("error").backtrace() = Backtrace [
{ fn: "backtrace::backtrace::libunwind::trace", file: "/Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs", line: 86 },
{ fn: "backtrace::backtrace::trace_unsynchronized", file: "/Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs", line: 66 },
{ fn: "std::backtrace::Backtrace::create", file: "src/libstd/backtrace.rs", line: 308 },
{ fn: "std::backtrace::Backtrace::capture", file: "src/libstd/backtrace.rs", line: 277 },
{ fn: "anyhow::error::<impl anyhow::Error>::msg", file: "/Users/jbr/.cargo/registry/src/github.com-1ecc6299db9ec823/anyhow-1.0.31/src/backtrace.rs", line: 10 },
{ fn: "http_types::error::Error::from_str", file: "src/error.rs", line: 40 },
{ fn: "http_types::error::Error::new_adhoc", file: "src/error.rs", line: 49 },
{ fn: "http_types::private::new_adhoc", file: "src/lib.rs", line: 199 },
{ fn: "backtrace::main", file: "examples/backtrace.rs", line: 2 },
{ fn: "std::rt::lang_start::{{closure}}", file: "/Users/jbr/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/rt.rs", line: 67 },
{ fn: "std::rt::lang_start_internal::{{closure}}", file: "src/libstd/rt.rs", line: 52 },
{ fn: "std::panicking::try::do_call", file: "src/libstd/panicking.rs", line: 348 },
{ fn: "std::panicking::try", file: "src/libstd/panicking.rs", line: 325 },
{ fn: "std::panic::catch_unwind", file: "src/libstd/panic.rs", line: 394 },
{ fn: "std::rt::lang_start_internal", file: "src/libstd/rt.rs", line: 51 },
{ fn: "std::rt::lang_start", file: "/Users/jbr/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/rt.rs", line: 67 },
{ fn: "main", line: 0 },
] |
8f92490
to
9446815
Compare
With the addition of 9446815, this is technically a breaking change. However, it's safe to say that zero apps were depending on this function, as was always conditionally compiled out regardless of toolchain and RUST_LIB_BACKTRACE. With that commit, here are three different outputs for the same examples/backtrace.rs example above:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woah; this is a massive hack. Very clever! Can we perhaps add a big prelude to the build file, linking to anyhow's file and this PR? Once Rust gains feature detection we can probably remove the file, but until then having a clear explainer and reference for what it is we're doing in the build.rs file would probably be good 😅
Added a nice big comment explaining how this works. Didn't link to the PR because I think the comment probably explains it better than this PR does thus far, but can tack a link on if you think it helps |
The comment helps a lot, this seems pretty sketch but I guess if it does what we need then that's just how it is. I can't say I understand a lot about rust backtraces, so... |
I suggest we go with this pr instead of #215, since this is only semver-minor, and treat eyre as a later change |
Going to merge this since it was approved and just enables the feature as documented, basically making this a bugfix |
This is the result of a yakshave to get backtraces on http-types errors, which were not previously functional as documented.
#[cfg(backtrace)]
does nothing by itself. Anyhow provides this flag to itself using a build script, so this PR adds a nearly-identical build script to http-types.It also correctly forwards Debug fmt to the inner error.