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 6 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
96 changes: 95 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,
silence_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,
silence_errors: self.silence_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,
silence_errors: self.silence_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,
silence_errors: self.silence_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,
silence_errors: self.silence_errors,
make_writer: TestWriter::default(),
_inner: self._inner,
}
Expand All @@ -278,6 +283,14 @@ impl<S, N, E, W> Layer<S, N, E, W> {
}
}

/// Whether to write errors to the Writer, or ignore them.
bryangarza marked this conversation as resolved.
Show resolved Hide resolved
pub fn with_silence_errors(self, silence_errors: bool) -> Self {
Self {
silence_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 +319,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,
silence_errors: self.silence_errors,
make_writer: f(self.make_writer),
_inner: self._inner,
}
Expand Down Expand Up @@ -337,6 +351,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
silence_errors: self.silence_errors,
_inner: self._inner,
}
}
Expand All @@ -349,6 +364,7 @@ where
fmt_span: self.fmt_span.without_time(),
make_writer: self.make_writer,
is_ansi: self.is_ansi,
silence_errors: self.silence_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -477,6 +493,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
silence_errors: self.silence_errors,
_inner: self._inner,
}
}
Expand All @@ -491,6 +508,7 @@ where
fmt_span: self.fmt_span,
make_writer: self.make_writer,
is_ansi: self.is_ansi,
silence_errors: self.silence_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -521,6 +539,7 @@ where
make_writer: self.make_writer,
// always disable ANSI escapes in JSON mode!
is_ansi: false,
silence_errors: self.silence_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -587,6 +606,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,
silence_errors: self.silence_errors,
_inner: self._inner,
}
}
Expand Down Expand Up @@ -617,6 +637,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,
silence_errors: self.silence_errors,
_inner: self._inner,
}
}
Expand All @@ -630,6 +651,7 @@ impl<S> Default for Layer<S> {
fmt_span: format::FmtSpanConfig::default(),
make_writer: io::stdout,
is_ansi: cfg!(feature = "ansi"),
silence_errors: false,
_inner: PhantomData,
}
}
Expand Down Expand Up @@ -749,6 +771,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 +922,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.silence_errors {
if let Err(e) = res {
eprintln!("Unable to write an event to the Writer for this Subscriber! Error: {}\n", e);
}
}
} else if !self.silence_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!("Unable to write an \"event formatting error\" to the Writer for this Subscriber! Error: {}\n", e);
bryangarza marked this conversation as resolved.
Show resolved Hide resolved
}
}

buf.clear();
Expand Down Expand Up @@ -1192,6 +1232,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 = "Unable to format the following event. Name: event tracing-subscriber/src/fmt/fmt_layer.rs:";
bryangarza marked this conversation as resolved.
Show resolved Hide resolved
assert!(actual.as_str().starts_with(expected), "\nactual = {}\nshould start with expected = {}\n", actual, expected);
}

#[test]
fn format_error_ignore_if_silence_errors_is_true() {
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)
.with_silence_errors(true)
.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
13 changes: 13 additions & 0 deletions tracing-subscriber/src/fmt/mod.rs
Expand Up @@ -615,6 +615,19 @@ where
}
}

/// Whether to write errors to the Writer, or ignore them.
///
/// An error in this case refers to errors which may occur within the
/// Subscriber implementation itself, or in code that it depends on.
///
/// It does not refer to traces set to the level of Error.
bryangarza marked this conversation as resolved.
Show resolved Hide resolved
pub fn with_silence_errors(self, silence_errors: bool) -> SubscriberBuilder<N, format::Format<L, T>, F, W> {
SubscriberBuilder {
inner: self.inner.with_silence_errors(silence_errors),
..self
}
}

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