rust-lang / project-error-handling

Error handling project group
Apache License 2.0
268 stars 18 forks source link

Globally consistent error reporting formats #10

Open yaahc opened 3 years ago

yaahc commented 3 years ago

Problem

Right now there's no easy way to ensure that all errors are printed with a consistent error report format such as via eyre::Report + color-eyre. Errors that have not been converted to an error reporting type prior to panicking in functions like unwrap cannot then be correctly reported via the panic handler because the error is converted to a string and discarded. Even if it was not such as via panic!(error) directly you can only downcast to the exact type, so your error reporting hook would need an exhaustive list of all error types that it should try downcasting to, which is infeasible.

Possible Solutions

Carry original type in payloads for panics and add support for casting from &dyn Any to &dyn Error

One possible approach to this would be to adjust panics to include the data they were created with in the payload along with the formatted message and then add support for somehow extracting error trait objects back out of those &dyn Any playloads. Ideally the solution for extracting trait objects of other kinds from &dyn Any would not be specific to &dyn Error. If we could create a language / library level feature for casting between arbitrary trait objects or even only from &dyn Any to arbitrary trait objects that the underlying type also implements that would be ideal.

@oli-obk has suggested the possibility of using linker shenanigans to accomplish this:

linker tricks could make that actually feasible to implement like each type would allocate a named value and each impl block would append to that list not sure how we'd get the order right

@DianaNites has pointed out that the following libraries may be useful as sources of prior art

Use new format traits and specialization to format errors in a more structured way

https://github.com/rust-lang/project-error-handling/issues/10#issuecomment-720652610

kellpossible commented 3 years ago

I'm not entirely sure if this is related, and how it might relate to https://github.com/rust-lang/rfcs/pull/2895, but I'm currently trying to use tracing::Value error impl to render a logged eyre/color-eyre error nicely, with full context including backtrace/spantrace to be used in a sentry-core Event, while retaining the type information in tracing that this is indeed an error type, and not just a message.

e.g.

let boxed_error: Box<dyn std::error::Error + 'static> = eyre::eyre!("Logged an eyre error using `tracing`!").into();
tracing::error!(error = &*boxed_error);

My attempts to downcast &(std::error::Error + 'static) to eyre::Report using Any were blocked because it's not a static reference.

If https://github.com/rust-lang/rfcs/pull/2895 was stabilized, as I understand it, I should be able to access the extra metadata and render it again myself, but it might turn out differently to what it normally produced by color-eyre.

yaahc commented 3 years ago

The downcast won't work because the type that is actually used to create the Box<dyn Error> isn't actually the Report type, but rather the private inner type ErrorImpl<E> which does implement the error trait. Report itself cannot implement the error trait because of the overlap rule.

The generic member access RFC would probably help in this case, though I'm sure how best to integrate it. The easiest way would be to have the same generic member access API exist on the EyreHandler trait, and to implement the member access fn for ErrorImpl<E> to first attempt to grab context from the Handler, and then to forward any unfulfilled requests on to the inner error type. However, that context still has to be proactively requested by the tracing::Value implementation for std::error::Error, so you wouldn't necessarily be preserving all context, just the context you know to look for.

I've not looked at how tracing::Value works in a little while but I think a better solution might be to have some conditionally enabled tracing support within eyre itself, where it can implement tracing::Value. If its possible to compose implementations then we could have eyre::Report's Value implementation return the value implementations of the handler, if it has one, and the error, if it has one, and then fall back to using the std::error::Error Value implementation provided by tracing if the error doesn't provide a tracing::Value implementation.

edit: a quick check at the docs shows that Value is sealed, which is what I recalled. I'm not sure if there are plans to allow user implementations in the future but if not then the best bet is probably the former, where once the generic member access RFC is merged (assuming this ever happens) we update the tracing::Value impl for std::error::Error to try looking up a SpanTrace, other forms of context such as custom sections in color-eyre would still get lost this way though.

cc @hawkw

yaahc commented 3 years ago

I think this is currently my best plan but I don't really have an idea for how exactly to accomplish it let alone prototype it.

Report Trait and Reporter

The idea here is to reverse the responsibility for reporting errors, much the same way eyre does. Here are the main steps required to enact the plan:

The described traits might look something like this:

trait Report {
    fn fmt(&self, reporter: &dyn fmt::Reporter, f: &mut fmt::Formatter<'_>) -> fmt::Result;
}

trait Reporter {
   fn fmt_error(&self, error: &dyn Error, f: &mut fmt::Formatter<'_>) -> fmt::Result;
}

The default impls would look something like this:

default impl<D> Report for D
where
    D: Debug,
{
    fn fmt(&self, _reporter: &dyn Reporter, f: &mut Formatter<'_>) -> fmt::Result {
        Debug::fmt(self, f)
    }
}

default impl<E> Report for E
where
    E: Error,
{
    fn fmt(&self, reporter: &dyn Reporter, f: &mut Formatter<'_>) -> fmt::Result {
        reporter.fmt_error(&self, f)
    }
}

By default most errors would never need to interact with this trait, but reporting types like eyre or anyhow could override it to print their fancy reports without needing rely upon Debug, which is something of a hack right now.

The Reporter exists so we can separate the formatting for Errors from the trait used to display them, and I think its vaguely analogous to how structured logging or serialization works, were we can pass ourselves to the reporter in a structured way and let it handle laying out the various pieces of information (our sources, backtrace, etc).

Unresolved Questions

Once we have this we still have to figure out how to design the dyn Reporter trait, which is where the actual formatting definition is supplied. My current best idea is to have a globally installed one similar to the panic::set_hook fn which is used by default in panic! calls, and a write! like macro which can be used to pass one in manually, which would be used by crates like eyre.

Known Issues

  1. This design would require that fmt_error doesn't have a 'static bound, which would make downcasting the original source error while reporting it much more annoying, but it should still be possible via the generic member access API, though less convenient.
  2. Using traits here might introduce some backwards compatibility concerns
  3. Panics would still stringify their errors, so if your error captured a backtrace and your reporter knew to print it, the panic handler wouldn't know that the string it receives contains a formatted backtrace
  4. this would break functions like fn bla<E: Debug>(...) { let a: Result<.., E> = ...; a.unwrap() }, which might be fixable with a specialization for unwrap