From 29829724efa81599a3ea6445d03682d5f12529a7 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Thu, 19 Dec 2024 00:05:51 -0800 Subject: [PATCH] Add ability to capture `SpanTrace`s 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 --- .vscode/settings.json | 4 ++ Cargo.toml | 10 ++++ examples/spantrace.rs | 48 +++++++++++++++++++ impl/src/attr.rs | 2 + impl/src/expand.rs | 20 +++++++- impl/src/lib.rs | 2 +- impl/src/prop.rs | 60 ++++++++++++++++++++++++ impl/src/valid.rs | 27 +++++++++-- src/lib.rs | 3 ++ tests/ui/from-backtrace-backtrace.stderr | 2 +- 10 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 examples/spantrace.rs diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..ddc0c237 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,4 @@ +{ + "rust-analyzer.cargo.features": ["span_trace"] +} + diff --git a/Cargo.toml b/Cargo.toml index 578608b2..e1cf95f9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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] @@ -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"] diff --git a/examples/spantrace.rs b/examples/spantrace.rs new file mode 100644 index 00000000..43ccfc7f --- /dev/null +++ b/examples/spantrace.rs @@ -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), +} diff --git a/impl/src/attr.rs b/impl/src/attr.rs index 7ad83e02..a08c4d9f 100644 --- a/impl/src/attr.rs +++ b/impl/src/attr.rs @@ -12,6 +12,7 @@ pub struct Attrs<'a> { pub display: Option>, pub source: Option>, pub backtrace: Option<&'a Attribute>, + pub span_trace: Option<&'a Attribute>, pub from: Option>, pub transparent: Option>, pub fmt: Option>, @@ -71,6 +72,7 @@ pub fn get(input: &[Attribute]) -> Result { display: None, source: None, backtrace: None, + span_trace: None, from: None, transparent: None, fmt: None, diff --git a/impl/src/expand.rs b/impl/src/expand.rs index fdda8512..42e1df5e 100644 --- a/impl/src/expand.rs +++ b/impl/src/expand.rs @@ -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 { @@ -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 { @@ -505,6 +507,7 @@ fn use_as_display(needs_as_display: bool) -> Option { 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; @@ -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 }) } diff --git a/impl/src/lib.rs b/impl/src/lib.rs index 7f3767ef..5bcbcade 100644 --- a/impl/src/lib.rs +++ b/impl/src/lib.rs @@ -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() diff --git a/impl/src/prop.rs b/impl/src/prop.rs index 0a101fc0..e84f6406 100644 --- a/impl/src/prop.rs +++ b/impl/src/prop.rs @@ -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<'_> { @@ -63,10 +72,19 @@ 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<'_> { @@ -74,6 +92,10 @@ impl Field<'_> { 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 @@ -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>, @@ -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, @@ -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() +} diff --git a/impl/src/valid.rs b/impl/src/valid.rs index 21bd885e..27828d37 100644 --- a/impl/src/valid.rs +++ b/impl/src/valid.rs @@ -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() { @@ -182,6 +184,16 @@ 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, @@ -189,6 +201,7 @@ fn check_field_attrs(fields: &[Field]) -> Result<()> { )); } 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 { @@ -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", )); } } diff --git a/src/lib.rs b/src/lib.rs index 579865af..17c25259 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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; } diff --git a/tests/ui/from-backtrace-backtrace.stderr b/tests/ui/from-backtrace-backtrace.stderr index 5c0b9a3b..866edc00 100644 --- a/tests/ui/from-backtrace-backtrace.stderr +++ b/tests/ui/from-backtrace-backtrace.stderr @@ -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]