Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Globally consistent error reporting formats #10

Open
yaahc opened this issue Sep 23, 2020 · 3 comments
Open

Globally consistent error reporting formats #10

yaahc opened this issue Sep 23, 2020 · 3 comments

Comments

@yaahc
Copy link
Member

yaahc commented Sep 23, 2020

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

#10 (comment)

@yaahc yaahc changed the title Research: Globally consistent error reporting formats Globally consistent error reporting formats Sep 23, 2020
@kellpossible
Copy link

kellpossible commented Oct 11, 2020

I'm not entirely sure if this is related, and how it might relate to rust-lang/rfcs#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 rust-lang/rfcs#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
Copy link
Member Author

yaahc commented Oct 12, 2020

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
Copy link
Member Author

yaahc commented Nov 2, 2020

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:

  • Introduce a new std::fmt::Report trait
  • Introduce another std::fmt::Reporter trait which acts like https://docs.rs/eyre/0.6.2/eyre/trait.EyreHandler.html
  • using specialization add impls for this trait for types that impl Debug and Error, shown below
  • change unwrap and similar methods that print error types in Results to print the Err type with this Report trait instead of Debug, this is made backwards compatible by the specializable blanket impl on Debug
  • similarly, change the Termination impl on Result used for printing errors in fn main() -> Result<T, E> to use Report instead of Debug.

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants