error-stack: Report<E> is not compatible with std::error::Error

[Tohman21]

Related Problem

Hello! I ran into a problem, which is better to show with example:

struct Foo;

impl<V> Foo for SomeTrait<V>
  where
    V: TryInto<Vec<u8>>,
   <V as TryInto<Vec<u8>>>::Error: error_stack::Context,
{
  fn do_stuff(&self, value: V) -> error_stack::Result<(), MyError> {
    ...
    let my_vec: Vec<u8> = value.try_into().into_report().change_context(MyError);
    ...
  }
}

struct Bar;

impl TryInto<Vec<u8>> for Bar {
  type Error = error_stack::Report<TransformError>;

  fn try_into(self) -> Result<Vec<u8>, Self::Error> {
    /// Some serialization
  }
}

fn main() {
  let foo = Foo;
  let bar = Bar;
  let baz = "baz".to_owned();

  foo.do_stuff(baz); // Good
  foo.do_stuff(bar); // Error. The trait `StdError` is not implemented for `error_stack::Report<TransformationError>`
}

I have implementation of SomeTrait that requires TryInto<Vec<u8>>::Error implement error_stack::Context. But Report is not impl Context and is not impl Error.

Proposed Solution

Implement Error for Report:

impl<E> std:error::Error for Report<E> {}

It's simple and allows to write more abstract code. In other words Report will compatible with Errors

Alternatives

No response

Additional context

No response

[TimDiekmann]

Hi @Tohman21 and thanks for filing the issue.

TL;DR: This is blocked by the specialization feature.

I'd love to implement Error for Report but currently that's sadly not possible for a few reasons:

• (major) With an upcoming change we remove IntoReport, which implies impl<T, C: Context> ResultExt for Result<T, C>. Implementing Error for Report would imply a conflicting implementation. It's likely, that this can be solved with feature(specialization) enabled, but I have not tested this, yet. If this would be a breaking change this is fine as we want to adjust other parts of the API as soon as specialization is possible (e.g. merging attach and attach_printable).
• (major) If Error is implemented on Report it's expected that Error::source is behaving correctly. As error-stack supports multiple source errors, the mapping does not work correctly. We're planning some overhaul for this "related-errors" feature in the near future and I will definitely will keep this issue in mind.
• (probably the hardest issue to solve) It's easily possible to create Report<Report<E>>, which results in unexpected behavior, annoying errors, and hard-to-debug issues. It's simply not what people want to use. This cannot easily be solved without some kind of negative trait bounds which, AFAIK, is nothing that is immediately planned right now. Possibly it's possible to solve this via specialization, though.
• (minor) There is a conflicting implementation between the From conversion of Report<E> and Box<dyn Error> when Error is implemented for Report<E>, but if anything like this is going to be added, we'd probably implement Context for Report<E: 'static> instead to be more generic. This is probably not a big issue but another annoyance when dealing with the Error trait. Ultimately we probably going to remove the Context trait when core::error::Error is stabilized but that's not fully decided yet as Context is potentially more generic than Error semantically.

One other possible route would also be to not remove IntoReport, yet, but rather try to implement Error on Report earlier (I don't expect specialization to be stabilized soon), but we still need to figure out the other issues first. However, with IntoReport and Report<E>: Error creating Report<Report<E>> is very easy by calling Result<T, Report<E>>::into_report(). We even have a dedicated test case to forbid this:

hash/libs/error-stack/tests/ui/into_report.rs

Lines 19 to 24 in 12fecc4

	let result = Err::<(), _>(RootError);
	let _: Result<(), Report<RootError>> = result.into_report();
	let result = Err::<(), _>(Report::new(RootError));
	// Not allowed
	let _ = result.into_report();

it's possible that one of the listed issues is not really an issue or that I forgot something but it's definitely not easily possible now - and I'm very sad about this! We'd be very happy to hear more thoughts about this as well!

Happy to hear some thoughts from @indietyp as well.

[vilkinsons]

@Tohman21 FYI, until some of the external blockers outlined in Tim's answer have been resolved we're going to convert this issue to a discussion.

[Tohman21]

Thanks for quick answer. Yep, the problem is deeper than I thought