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

Value implementation for dyn Error + 'static prints first cause only #1347

Closed
mbr opened this issue Apr 4, 2021 · 1 comment
Closed

Value implementation for dyn Error + 'static prints first cause only #1347

mbr opened this issue Apr 4, 2021 · 1 comment
Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/feature New feature or request

Comments

@mbr
Copy link

mbr commented Apr 4, 2021

Feature Request

When logging a &std::error::Error directly, only the first .source() is printed, as seen in the example. This should be changed to log all of them instead.

Crates

Likely tracing, if changing the tracing::Value implementation.

Motivation

It's pretty common to have errors that are fairly nondescript until one follows more than step up the source chain, so it would be good to have the underlying causes logged.

Proposal

Alternatives

Implement your own newtype wrapper with a Display impl.

Somewhat related: #1308

@hawkw hawkw added crate/subscriber Related to the `tracing-subscriber` crate kind/feature New feature or request labels Apr 5, 2021
@hawkw
Copy link
Member

hawkw commented Apr 5, 2021

The issue here is in the default field formatter implementation for tracing_subscriber::fmt. If we look at these two lines:

if let Some(source) = value.source() {
self.record_debug(field, &format_args!("{}, {}: {}", value, field, source))

we see that only the immediate source is formatted, and any subsequent sources are skipped.

We could fix this pretty easily by changing that code to format the whole source chain.

We should probably also change the delimiters for the source so that they match the way other fields are formatted. Right now, a macro invocation like

tracing::info!(foo = "some field", err = error.as_ref());

will be formatted like

foo="some field" err=<error's display impl> err: <error's source's display impl>

which is not consistent with how other fields are formatted. We probably want something more like

foo="some field" err=<error's display impl> err.source=<error's source's display impl>

or, when there are multiple sources, maybe

foo="some field" err=<error's display impl> err.sources=[<error's source's display impl>, ...]

If we also want to change the behavior of the JSON formatter to emit error sources as a JSON list, we could add a record_error impl here:

impl<'a> field::Visit for JsonVisitor<'a> {

nightkr added a commit to nightkr/tracing that referenced this issue Jul 9, 2021
nightkr added a commit to nightkr/tracing that referenced this issue Jul 9, 2021
hawkw added a commit that referenced this issue Aug 6, 2021
## 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 <[email protected]>
@hawkw hawkw closed this as completed in c307e2a Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants