Skip to content

Commit

Permalink
subscriber: don't bail when timestamp formatting fails (#1689)
Browse files Browse the repository at this point in the history
## Motivation

Currently, `tracing_subscriber::fmt` will bail out of formatting the log
line when formatting the timestamp fails. Previously, when timestamps
were formatted using the `chrono` crate, this was the correct behavior,
as getting the timestamp was infallible, and an error would only be
returned if *formatting* the timestamp to the buffer fails. This should
never actually happen, because we are writing the timestamp to a string,
which should never fail; using `?` is just a bit more efficient than
`.expect` because it doesn't require generating unwinding code.

However, this is no longer the case when using the `time` crate. In the
`time` API, actually getting a timestamp is fallible, and in some cases,
will fail. The current code will bail out of formatting the entire log
line if getting the timestamp fails, which is not great --- using the
wrong timestamp formatter will result in silently dropping all log
lines.

## Solution

This branch changes the `format_timestamp` method to print `<unknown
time>` when the timestamp formatting fails, rather than bailing. This
way, we at least print the rest of the log line for the event.

This fixes half of the issue described in #1688 (the other half is
improving documentation).
  • Loading branch information
hawkw committed Nov 30, 2021
1 parent a26ccd8 commit cc8846a
Showing 1 changed file with 12 additions and 2 deletions.
14 changes: 12 additions & 2 deletions tracing-subscriber/src/fmt/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -692,14 +692,24 @@ impl<F, T> Format<F, T> {
if writer.has_ansi_escapes() {
let style = Style::new().dimmed();
write!(writer, "{}", style.prefix())?;
self.timer.format_time(writer)?;

// If getting the timestamp failed, don't bail --- only bail on
// formatting errors.
if self.timer.format_time(writer).is_err() {
writer.write_str("<unknown time>")?;
}

write!(writer, "{} ", style.suffix())?;
return Ok(());
}
}

// Otherwise, just format the timestamp without ANSI formatting.
self.timer.format_time(writer)?;
// If getting the timestamp failed, don't bail --- only bail on
// formatting errors.
if self.timer.format_time(writer).is_err() {
writer.write_str("<unknown time>")?;
}
writer.write_char(' ')
}
}
Expand Down

0 comments on commit cc8846a

Please sign in to comment.