Skip to content

Commit

Permalink
subscriber::fmt: print all error sources (#1460)
Browse files Browse the repository at this point in the history
## Motivation

Fixes #1347  

## Solution

Change the format from `err=message, err: source1` to `err=message
err.sources=[source1, source2, source3, ...]`, as suggested in
#1347 (comment)
(still leaving out the sources if there are none). 

## Caveats

Haven't changed the JSON formatter, since I'm not really sure about how
to do that. The current format is `{.., "fields": {.., "err":
"message"}}`, disregarding the sources entirely. 

We could change that to `{.., "fields": {.., "err": "message",
"err.sources": ["source1", "source2", "source3", ..]}}`, which would
keep backwards compatibility but looks pretty ugly.

Another option would be `{.., "fields": {.., "err": {"message":
"message", "sources": ["source1", "source2", "source3", ..]}}}` which
leaves room for future expansion.

Then again, that begs the question about why the first error is special,
so maybe it ought to be `{.., "fields": {.., "err": {"message":
"message", "source": {"message": "source1", "source": ..}}}}`.

But that style of linked list is pretty annoying to parse, so maybe it
ought to be flattened to `{.., "fields": {.., "err": [{"message":
"message"}, {"message": "source1"}, ..]}}`?

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
nightkr and hawkw authored Aug 6, 2021
1 parent 5f0f5cc commit aeae4fa
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 8 deletions.
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
struct ErrorSourceList<'a>(&'a (dyn std::error::Error + 'static));

impl<'a> Display for ErrorSourceList<'a> {
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

0 comments on commit aeae4fa

Please sign in to comment.