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

subscriber::fmt: Print all error sources #1460

Merged
merged 10 commits into from
Aug 6, 2021
4 changes: 4 additions & 0 deletions examples/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,7 @@ tempdir = "0.3.7"
# opentelemetry example
opentelemetry = { version = "0.15", default-features = false, features = ["trace"] }
opentelemetry-jaeger = "0.14"

# fmt examples
snafu = "0.6.10"
thiserror = "1.0.26"
33 changes: 28 additions & 5 deletions examples/examples/fmt/yak_shave.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::{error::Error, io};
use snafu::{ResultExt, Snafu};
use std::error::Error;
use thiserror::Error;
use tracing::{debug, error, info, span, trace, warn, Level};

// the `#[tracing::instrument]` attribute creates and enters a span
Expand All @@ -14,10 +16,11 @@ pub fn shave(yak: usize) -> Result<(), Box<dyn Error + 'static>> {
trace!(excitement = "yay!", "hello! I'm gonna shave a yak");
if yak == 3 {
warn!("could not locate yak");
// note that this is intended to demonstrate `tracing`'s features, not idiomatic
// error handling! in a library or application, you should consider returning
// a dedicated `YakError`. libraries like snafu or thiserror make this easy.
return Err(io::Error::new(io::ErrorKind::Other, "missing yak").into());
return OutOfCash
.fail()
.map_err(|source| MissingYakError::OutOfSpace { source })
.context(MissingYak)
.map_err(|err| err.into());
} else {
trace!("yak shaved successfully");
}
Expand Down Expand Up @@ -54,3 +57,23 @@ pub fn shave_all(yaks: usize) -> usize {

yaks_shaved
}

// Error types
// Usually you would pick one error handling library to use, but they can be mixed freely
#[derive(Debug, Snafu)]
enum OutOfSpaceError {
#[snafu(display("out of cash"))]
OutOfCash,
}

#[derive(Debug, Error)]
enum MissingYakError {
#[error("out of space")]
OutOfSpace { source: OutOfSpaceError },
}

#[derive(Debug, Snafu)]
enum YakError {
#[snafu(display("missing yak"))]
MissingYak { source: MissingYakError },
}
20 changes: 19 additions & 1 deletion tracing-subscriber/src/fmt/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,10 @@ impl<'a> field::Visit for DefaultVisitor<'a> {

fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) {
if let Some(source) = value.source() {
self.record_debug(field, &format_args!("{}, {}: {}", value, field, source))
self.record_debug(
field,
&format_args!("{} {}.sources={}", value, field, ErrorSourceList(source)),
)
} else {
self.record_debug(field, &format_args!("{}", value))
}
Expand Down Expand Up @@ -806,6 +809,21 @@ impl<'a> fmt::Debug for DefaultVisitor<'a> {
}
}

/// Renders an error into a list of sources, *including* the error
hawkw marked this conversation as resolved.
Show resolved Hide resolved
struct ErrorSourceList<'a>(&'a (dyn std::error::Error + 'static));

impl<'a> Display for ErrorSourceList<'a> {
Comment on lines +813 to +815
Copy link
Member

Choose a reason for hiding this comment

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

another thought i had, but again, take it or leave it: i wonder if a nicer way to factor this would be to have this type just implement Iterator, analogous to the Error::chain unstable API. then, we could have the default formatter use f.debug_list().entries(ErrorSourceList(&error)).finish(), but the Pretty formatter could do something arbitrarily fancy with the iterator, like delimiting the errors with indentation...and eventually, we could also use the iterator in the JSON formatter to serialize the error chain as a JSON list.

what do you think? i'd be fine with merging this as-is, and making that change later if and when it's necessary, but it seems like it might be a somewhat nicer way to factor this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

then, we could have the default formatter

The Display of ErrorSourceList?

fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut list = f.debug_list();
let mut curr = Some(self.0);
while let Some(curr_err) = curr {
list.entry(&format_args!("{}", curr_err));
curr = curr_err.source();
}
list.finish()
}
}

struct FmtCtx<'a, S, N> {
ctx: &'a FmtContext<'a, S, N>,
span: Option<&'a span::Id>,
Expand Down
4 changes: 2 additions & 2 deletions tracing-subscriber/src/fmt/format/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,12 @@ impl<'a> field::Visit for PrettyVisitor<'a> {
self.record_debug(
field,
&format_args!(
"{}, {}{}.source{}: {}",
"{}, {}{}.sources{}: {}",
value,
bold.prefix(),
field,
bold.infix(self.style),
source,
ErrorSourceList(source),
),
)
} else {
Expand Down