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

enable error backtraces #212

Merged
merged 4 commits into from
Aug 12, 2020
Merged

enable error backtraces #212

merged 4 commits into from
Aug 12, 2020

Conversation

jbr
Copy link
Member

@jbr jbr commented Aug 4, 2020

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.

@jbr
Copy link
Member Author

jbr commented Aug 4, 2020

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());
}

env RUST_LIB_BACKTRACE=1 cargo run --example 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 },
]

@jbr jbr requested review from yoshuawuyts and Fishrock123 August 4, 2020 23:19
@jbr jbr force-pushed the enable-error-backtraces branch from 8f92490 to 9446815 Compare August 5, 2020 00:55
@jbr
Copy link
Member Author

jbr commented Aug 5, 2020

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:

$ RUST_LIB_BACKTRACE=0 cargo +nightly run --example backtrace
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/examples/backtrace`
[examples/backtrace.rs:2] http_types::format_err!("error").backtrace() = None
$ RUST_LIB_BACKTRACE=1 cargo +nightly run --example backtrace
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/examples/backtrace`
[examples/backtrace.rs:2] http_types::format_err!("error").backtrace() = Some(
    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 },
    ],
)
$ cargo +stable run --example backtrace # stable ignores rust_lib_backtrace
    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/examples/backtrace`
[examples/backtrace.rs:2] http_types::format_err!("error").backtrace() = None

Copy link
Member

@yoshuawuyts yoshuawuyts left a 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 😅

@jbr
Copy link
Member Author

jbr commented Aug 5, 2020

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

@Fishrock123
Copy link
Member

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...

@jbr jbr mentioned this pull request Aug 6, 2020
@goto-bus-stop goto-bus-stop changed the base branch from master to main August 9, 2020 12:58
@jbr
Copy link
Member Author

jbr commented Aug 12, 2020

I suggest we go with this pr instead of #215, since this is only semver-minor, and treat eyre as a later change

@jbr
Copy link
Member Author

jbr commented Aug 12, 2020

Going to merge this since it was approved and just enables the feature as documented, basically making this a bugfix

@jbr jbr merged commit 6a263e1 into http-rs:main Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants