Structural conversion from anyhow errors

[ten3roberts]

This implements a structural conversion to convert betwen other errors kinds and eyre Reports.

This allows richer error types to provide more context to the eyre report, such as anyhow by preserving backtrace and other context where possible.

This does not apply for anyhow, but future crates as well by implementing IntoEyreReport. This can also be used to provide a conversion from a custom error type or enum which contains a Report that does not nest the Report within a new report, but flattens it which ensures the spantrace and backtrace are forwarded instead of being debug printed without structure inside another backtrace, which can be very confusing.

This is an issue I've had with flax, where the library error enum has a variant for a boxed user error, as well as concrete error variants. I wanted to avoid nesting a eyre::Report or Box<dyn Error> inside another Report if a user wraps flax's error themselves.

Resolves: #65 (comment)

Resolves: #31

[ten3roberts]

[yaahc]

the main rough edge to watch out for is accidentally using Into instead of IntoReport and losing context.

you might be able to catch these at runtime with debug asserts inside of the eyre hook when converting to a report, by inspecting the &dyn Error to look for eyre::Reports or other known types you want to use IntoReport on.

[ten3roberts]

the main rough edge to watch out for is accidentally using Into instead of IntoReport and losing context.

you might be able to catch these at runtime with debug asserts inside of the eyre hook when converting to a report, by inspecting the &dyn Error to look for eyre::Reports or other known types you want to use IntoReport on.

Yes, this is a footgun and unfortunately as far as I can tell a limitation with Rust.

For reference we discussed this a bit yesterday, and while the IntoEyre trait will allow you to convert types to eyre::Reports in the absence of std::error::Error, such as #![no_std], it will also allow you to use ? when the Error type does implement error, in which case it will skip the specialized flattening and backtrace re-attaching for e.g errors containing eyre reports themselves, such as https://github.com/ten3roberts/flax/blob/main/src/error.rs.

Something we could do, is try and rethink the impl<T> FromT> for eyre::Report where T: Error and see if we can work around that one, as it is the root cause of the conflicting traits that disallow implementing Into<eyre::Report>.

Any ideas are highly welcome ❤️

[ten3roberts]

Some further consideration and ideas regarding this:

1. We allow std::error::Error and IntoEyre to coexist, and while the user can use ? and lose some context, they can go back to the code and change it into into_eyre to get more information when needed, similar to how one today goes back and adds or adds a .wrap_err, or gives it more information.
2. We modify the eyre! to firstly look for an IntoEyre implementation, before falling back on Error or Display
3. We create a conflicting impl with Error to force downstream users to implement one or the other, not both. This is most disruptive and may make interop harder if your type can't also be a normal error. This would be detrimental for flax.
4. Modify downstream eyre handlers to check for nested eyre Reports. This is very unstable and will be prone for false-positives or impossibilties where the type system is not capable of using IntoEyre, most commonly because an error arrives as a Box<dyn Error> or impl Error, but the current concrete type happens to contain an eyre Report or implement IntoEyre. This would be a nightmare in stability and leakage of error internals for erased types, and it would also not catch all since an IntoEyre may do more than just flattening, and we could not detect that and we'd miss missing uses of IntoEyre.

[yaahc]

My preference is 1 & 2

[thenorili]

Some further consideration and ideas regarding this:

1. We allow std::error::Error and IntoEyre to coexist, and while the user can use ? and lose some context, they can go back to the code and change it into into_eyre to get more information when needed, similar to how one today goes back and adds or adds a .wrap_err, or gives it more information.
2. We modify the eyre! to firstly look for an IntoEyre implementation, before falling back on Error or Display
3. We create a conflicting impl with Error to force downstream users to implement one or the other, not both. This is most disruptive and may make interop harder if your type can't also be a normal error. This would be detrimental for flax.
4. Modify downstream eyre handlers to check for nested eyre Reports. This is very unstable and will be prone for false-positives or impossibilties where the type system is not capable of using IntoEyre, most commonly because an error arrives as a Box<dyn Error> or impl Error, but the current concrete type happens to contain an eyre Report or implement IntoEyre. This would be a nightmare in stability and leakage of error internals for erased types, and it would also not catch all since an IntoEyre may do more than just flattening, and we could not detect that and we'd miss missing uses of IntoEyre.

I'm of the same mind as Jane, 1+2 sounds best. I think there's a compromise with 3. We could, in theory, publish a non-default lint that checks for usage of ? or into() where into_eyre() is also possible. That way folks who do care about not losing context have that option while the general case stays flexible and nondisruptive.