H-4706: Support `Report<dyn Error>` as anonymous context in error-stack `Report`s

[zakstucke]

Alternative to #7256 that adds ? support to Report<dyn Context + Send + Sync + 'static> instead of an AnyReport wrapper.

This is even more powerful because the error_stack::Result type alias can be updated to:

pub type Result<T, C = dyn Context + Send + Sync + 'static> = core::result::Result<T, Report<C>>;

I see it's deprecated, but it's something that can be done in userland.

Which makes switching between specific contexts and a catch all completely painless. Result<T, E> can simply remove the E to make it catch-all.

This is at the cost of a breaking change, the new From impl breaks Report::from type inference, meaning some Report::from's need some extra hinting. #7256 is not a breaking change.

[zakstucke]

I have now updated the Report struct definition to:

struct Report<C: ?Sized = dyn Context + Send + Sync + 'static>

this makes an "untyped report" a first class citizen and means Report and Report<C> can be used interchangeably, an untyped Report acts like AnyReport does in #7256.

The concept of an untyped Report feeds nicely into a new feature, owned type extraction:

impl<C> Report<C> {
    pub fn pop_current_context(self) -> (Report, C)
        where
            C: Send + Sync + 'static

    pub fn into_current_context(self) -> C
        where
            C: Send + Sync + 'static
}

impl<C: ?Sized> Report<C> {
    pub fn downcast<T: Send + Sync + 'static>(self) -> Result<T, Self>

    pub fn downcast_take<T: Send + Sync + 'static>(self) -> Result<(Report, T), Self>
}

This addresses the second painpoint of ES, some error types don't implement Clone, or cloning is very expensive, despite needing to own them.

This PR now implements these methods, making sure to not affect the printable representation of a report despite extracting types, by eagerly formatting on extraction.

[zakstucke]

...behaves differently as soon as you import error_stack::Result. This was quite annoying...

I agree, I actually don't use this, but a renamed ResultStack version instead that does the same thing, the alias is useful, it just shouldn't overwrite core::result::Result. Anyhow, it's scheduled for removal and can be done in userland, I've reverted the change.

Context is not needed at all and we should not build anything on top of it but simply use dyn Error here.

Agreed, updated!

I don't like us to add a default to it as this encourages omitting C in most cases. The user should explicitly opt-in to the generic error.

I disagree with this, but understand the stance. I think having a "split"/"untyped" Report as a concept is powerful and useful as shown in the C extraction methods, I also feel ES can explain in docs and style how it's intended to be used, without introducing hurdles for users that do want to use ES in more flexible ways. Finally like with the alias, it makes migrations easier and more gradual. Given a user can re-type alias Report in userland, I'd be fine with removing this default if it unblocked this PR.

[zakstucke]

Thanks! I've split out the extract methods into #7324 to not block this one, I've explained the usecase there and in other review comments.

Another thing we might want to explore is if we could solve https://github.com/hashintel/hash/issues/4295 with Report<dyn Error>.

I've just tested the reproducer for that issue, it seems like it already works with the current version of error stack.

Compiles fine after updating the reproducer repo's Cargo.toml they provided with:

[patch.crates-io]
error-stack.git = "https://github.com/hashintel/hash"

[dependencies]
error-stack = { version = "0.5", features = ["eyre"] }

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 45.45455% with 18 lines in your changes missing coverage. Please review.

Project coverage is 53.05%. Comparing base (339d4db) to head (4b7be78). Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
libs/error-stack/src/report.rs	0.00%	15 Missing :warning:
libs/error-stack/src/sink.rs	81.25%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7274      +/-   ##
==========================================
- Coverage   56.77%   53.05%   -3.72%     
==========================================
  Files        1044     1043       -1     
  Lines       99735    92020    -7715     
  Branches     4411     4386      -25     
==========================================
- Hits        56622    48821    -7801     
- Misses      42163    42276     +113     
+ Partials      950      923      -27     

Flag	Coverage Δ
apps.hash-ai-worker-ts	1.32% <ø> (ø)
apps.hash-api	0.00% <ø> (ø)
blockprotocol.type-system	35.85% <ø> (ø)
local.harpc-client	50.93% <ø> (ø)
local.hash-backend-utils	3.96% <ø> (ø)
local.hash-graph-sdk	0.00% <ø> (ø)
local.hash-isomorphic-utils	0.00% <ø> (ø)
rust.deer	74.75% <ø> (ø)
rust.error-stack	88.65% <41.93%> (-0.43%)	:arrow_down:
rust.harpc-codec	84.97% <ø> (ø)
rust.harpc-net	95.87% <ø> (+0.01%)	:arrow_up:
rust.harpc-tower	66.34% <100.00%> (ø)
rust.harpc-wire-protocol	91.62% <ø> (ø)
rust.hash-codec	75.38% <ø> (ø)
rust.hash-graph-api	5.04% <ø> (-0.01%)	:arrow_down:
rust.hash-graph-authorization	28.84% <ø> (ø)
rust.hash-graph-postgres-store	21.37% <ø> (-0.18%)	:arrow_down:
rust.hash-graph-store	27.99% <ø> (ø)
rust.hash-graph-temporal-versioning	48.22% <ø> (ø)
rust.hash-graph-types	0.00% <ø> (ø)
rust.hash-graph-validation	83.30% <ø> (ø)
rust.hashql-ast	85.74% <ø> (ø)
rust.hashql-compiletest	53.86% <ø> (ø)
rust.hashql-core	79.88% <ø> (-6.58%)	:arrow_down:
rust.hashql-diagnostics	50.24% <ø> (ø)
rust.hashql-hir	81.69% <ø> (ø)
rust.hashql-syntax-jexpr	94.10% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[TimDiekmann]

As requested changes were not applied (i.e. documentation changes) and we also don't encourage using Report<dyn Error>, I'm going ahead and close this.