Skip to content

Commit

Permalink
Add ability to capture SpanTraces
Browse files Browse the repository at this point in the history
This was inspired by reading a blog post that showed some shortcomings
of thiserror compared to snafu in capturing backtraces.

The approach here captures a SpanTrace created by the tracing-error
crate. https://docs.rs/tracing-error/

Note: this is a fairly mechanical copy of the relevant backtrace methods
for discussion. I have not yet checked whether theres simplifications
that make sense. It also is problematic due to the unstable version of
the tracing crates, so it's doubtful that this should be merged as-is.

My goal with this PR is to start a conversation about how it might work.

Fixes #400
  • Loading branch information
joshka committed Dec 19, 2024
1 parent 2bd2982 commit 2982972
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 9 deletions.
4 changes: 4 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"rust-analyzer.cargo.features": ["span_trace"]
}

10 changes: 10 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@ default = ["std"]
#
# Without std, this would need to be written #[error("... {}", path.display())].
std = []
span_trace = ["dep:tracing-error"]

[dependencies]
thiserror-impl = { version = "=2.0.8", path = "impl" }
tracing-error = { version = "0.2.1", optional = true }

[dev-dependencies]
anyhow = "1.0.73"
ref-cast = "1.0.18"
rustversion = "1.0.13"
tracing = { version = "0.1.27" }
tracing-error = { version = "0.2.1" }
tracing-subscriber = { version = "0.3.19" }
trybuild = { version = "1.0.81", features = ["diff"] }

[workspace]
Expand All @@ -42,3 +47,8 @@ members = ["impl", "tests/no-std"]
[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
rustdoc-args = ["--generate-link-to-definition"]

[[example]]
name = "spantrace"
path = "examples/spantrace.rs"
required-features = ["span_trace"]
48 changes: 48 additions & 0 deletions examples/spantrace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use thiserror::Error;
use tracing_error::SpanTrace;
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};

fn main() {
tracing_subscriber::registry()
.with(tracing_subscriber::fmt::layer())
.with(tracing_error::ErrorLayer::default())
.init();

let result = boom();
match result {
Err(err) => {
eprintln!("error: {}", err);
match err {
Error::MyError(source, span_trace) => {
eprintln!("source: {}", source);
eprintln!("span trace: {:#?}", span_trace);
}
}
}
_ => unreachable!(),
}
}

#[tracing::instrument]
fn boom() -> Result<(), Error> {
inner_boom()?;
Ok(())
}

#[tracing::instrument]
fn inner_boom() -> Result<(), Error> {
non_span_trace()?;
Ok(())
}

#[tracing::instrument]
fn non_span_trace() -> std::io::Result<()> {
std::fs::read_to_string("nonexistent-file")?;
Ok(())
}

#[derive(Error, Debug)]
enum Error {
#[error("I/O error: {0}")]
MyError(#[from] std::io::Error, SpanTrace),
}
2 changes: 2 additions & 0 deletions impl/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub struct Attrs<'a> {
pub display: Option<Display<'a>>,
pub source: Option<Source<'a>>,
pub backtrace: Option<&'a Attribute>,
pub span_trace: Option<&'a Attribute>,
pub from: Option<From<'a>>,
pub transparent: Option<Transparent<'a>>,
pub fmt: Option<Fmt<'a>>,
Expand Down Expand Up @@ -71,6 +72,7 @@ pub fn get(input: &[Attribute]) -> Result<Attrs> {
display: None,
source: None,
backtrace: None,
span_trace: None,
from: None,
transparent: None,
fmt: None,
Expand Down
20 changes: 18 additions & 2 deletions impl/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,10 @@ fn impl_struct(input: Struct) -> TokenStream {
let from_impl = input.from_field().map(|from_field| {
let span = from_field.attrs.from.unwrap().span;
let backtrace_field = input.distinct_backtrace_field();
let span_trace_field = input.distinct_span_trace_field();
let from = unoptional_type(from_field.ty);
let source_var = Ident::new("source", span);
let body = from_initializer(from_field, backtrace_field, &source_var);
let body = from_initializer(from_field, backtrace_field, span_trace_field, &source_var);
let from_impl = quote_spanned! {span=>
#[automatically_derived]
impl #impl_generics ::core::convert::From<#from> for #ty #ty_generics #where_clause {
Expand Down Expand Up @@ -432,10 +433,11 @@ fn impl_enum(input: Enum) -> TokenStream {
let from_field = variant.from_field()?;
let span = from_field.attrs.from.unwrap().span;
let backtrace_field = variant.distinct_backtrace_field();
let span_trace_field = variant.distinct_span_trace_field();
let variant = &variant.ident;
let from = unoptional_type(from_field.ty);
let source_var = Ident::new("source", span);
let body = from_initializer(from_field, backtrace_field, &source_var);
let body = from_initializer(from_field, backtrace_field, span_trace_field, &source_var);
let from_impl = quote_spanned! {span=>
#[automatically_derived]
impl #impl_generics ::core::convert::From<#from> for #ty #ty_generics #where_clause {
Expand Down Expand Up @@ -505,6 +507,7 @@ fn use_as_display(needs_as_display: bool) -> Option<TokenStream> {
fn from_initializer(
from_field: &Field,
backtrace_field: Option<&Field>,
span_trace_field: Option<&Field>,
source_var: &Ident,
) -> TokenStream {
let from_member = &from_field.member;
Expand All @@ -525,9 +528,22 @@ fn from_initializer(
}
}
});
let spantrace = span_trace_field.map(|span_trace_field| {
let span_trace_member = &span_trace_field.member;
if type_is_option(span_trace_field.ty) {
quote! {
#span_trace_member: ::core::option::Option::Some(::thiserror::__private::SpanTrace::capture()),
}
} else {
quote! {
#span_trace_member: ::core::convert::From::from(::thiserror::__private::SpanTrace::capture()),
}
}
});
quote!({
#from_member: #some_source,
#backtrace
#spantrace
})
}

Expand Down
2 changes: 1 addition & 1 deletion impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ mod valid;
use proc_macro::TokenStream;
use syn::{parse_macro_input, DeriveInput};

#[proc_macro_derive(Error, attributes(backtrace, error, from, source))]
#[proc_macro_derive(Error, attributes(backtrace, error, from, source, spantrace))]
pub fn derive_error(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
expand::derive(&input).into()
Expand Down
60 changes: 60 additions & 0 deletions impl/src/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,19 @@ impl Struct<'_> {
backtrace_field(&self.fields)
}

pub(crate) fn span_trace_field(&self) -> Option<&Field> {
span_trace_field(&self.fields)
}

pub(crate) fn distinct_backtrace_field(&self) -> Option<&Field> {
let backtrace_field = self.backtrace_field()?;
distinct_backtrace_field(backtrace_field, self.from_field())
}

pub(crate) fn distinct_span_trace_field(&self) -> Option<&Field> {
let span_trace_field = self.span_trace_field()?;
distinct_span_trace_field(span_trace_field, self.from_field())
}
}

impl Enum<'_> {
Expand Down Expand Up @@ -63,17 +72,30 @@ impl Variant<'_> {
backtrace_field(&self.fields)
}

pub(crate) fn span_trace_field(&self) -> Option<&Field> {
span_trace_field(&self.fields)
}

pub(crate) fn distinct_backtrace_field(&self) -> Option<&Field> {
let backtrace_field = self.backtrace_field()?;
distinct_backtrace_field(backtrace_field, self.from_field())
}

pub(crate) fn distinct_span_trace_field(&self) -> Option<&Field> {
let span_trace_field = self.span_trace_field()?;
distinct_span_trace_field(span_trace_field, self.from_field())
}
}

impl Field<'_> {
pub(crate) fn is_backtrace(&self) -> bool {
type_is_backtrace(self.ty)
}

pub(crate) fn is_span_trace(&self) -> bool {
type_is_span_trace(self.ty)
}

pub(crate) fn source_span(&self) -> Span {
if let Some(source_attr) = &self.attrs.source {
source_attr.span
Expand Down Expand Up @@ -123,6 +145,20 @@ fn backtrace_field<'a, 'b>(fields: &'a [Field<'b>]) -> Option<&'a Field<'b>> {
None
}

fn span_trace_field<'a, 'b>(fields: &'a [Field<'b>]) -> Option<&'a Field<'b>> {
for field in fields {
if field.attrs.span_trace.is_some() {
return Some(field);
}
}
for field in fields {
if field.is_span_trace() {
return Some(field);
}
}
None
}

// The #[backtrace] field, if it is not the same as the #[from] field.
fn distinct_backtrace_field<'a, 'b>(
backtrace_field: &'a Field<'b>,
Expand All @@ -137,6 +173,20 @@ fn distinct_backtrace_field<'a, 'b>(
}
}

// The #[span_trace] field, if it is not the same as the #[from] field.
fn distinct_span_trace_field<'a, 'b>(
span_trace_field: &'a Field<'b>,
from_field: Option<&Field>,
) -> Option<&'a Field<'b>> {
if from_field.map_or(false, |from_field| {
from_field.member == span_trace_field.member
}) {
None
} else {
Some(span_trace_field)
}
}

fn type_is_backtrace(ty: &Type) -> bool {
let path = match ty {
Type::Path(ty) => &ty.path,
Expand All @@ -146,3 +196,13 @@ fn type_is_backtrace(ty: &Type) -> bool {
let last = path.segments.last().unwrap();
last.ident == "Backtrace" && last.arguments.is_empty()
}

fn type_is_span_trace(ty: &Type) -> bool {
let path = match ty {
Type::Path(ty) => &ty.path,
_ => return false,
};

let last = path.segments.last().unwrap();
last.ident == "SpanTrace" && last.arguments.is_empty()
}
27 changes: 22 additions & 5 deletions impl/src/valid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ fn check_field_attrs(fields: &[Field]) -> Result<()> {
let mut from_field = None;
let mut source_field = None;
let mut backtrace_field = None;
let mut span_trace_field = None;
let mut has_backtrace = false;
let mut has_span_trace = false;
for field in fields {
if let Some(from) = field.attrs.from {
if from_field.is_some() {
Expand Down Expand Up @@ -182,13 +184,24 @@ fn check_field_attrs(fields: &[Field]) -> Result<()> {
backtrace_field = Some(field);
has_backtrace = true;
}
if let Some(span_trace) = field.attrs.span_trace {
if span_trace_field.is_some() {
return Err(Error::new_spanned(
span_trace,
"duplicate #[span_trace] attribute",
));
}
span_trace_field = Some(field);
has_span_trace = true;
}
if let Some(transparent) = field.attrs.transparent {
return Err(Error::new_spanned(
transparent.original,
"#[error(transparent)] needs to go outside the enum or struct, not on an individual field",
));
}
has_backtrace |= field.is_backtrace();
has_span_trace |= field.is_span_trace();
}
if let (Some(from_field), Some(source_field)) = (from_field, source_field) {
if from_field.member != source_field.member {
Expand All @@ -199,14 +212,18 @@ fn check_field_attrs(fields: &[Field]) -> Result<()> {
}
}
if let Some(from_field) = from_field {
let max_expected_fields = match backtrace_field {
Some(backtrace_field) => 1 + (from_field.member != backtrace_field.member) as usize,
None => 1 + has_backtrace as usize,
let max_backtrace_fields = match backtrace_field {
Some(backtrace_field) => (from_field.member != backtrace_field.member) as usize,
None => has_backtrace as usize,
};
let max_span_trace_fields = match span_trace_field {
Some(span_trace_field) => (from_field.member != span_trace_field.member) as usize,
None => has_span_trace as usize,
};
if fields.len() > max_expected_fields {
if fields.len() > 1 + max_backtrace_fields + max_span_trace_fields {
return Err(Error::new_spanned(
from_field.attrs.from.unwrap().original,
"deriving From requires no fields other than source and backtrace",
"deriving From requires no fields other than source, backtrace, and span_trace",
));
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,4 +301,7 @@ pub mod __private {
#[cfg(all(feature = "std", not(thiserror_no_backtrace_type)))]
#[doc(hidden)]
pub use std::backtrace::Backtrace;
#[cfg(all(feature = "std", feature = "span_trace"))]
#[doc(hidden)]
pub use tracing_error::SpanTrace;
}
2 changes: 1 addition & 1 deletion tests/ui/from-backtrace-backtrace.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: deriving From requires no fields other than source and backtrace
error: deriving From requires no fields other than source, backtrace, and span_trace
--> tests/ui/from-backtrace-backtrace.rs:9:5
|
9 | #[from]
Expand Down

0 comments on commit 2982972

Please sign in to comment.