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

Fix compile probe and nightly backtraces #160

Merged
merged 6 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 16 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).
thenorili marked this conversation as resolved.
Show resolved Hide resolved

```rust
use thiserror::Error;
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per #121, some folks will continue to be unhappy about enforcing unstable features whenever we detect nightly. IIRC we discussed a while ago that we want to accomodate that use case. It doesn't make it any worse so I think it's fine to put that work off, but I thought it was worth mentioning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not wanting to break existing behavior in a "fix" PR, but rather make existing up to date nightly a tricky beast when it comes to libraries.


## Comparison to failure

The `eyre::Report` type works something like `failure::Error`, but unlike
Expand All @@ -195,8 +207,6 @@ you need an error type that can be handled via match or reported. This is
common in library crates where you don't know how your users will handle
your errors.

[thiserror]: https://github.com/dtolnay/thiserror

## Compatibility with `anyhow`

This crate does its best to be usable as a drop in replacement of `anyhow` and
Expand Down
4 changes: 4 additions & 0 deletions eyre/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@ 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"
thiserror = "1.0"
# TODO: 1.0.90 uses rustc-1.70
trybuild = { version = "=1.0.83", features = ["diff"] }
backtrace = "0.3.46"
anyhow = "1.0.28"
Expand Down
171 changes: 72 additions & 99 deletions eyre/build.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was your motivation for rewriting this file?

Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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::<MyBacktrace>(&self.backtrace);
}
}
"#;

const TRACK_CALLER_PROBE: &str = r#"
#![allow(dead_code)]

#[track_caller]
fn foo() {
let _location = std::panic::Location::caller();
}
"#;

fn main() {
match compile_probe(BACKTRACE_PROBE) {
Some(status) if status.success() => println!("cargo:rustc-cfg=backtrace"),
_ => {}
}

match compile_probe(TRACK_CALLER_PROBE) {
Some(status) if status.success() => println!("cargo:rustc-cfg=track_caller"),
_ => {}
}
fn compile_probe(probe: &str) -> Option<ExitStatus> {
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()?;

let version = match rustc_version_info() {
Some(version) => version,
None => return,
};
let rustc_wrapper = env::var_os("RUSTC_WRAPPER").filter(|wrapper| !wrapper.is_empty());
let rustc_workspace_wrapper =
env::var_os("RUSTC_WORKSPACE_WRAPPER").filter(|wrapper| !wrapper.is_empty());
let mut rustc = rustc_wrapper
.into_iter()
.chain(rustc_workspace_wrapper)
.chain(std::iter::once(rustc));

version.toolchain.set_feature();
let mut cmd = Command::new(rustc.next().unwrap());
cmd.args(rustc);

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<ExitStatus> {
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")
Expand All @@ -88,45 +103,3 @@ fn compile_probe(probe: &str) -> Option<ExitStatus> {
.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<VersionInfo> {
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)
}
22 changes: 18 additions & 4 deletions eyre/src/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<std::backtrace::Backtrace>($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!()
};
}
9 changes: 3 additions & 6 deletions eyre/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down Expand Up @@ -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)> {
Expand Down
5 changes: 5 additions & 0 deletions eyre/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,11 @@ impl<E> StdError for ErrorImpl<E>
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()
}
Expand Down
11 changes: 8 additions & 3 deletions eyre/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<dyn EyreHandler> {
// Capture the backtrace if the source error did not already capture one
let backtrace = backtrace_if_absent!(error);

Box::new(Self {
Expand Down Expand Up @@ -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::<Backtrace>(error))
.expect("backtrace capture failed");

if let BacktraceStatus::Captured = backtrace.status() {
write!(f, "\n\nStack backtrace:\n{}", backtrace)?;
}
Expand Down
6 changes: 3 additions & 3 deletions eyre/src/wrapper.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not your code / out of scope: We're getting a couple dead code errors in this file with DisplayError and NoneError. I'm surprised they're triggered even though we marked them public. Why don't we tag these structs as #[allow(dead_code)]?

I am similarly distressed by the other warnings we get a dozen copies of down below. I thought the missing_doc_code_examples thing was resolved ages ago.

Copy link

@RalfJung RalfJung May 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised they're triggered even though we marked them public.

pub in a private module can still be dead code.

pub(crate) is not even public, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I understand that. I guess my surprise is that it's existing code that's had attention paid to its visibility and yet it's dead. I'll find the last time it wasn't dead code and figure its story out.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use core::fmt::{self, Debug, Display};

#[repr(transparent)]
pub(crate) struct DisplayError<M>(pub(crate) M);

Check warning on line 5 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (beta, --no-default-features --features track-caller)

struct `DisplayError` is never constructed

Check warning on line 5 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (beta, --no-default-features)

struct `DisplayError` is never constructed

Check warning on line 5 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (beta, --no-default-features --features auto-install)

struct `DisplayError` is never constructed

Check warning on line 5 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (nightly, --no-default-features --features track-caller)

struct `DisplayError` is never constructed

Check warning on line 5 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (nightly, --no-default-features --features auto-install)

struct `DisplayError` is never constructed

Check warning on line 5 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (nightly, --no-default-features)

struct `DisplayError` is never constructed

#[repr(transparent)]
/// Wraps a Debug + Display type as an error.
Expand All @@ -10,7 +10,7 @@
/// Its Debug and Display impls are the same as the wrapped type.
pub(crate) struct MessageError<M>(pub(crate) M);

pub(crate) struct NoneError;

Check warning on line 13 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (beta, --no-default-features --features track-caller)

struct `NoneError` is never constructed

Check warning on line 13 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (beta, --no-default-features)

struct `NoneError` is never constructed

Check warning on line 13 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (beta, --no-default-features --features auto-install)

struct `NoneError` is never constructed

Check warning on line 13 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (nightly, --no-default-features --features track-caller)

struct `NoneError` is never constructed

Check warning on line 13 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (nightly, --no-default-features --features auto-install)

struct `NoneError` is never constructed

Check warning on line 13 in eyre/src/wrapper.rs

View workflow job for this annotation

GitHub Actions / Test Suite (nightly, --no-default-features)

struct `NoneError` is never constructed

impl<M> Debug for DisplayError<M>
where
Expand Down Expand Up @@ -82,9 +82,9 @@
}

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)> {
Expand Down
Loading
Loading