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: if error occurs when formatting event, write error to Writer #2102

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
106 changes: 105 additions & 1 deletion tracing-subscriber/src/fmt/fmt_layer.rs
Expand Up @@ -70,6 +70,7 @@ pub struct Layer<
fmt_event: E,
fmt_span: format::FmtSpanConfig,
is_ansi: bool,
log_internal_errors: bool,
_inner: PhantomData<fn(S)>,
}

Expand Down Expand Up @@ -119,6 +120,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -148,6 +150,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -180,6 +183,7 @@ impl<S, N, E, W> Layer<S, N, E, W> {
fmt_event: self.fmt_event,
fmt_span: self.fmt_span,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
make_writer,
_inner: self._inner,
}
Expand Down Expand Up @@ -263,6 +267,7 @@ impl<S, N, E, W> Layer<S, N, E, W> {
fmt_event: self.fmt_event,
fmt_span: self.fmt_span,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
make_writer: TestWriter::default(),
_inner: self._inner,
}
Expand All @@ -278,6 +283,24 @@ impl<S, N, E, W> Layer<S, N, E, W> {
}
}

/// Sets whether to write errors from [`FormatEvent`] to the writer.
/// Defaults to true.
///
/// By default, `fmt::Layer` will write any `FormatEvent`-internal errors to
/// the writer. These errors are unlikely and will only occur if there is a
/// bug in the `FormatEvent` implementation or its dependencies.
///
/// If writing to the writer fails, the error message is printed to stderr
/// as a fallback.
///
/// [`FormatEvent`]: crate::fmt::FormatEvent
pub fn log_internal_errors(self, log_internal_errors: bool) -> Self {
Self {
log_internal_errors,
..self
}
}

/// Updates the [`MakeWriter`] by applying a function to the existing [`MakeWriter`].
///
/// This sets the [`MakeWriter`] that the layer being built will use to write events.
Expand Down Expand Up @@ -306,6 +329,7 @@ impl<S, N, E, W> Layer<S, N, E, W> {
fmt_event: self.fmt_event,
fmt_span: self.fmt_span,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
make_writer: f(self.make_writer),
_inner: self._inner,
}
Expand Down Expand Up @@ -337,6 +361,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand All @@ -349,6 +374,7 @@ where
fmt_span: self.fmt_span.without_time(),
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -477,6 +503,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand All @@ -491,6 +518,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -521,6 +549,7 @@ where
make_writer: self.make_writer,
// always disable ANSI escapes in JSON mode!
is_ansi: false,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -587,6 +616,7 @@ impl<S, N, E, W> Layer<S, N, E, W> {
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -617,6 +647,7 @@ impl<S, N, E, W> Layer<S, N, E, W> {
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
log_internal_errors: self.log_internal_errors,
_inner: self._inner,
}
}
Expand All @@ -630,6 +661,7 @@ impl<S> Default for Layer<S> {
fmt_span: format::FmtSpanConfig::default(),
make_writer: io::stdout,
is_ansi: cfg!(feature = "ansi"),
log_internal_errors: false,
_inner: PhantomData,
}
}
Expand Down Expand Up @@ -749,6 +781,11 @@ where
{
fields.was_ansi = self.is_ansi;
extensions.insert(fields);
} else {
eprintln!(
"[tracing-subscriber] Unable to format the following event, ignoring: {:?}",
attrs
);
}
}

Expand Down Expand Up @@ -895,7 +932,20 @@ where
.is_ok()
{
let mut writer = self.make_writer.make_writer_for(event.metadata());
let _ = io::Write::write_all(&mut writer, buf.as_bytes());
let res = io::Write::write_all(&mut writer, buf.as_bytes());
if self.log_internal_errors {
if let Err(e) = res {
eprintln!("[tracing-subscriber] Unable to write an event to the Writer for this Subscriber! Error: {}\n", e);
}
}
} else if self.log_internal_errors {
let err_msg = format!("Unable to format the following event. Name: {}; Fields: {:?}\n",
event.metadata().name(), event.fields());
let mut writer = self.make_writer.make_writer_for(event.metadata());
let res = io::Write::write_all(&mut writer, err_msg.as_bytes());
if let Err(e) = res {
eprintln!("[tracing-subscriber] Unable to write an \"event formatting error\" to the Writer for this Subscriber! Error: {}\n", e);
}
}

buf.clear();
Expand Down Expand Up @@ -1192,6 +1242,60 @@ mod test {
re.replace_all(s.as_str(), "timing").to_string()
}

#[test]
fn format_error_print_to_stderr() {
struct AlwaysError;

impl std::fmt::Debug for AlwaysError {
fn fmt(&self, _f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
Err(std::fmt::Error)
}
}

let make_writer = MockMakeWriter::default();
let subscriber = crate::fmt::Subscriber::builder()
.with_writer(make_writer.clone())
.with_level(false)
.with_ansi(false)
.with_timer(MockTime)
.finish();

with_default(subscriber, || {
tracing::info!(?AlwaysError);
});
let actual = sanitize_timings(make_writer.get_string());

// Only assert the start because the line number and callsite may change.
let expected = concat!("Unable to format the following event. Name: event ", file!(), ":");
assert!(actual.as_str().starts_with(expected), "\nactual = {}\nshould start with expected = {}\n", actual, expected);
}

#[test]
fn format_error_ignore_if_log_internal_errors_is_false() {
struct AlwaysError;

impl std::fmt::Debug for AlwaysError {
fn fmt(&self, _f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
Err(std::fmt::Error)
}
}

let make_writer = MockMakeWriter::default();
let subscriber = crate::fmt::Subscriber::builder()
.with_writer(make_writer.clone())
.with_level(false)
.with_ansi(false)
.with_timer(MockTime)
.log_internal_errors(false)
.finish();

with_default(subscriber, || {
tracing::info!(?AlwaysError);
});
let actual = sanitize_timings(make_writer.get_string());
assert_eq!("", actual.as_str());
}

#[test]
fn synthesize_span_none() {
let make_writer = MockMakeWriter::default();
Expand Down
20 changes: 19 additions & 1 deletion tracing-subscriber/src/fmt/mod.rs
Expand Up @@ -465,7 +465,7 @@ impl Default for SubscriberBuilder {
SubscriberBuilder {
filter: Subscriber::DEFAULT_MAX_LEVEL,
inner: Default::default(),
}
}.log_internal_errors(true)
}
}

Expand Down Expand Up @@ -620,6 +620,24 @@ where
}
}

/// Sets whether to write errors from [`FormatEvent`] to the writer.
/// Defaults to true.
///
/// By default, `fmt::Layer` will write any `FormatEvent`-internal errors to
/// the writer. These errors are unlikely and will only occur if there is a
/// bug in the `FormatEvent` implementation or its dependencies.
///
/// If writing to the writer fails, the error message is printed to stderr
/// as a fallback.
///
/// [`FormatEvent`]: crate::fmt::FormatEvent
pub fn log_internal_errors(self, log_internal_errors: bool) -> SubscriberBuilder<N, format::Format<L, T>, F, W> {
SubscriberBuilder {
inner: self.inner.log_internal_errors(log_internal_errors),
..self
}
}

/// Sets whether or not an event's target is displayed.
pub fn with_target(
self,
Expand Down