From 4881cd24f96f452e58180435c61c0b3090d977bb Mon Sep 17 00:00:00 2001 From: Freja Roberts Date: Tue, 26 Mar 2024 13:13:45 +0100 Subject: [PATCH] fix: compile probe and nightly backtraces This addresses the invalid compile probe testing for a non-longer existing feature by updating it and eyre to use the `error_generic_member_access` features instead. The report and errors have all been updated to accomodate this and the new backtrace provide API Fixes: #84 and #97 --- Cargo.toml | 1 + README.md | 20 +++- eyre/Cargo.toml | 3 + eyre/build.rs | 167 ++++++++++++---------------- eyre/src/backtrace.rs | 22 +++- eyre/src/context.rs | 9 +- eyre/src/error.rs | 5 + eyre/src/lib.rs | 11 +- eyre/src/wrapper.rs | 6 +- eyre/tests/generic_member_access.rs | 67 +++++++++++ eyre/tests/test_toolchain.rs | 34 ------ 11 files changed, 194 insertions(+), 151 deletions(-) create mode 100644 eyre/tests/generic_member_access.rs delete mode 100644 eyre/tests/test_toolchain.rs diff --git a/Cargo.toml b/Cargo.toml index b58a509..125e065 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ rust-version = "1.65.0" indenter = "0.3.0" once_cell = "1.18.0" owo-colors = "3.2.0" +autocfg = "1.0" [profile.dev.package.backtrace] opt-level = 3 diff --git a/README.md b/README.md index 31e25a9..a0a99fb 100644 --- a/README.md +++ b/README.md @@ -122,9 +122,12 @@ avoid using `eyre::Report` as your public error type. } ``` -- If using the nightly channel, a backtrace is captured and printed with the - error if the underlying error type does not already provide its own. In order - to see backtraces, they must be enabled through the environment variables +- If using rust >1.65, a backtrace is captured and printed with the + error. + + On nightly eyre will use the underlying error's backtrace if it has one. + + In order to see backtraces, they must be enabled through the environment variables described in [`std::backtrace`]: - If you want panics and errors to both have backtraces, set @@ -141,7 +144,7 @@ avoid using `eyre::Report` as your public error type. - Eyre works with any error type that has an impl of `std::error::Error`, including ones defined in your crate. We do not bundle a `derive(Error)` macro but you can write the impls yourself or use a standalone macro like - [thiserror]. + [thiserror](https://github.com/dtolnay/thiserror). ```rust use thiserror::Error; @@ -178,6 +181,15 @@ No-std support was removed in 2020 in [commit 608a16a] due to unaddressed upstre [commit 608a16a]: https://github.com/eyre-rs/eyre/pull/29/commits/608a16aa2c2c27eca6c88001cc94c6973c18f1d5 + +## Backtrace support + +The built in default handler has support for capturing backtrace using `rustc-1.65` or later. + +Backtraces are captured when an error is converted to an `eyre::Report` (such as using `?` or `eyre!`). + +If using the nightly toolchain, backtraces will also be captured and accessed from other errors using [error_generic_member_access](https://github.com/rust-lang/rfcs/pull/2895) if available. + ## Comparison to failure The `eyre::Report` type works something like `failure::Error`, but unlike diff --git a/eyre/Cargo.toml b/eyre/Cargo.toml index 1085cbb..36b38ee 100644 --- a/eyre/Cargo.toml +++ b/eyre/Cargo.toml @@ -23,6 +23,9 @@ indenter = { workspace = true } once_cell = { workspace = true } pyo3 = { version = "0.20", optional = true, default-features = false } +[build-dependencies] +autocfg = { workspace = true } + [dev-dependencies] futures = { version = "0.3", default-features = false } rustversion = "1.0" diff --git a/eyre/build.rs b/eyre/build.rs index e0b2d29..ab36c2a 100644 --- a/eyre/build.rs +++ b/eyre/build.rs @@ -1,23 +1,53 @@ -use std::env; -use std::ffi::OsString; -use std::fs; -use std::path::Path; -use std::process::{Command, ExitStatus}; -use std::str; - -// This code exercises the surface area that we expect of the std Backtrace -// type. If the current toolchain is able to compile it, we go ahead and use -// backtrace in eyre. -const BACKTRACE_PROBE: &str = r#" - #![feature(backtrace)] +use std::{ + env, fs, + path::Path, + process::{Command, ExitStatus}, +}; + +fn main() { + let ac = autocfg::new(); + + // https://github.com/rust-lang/rust/issues/99301 [nightly] + // + // Autocfg does currently not support custom probes, or `nightly` only features + match compile_probe(GENERIC_MEMBER_ACCESS_PROBE) { + Some(status) if status.success() => autocfg::emit("generic_member_access"), + _ => {} + } + + // https://github.com/rust-lang/rust/issues/47809 [rustc-1.46] + ac.emit_expression_cfg("std::panic::Location::caller", "track_caller"); + + if ac.probe_rustc_version(1, 52) { + autocfg::emit("eyre_no_fmt_arguments_as_str"); + } + + if ac.probe_rustc_version(1, 58) { + autocfg::emit("eyre_no_fmt_args_capture"); + } + + if ac.probe_rustc_version(1, 65) { + autocfg::emit("backtrace") + } +} + +// This code exercises the surface area or the generic member access feature for the `std::error::Error` trait. +// +// This is use to detect and supply backtrace information through different errors types. +const GENERIC_MEMBER_ACCESS_PROBE: &str = r#" + #![feature(error_generic_member_access)] #![allow(dead_code)] - use std::backtrace::{Backtrace, BacktraceStatus}; - use std::error::Error; + use std::error::{Error, Request}; use std::fmt::{self, Display}; #[derive(Debug)] - struct E; + struct E { + backtrace: MyBacktrace, + } + + #[derive(Debug)] + struct MyBacktrace; impl Display for E { fn fmt(&self, _formatter: &mut fmt::Formatter) -> fmt::Result { @@ -26,59 +56,44 @@ const BACKTRACE_PROBE: &str = r#" } impl Error for E { - fn backtrace(&self) -> Option<&Backtrace> { - let backtrace = Backtrace::capture(); - match backtrace.status() { - BacktraceStatus::Captured | BacktraceStatus::Disabled | _ => {} - } - unimplemented!() + fn provide<'a>(&'a self, request: &mut Request<'a>) { + request + .provide_ref::(&self.backtrace); } } "#; -const TRACK_CALLER_PROBE: &str = r#" - #![allow(dead_code)] - - #[track_caller] - fn foo() { - let _location = std::panic::Location::caller(); - } -"#; +fn compile_probe(probe: &str) -> Option { + let rustc = env::var_os("RUSTC")?; + let out_dir = env::var_os("OUT_DIR")?; + let probefile = Path::new(&out_dir).join("probe.rs"); + fs::write(&probefile, probe).ok()?; -fn main() { - match compile_probe(BACKTRACE_PROBE) { - Some(status) if status.success() => println!("cargo:rustc-cfg=backtrace"), - _ => {} - } + // Supports invoking rustc thrugh a wrapper + let mut cmd = if let Some(wrapper) = env::var_os("RUSTC_WRAPPER") { + let mut cmd = Command::new(wrapper); - match compile_probe(TRACK_CALLER_PROBE) { - Some(status) if status.success() => println!("cargo:rustc-cfg=track_caller"), - _ => {} - } + cmd.arg(rustc); - let version = match rustc_version_info() { - Some(version) => version, - None => return, + cmd + } else { + Command::new(rustc) }; - version.toolchain.set_feature(); - - if version.minor < 52 { - println!("cargo:rustc-cfg=eyre_no_fmt_arguments_as_str"); + if let Some(target) = env::var_os("TARGET") { + cmd.arg("--target").arg(target); } - if version.minor < 58 { - println!("cargo:rustc-cfg=eyre_no_fmt_args_capture"); + // If Cargo wants to set RUSTFLAGS, use that. + if let Ok(rustflags) = env::var("CARGO_ENCODED_RUSTFLAGS") { + if !rustflags.is_empty() { + for arg in rustflags.split('\x1f') { + cmd.arg(arg); + } + } } -} -fn compile_probe(probe: &str) -> Option { - let rustc = env::var_os("RUSTC")?; - let out_dir = env::var_os("OUT_DIR")?; - let probefile = Path::new(&out_dir).join("probe.rs"); - fs::write(&probefile, probe).ok()?; - Command::new(rustc) - .arg("--edition=2018") + cmd.arg("--edition=2018") .arg("--crate-name=eyre_build") .arg("--crate-type=lib") .arg("--emit=metadata") @@ -88,45 +103,3 @@ fn compile_probe(probe: &str) -> Option { .status() .ok() } - -// TODO factor this toolchain parsing and related tests into its own file -#[derive(PartialEq)] -enum Toolchain { - Stable, - Beta, - Nightly, -} -impl Toolchain { - fn set_feature(self) { - match self { - Toolchain::Nightly => println!("cargo:rustc-cfg=nightly"), - Toolchain::Beta => println!("cargo:rustc-cfg=beta"), - Toolchain::Stable => println!("cargo:rustc-cfg=stable"), - } - } -} - -struct VersionInfo { - minor: u32, - toolchain: Toolchain, -} - -fn rustc_version_info() -> Option { - let rustc = env::var_os("RUSTC").unwrap_or_else(|| OsString::from("rustc")); - let output = Command::new(rustc).arg("--version").output().ok()?; - let version = str::from_utf8(&output.stdout).ok()?; - let mut pieces = version.split(['.', ' ', '-']); - if pieces.next() != Some("rustc") { - return None; - } - let _major: u32 = pieces.next()?.parse().ok()?; - let minor = pieces.next()?.parse().ok()?; - let _patch: u32 = pieces.next()?.parse().ok()?; - let toolchain = match pieces.next() { - Some("beta") => Toolchain::Beta, - Some("nightly") => Toolchain::Nightly, - _ => Toolchain::Stable, - }; - let version = VersionInfo { minor, toolchain }; - Some(version) -} diff --git a/eyre/src/backtrace.rs b/eyre/src/backtrace.rs index 6c00d7f..b1b378b 100644 --- a/eyre/src/backtrace.rs +++ b/eyre/src/backtrace.rs @@ -5,18 +5,32 @@ pub(crate) use std::backtrace::Backtrace; pub(crate) enum Backtrace {} #[cfg(backtrace)] +macro_rules! capture_backtrace { + () => { + Some(Backtrace::capture()) + }; +} + +#[cfg(not(backtrace))] +macro_rules! capture_backtrace { + () => { + None + }; +} +/// Capture a backtrace iff there is not already a backtrace in the error chain +#[cfg(generic_member_access)] macro_rules! backtrace_if_absent { ($err:expr) => { - match $err.backtrace() { + match std::error::request_ref::($err as &dyn std::error::Error) { Some(_) => None, - None => Some(Backtrace::capture()), + None => capture_backtrace!(), } }; } -#[cfg(not(backtrace))] +#[cfg(not(generic_member_access))] macro_rules! backtrace_if_absent { ($err:expr) => { - None + capture_backtrace!() }; } diff --git a/eyre/src/context.rs b/eyre/src/context.rs index aaba5bd..d3b6820 100644 --- a/eyre/src/context.rs +++ b/eyre/src/context.rs @@ -2,9 +2,6 @@ use crate::error::{ContextError, ErrorImpl}; use crate::{Report, StdError, WrapErr}; use core::fmt::{self, Debug, Display, Write}; -#[cfg(backtrace)] -use std::backtrace::Backtrace; - mod ext { use super::*; @@ -146,9 +143,9 @@ where D: Display, E: StdError + 'static, { - #[cfg(backtrace)] - fn backtrace(&self) -> Option<&Backtrace> { - self.error.backtrace() + #[cfg(generic_member_access)] + fn provide<'a>(&'a self, request: &mut std::error::Request<'a>) { + self.error.provide(request); } fn source(&self) -> Option<&(dyn StdError + 'static)> { diff --git a/eyre/src/error.rs b/eyre/src/error.rs index d6e3d5d..eed6cfe 100644 --- a/eyre/src/error.rs +++ b/eyre/src/error.rs @@ -855,6 +855,11 @@ impl StdError for ErrorImpl where E: StdError, { + #[cfg(generic_member_access)] + fn provide<'a>(&'a self, request: &mut std::error::Request<'a>) { + self._object.provide(request) + } + fn source(&self) -> Option<&(dyn StdError + 'static)> { ErrorImpl::<()>::error(self.erase()).source() } diff --git a/eyre/src/lib.rs b/eyre/src/lib.rs index 33c60d3..5f8bc94 100644 --- a/eyre/src/lib.rs +++ b/eyre/src/lib.rs @@ -355,7 +355,7 @@ unused_parens, while_true )] -#![cfg_attr(backtrace, feature(backtrace))] +#![cfg_attr(generic_member_access, feature(error_generic_member_access))] #![cfg_attr(doc_cfg, feature(doc_cfg))] #![allow( clippy::needless_doctest_main, @@ -778,6 +778,7 @@ impl DefaultHandler { #[allow(unused_variables)] #[cfg_attr(not(feature = "auto-install"), allow(dead_code))] pub fn default_with(error: &(dyn StdError + 'static)) -> Box { + // Capture the backtrace if the source error did not already capture one let backtrace = backtrace_if_absent!(error); Box::new(Self { @@ -837,15 +838,19 @@ impl EyreHandler for DefaultHandler { } } - #[cfg(backtrace)] + #[cfg(generic_member_access)] { use std::backtrace::BacktraceStatus; + // The backtrace can be stored either in the handler instance, or the error itself. + // + // If the source error has a backtrace, the handler should not capture one let backtrace = self .backtrace .as_ref() - .or_else(|| error.backtrace()) + .or_else(|| std::error::request_ref::(error)) .expect("backtrace capture failed"); + if let BacktraceStatus::Captured = backtrace.status() { write!(f, "\n\nStack backtrace:\n{}", backtrace)?; } diff --git a/eyre/src/wrapper.rs b/eyre/src/wrapper.rs index b4da68a..40dc01b 100644 --- a/eyre/src/wrapper.rs +++ b/eyre/src/wrapper.rs @@ -82,9 +82,9 @@ impl Display for BoxedError { } impl StdError for BoxedError { - #[cfg(backtrace)] - fn backtrace(&self) -> Option<&crate::backtrace::Backtrace> { - self.0.backtrace() + #[cfg(generic_member_access)] + fn provide<'a>(&'a self, request: &mut std::error::Request<'a>) { + self.0.provide(request); } fn source(&self) -> Option<&(dyn StdError + 'static)> { diff --git a/eyre/tests/generic_member_access.rs b/eyre/tests/generic_member_access.rs new file mode 100644 index 0000000..23d7753 --- /dev/null +++ b/eyre/tests/generic_member_access.rs @@ -0,0 +1,67 @@ +#![cfg_attr(generic_member_access, feature(error_generic_member_access))] + +#[cfg(generic_member_access)] +#[test] +/// Tests that generic member access works through an `eyre::Report` +fn generic_member_access() { + use eyre::WrapErr; + use std::backtrace::Backtrace; + use std::fmt::Display; + + fn fail() -> Result<(), MyError> { + Err(MyError { + cupcake: MyCupcake("Blueberry".into()), + backtrace: std::backtrace::Backtrace::capture(), + }) + } + + std::env::set_var("RUST_BACKTRACE", "1"); + + #[derive(Debug, PartialEq)] + struct MyCupcake(String); + + #[derive(Debug)] + struct MyError { + cupcake: MyCupcake, + backtrace: std::backtrace::Backtrace, + } + + impl Display for MyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Error: {}", self.cupcake.0) + } + } + + impl std::error::Error for MyError { + fn provide<'a>(&'a self, request: &mut std::error::Request<'a>) { + request + .provide_ref(&self.cupcake) + .provide_ref(&self.backtrace); + } + } + + let err = fail() + .wrap_err("Failed to bake my favorite cupcake") + .unwrap_err(); + + let err: Box = err.into(); + + assert!( + format!("{:?}", err).contains("generic_member_access::generic_member_access::fail"), + "should contain the source error backtrace" + ); + + assert_eq!( + std::error::request_ref::(&*err), + Some(&MyCupcake("Blueberry".into())) + ); + + let bt = std::error::request_ref::(&*err).unwrap(); + + assert!( + bt.to_string() + .contains("generic_member_access::generic_member_access::fail"), + "should contain the fail method as it was captured by the original error\n\n{}", + bt + ); +} diff --git a/eyre/tests/test_toolchain.rs b/eyre/tests/test_toolchain.rs deleted file mode 100644 index 0309398..0000000 --- a/eyre/tests/test_toolchain.rs +++ /dev/null @@ -1,34 +0,0 @@ -// These tests check our build script against rustversion. - -#[rustversion::attr(not(nightly), ignore)] -#[test] -fn nightlytest() { - if !cfg!(nightly) { - panic!("nightly feature isn't set when the toolchain is nightly."); - } - if cfg!(any(beta, stable)) { - panic!("beta, stable, and nightly are mutually exclusive features.") - } -} - -#[rustversion::attr(not(beta), ignore)] -#[test] -fn betatest() { - if !cfg!(beta) { - panic!("beta feature is not set when the toolchain is beta."); - } - if cfg!(any(nightly, stable)) { - panic!("beta, stable, and nightly are mutually exclusive features.") - } -} - -#[rustversion::attr(not(stable), ignore)] -#[test] -fn stabletest() { - if !cfg!(stable) { - panic!("stable feature is not set when the toolchain is stable."); - } - if cfg!(any(nightly, beta)) { - panic!("beta, stable, and nightly are mutually exclusive features.") - } -}