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: change FormatEvent::format_event trait return type to allow any arbirtrary error #2103

Open
bryangarza opened this issue Apr 30, 2022 · 1 comment
Labels
good first issue Good for newcomers

Comments

@bryangarza
Copy link
Member

Feature Request

Crates

tracing-subscriber

Motivation

Currently, the FormatEvent trait's format_event function has a return type of fmt::Result:

fn format_event(
&self,
ctx: &FmtContext<'_, S, N>,
writer: Writer<'_>,
event: &Event<'_>,
) -> fmt::Result;

fmt::Result is defined as

pub type Result = result::Result<(), Error>;

pub struct Error;

This means that any implementation of format_event loses the original error when returning. This makes it more difficult for the user to be able to figure out why something went wrong. One concrete example is the JSON formatter. If there is an error during serialization, tracing-subscriber is able to let the user know there was an error (once #2102 is merged) but not why.

Proposal

Update the format_event function's return type to allow arbitrary errors. This way, we can retain the original error when something goes wrong and the caller of the function can take the error and do something with it.

Along with this, we should update the JSON formatter to pass the error back up:

visit().map_err(|_| fmt::Error)?;

Alternatives

  • Instead of propagating the error back up, have each format_event implementation print out the errors to stderr on their own
    • Downside: User-provided FormatEvent impls may forget to include this
    • Downside: Don't have access to the same Writer as the Writer that is used for the traces themselves (unless we add that as a parameter in format_event, which is a breaking change too)

Considerations

This is a breaking change. It would only be able to be included in tracing-subscriber 0.4 or later.

@sanjayts
Copy link

Hi, I would be interested in picking this up and was hoping you could clarify what sort of new error type are we looking at? The usual Box<dyn Error + Sync + Send>? If yes, do you think we should use the standard Result type or expose a public type alias like pub type FmtError = std::result::Result<(), Box<dyn Error>>;.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants