Skip to content

Commit

Permalink
Don't create Option<Backtrace> when the source error provides a `Ba…
Browse files Browse the repository at this point in the history
…cktrace`
  • Loading branch information
shepmaster committed Oct 9, 2022
1 parent 0ce4e1a commit 791b50a
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 18 deletions.
4 changes: 4 additions & 0 deletions .cirrus.yml
Expand Up @@ -145,6 +145,10 @@ nightly_test_task:
- cd compatibility-tests/report-try-trait/
- rustc --version
- cargo test
report_provider_api_test_script:
- cd compatibility-tests/backtrace-provider-api/
- rustc --version
- cargo test
before_cache_script: rm -rf $CARGO_HOME/registry/index

unstable_std_backtraces_test_task:
Expand Down
9 changes: 9 additions & 0 deletions compatibility-tests/backtrace-provider-api/Cargo.toml
@@ -0,0 +1,9 @@
[package]
name = "backtrace-provider-api"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
snafu = { path = "../..", features = ["unstable-provider-api"] }
1 change: 1 addition & 0 deletions compatibility-tests/backtrace-provider-api/rust-toolchain
@@ -0,0 +1 @@
nightly
44 changes: 44 additions & 0 deletions compatibility-tests/backtrace-provider-api/src/lib.rs
@@ -0,0 +1,44 @@
#![cfg(test)]
#![feature(error_generic_member_access, provide_any)]

use snafu::{prelude::*, IntoError};

#[test]
fn does_not_capture_a_backtrace_when_source_provides_a_backtrace() {
#[derive(Debug, Snafu)]
struct InnerError {
backtrace: snafu::Backtrace,
}

#[derive(Debug, Snafu)]
struct OuterError {
source: InnerError,
backtrace: Option<snafu::Backtrace>,
}

enable_backtrace_capture();
let e = OuterSnafu.into_error(InnerSnafu.build());

assert!(e.backtrace.is_none());
}

#[test]
fn does_capture_a_backtrace_when_source_does_not_provide_a_backtrace() {
#[derive(Debug, Snafu)]
struct InnerError;

#[derive(Debug, Snafu)]
struct OuterError {
source: InnerError,
backtrace: Option<snafu::Backtrace>,
}

enable_backtrace_capture();
let e = OuterSnafu.into_error(InnerSnafu.build());

assert!(e.backtrace.is_some());
}

fn enable_backtrace_capture() {
std::env::set_var("RUST_LIB_BACKTRACE", "1");
}
74 changes: 56 additions & 18 deletions src/lib.rs
Expand Up @@ -1165,32 +1165,46 @@ pub trait AsBacktrace {
/// This value will be tested only once per program execution;
/// changing the environment variable after it has been checked will
/// have no effect.
///
/// ## Interaction with the Provider API
///
/// If you enable the [`unstable-provider-api` feature
/// flag][provider-ff], a backtrace will not be captured if the
/// original error is able to provide a `Backtrace`, even if the
/// appropriate environment variables are set. This prevents capturing
/// a redundant backtrace.
///
/// [provider-ff]: crate::guide::feature_flags#unstable-provider-api
#[cfg(any(feature = "std", test))]
impl GenerateImplicitData for Option<Backtrace> {
fn generate() -> Self {
use std::env;
use std::sync::{
atomic::{AtomicBool, Ordering},
Once,
};

static START: Once = Once::new();
static ENABLED: AtomicBool = AtomicBool::new(false);

START.call_once(|| {
// TODO: What values count as "true"?
let enabled = env::var_os("RUST_LIB_BACKTRACE")
.or_else(|| env::var_os("RUST_BACKTRACE"))
.map_or(false, |v| v == "1");
ENABLED.store(enabled, Ordering::SeqCst);
});

if ENABLED.load(Ordering::SeqCst) {
if backtrace_collection_enabled() {
Some(Backtrace::generate())
} else {
None
}
}

fn generate_with_source(source: &dyn crate::Error) -> Self {
#[cfg(feature = "unstable-provider-api")]
{
use core::any;

if !backtrace_collection_enabled() {
None
} else if any::request_ref::<Backtrace>(source).is_some() {
None
} else {
Some(Backtrace::generate_with_source(source))
}
}

#[cfg(not(feature = "unstable-provider-api"))]
{
let _source = source;
Self::generate()
}
}
}

#[cfg(any(feature = "std", test))]
Expand All @@ -1200,6 +1214,30 @@ impl AsBacktrace for Option<Backtrace> {
}
}

#[cfg(any(feature = "std", test))]
fn backtrace_collection_enabled() -> bool {
use std::{
env,
sync::{
atomic::{AtomicBool, Ordering},
Once,
},
};

static START: Once = Once::new();
static ENABLED: AtomicBool = AtomicBool::new(false);

START.call_once(|| {
// TODO: What values count as "true"?
let enabled = env::var_os("RUST_LIB_BACKTRACE")
.or_else(|| env::var_os("RUST_BACKTRACE"))
.map_or(false, |v| v == "1");
ENABLED.store(enabled, Ordering::SeqCst);
});

ENABLED.load(Ordering::SeqCst)
}

#[cfg(feature = "backtraces-impl-backtrace-crate")]
impl GenerateImplicitData for Backtrace {
fn generate() -> Self {
Expand Down

0 comments on commit 791b50a

Please sign in to comment.