WIP: Pass Trace transparently through withError

[kyay10]

This is achieved by sealing Raise to check DefaultRaise.isTraced.

[github-actions]

Kover Report

File	Coverage [94.29%]
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Fold.kt	95.38%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Raise.kt	97.22%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Trace.kt	50.00%

Total Project Coverage	59.59%

[kyay10]

Turning Raise into a sealed interface feels like a very hard change to justify. I think there's some use cases opened up here, though, like accessing isTraced.

The implementors of Raise should be very few and far in between, and both provided APIs (TransformingRaise and RaiseWrapper) should cover all use cases (one can throw an exception inside transform if they really want to override raise's behaviour).

RaiseWrapper can be very easily deprecated as well when context receivers come around, since its implementors are ones where migrating to contexts is very easy.
TransformingRaise might stick around for longer because it makes certain implementations of Raise have somewhat less allocations, and hence might be preferable (e.g. in SingletonRaise and RaiseAccumulate). I've ended up keeping it around in arrow-context for that reason, although I can see an argument that it's rather pointless to keep it and that, instead, one should just use withError.

[kyay10]

Requesting reviews just to get some thoughts on this. I'm not fully convinced that this PR is the right approach, but It's hard to see how else to conditionally trace based on whether the caller is in a traced context or not.

Food for thought: with this change and #3349, maybe it's time to redesign traced?

[serras]

Honestly, for me this is a no-go, we really want to keep this one open for extension (we even talk about that in the docs).

[kyay10]

Note that I'm keeping it open by instead providing RaiseWrapper and TransformingRaise. Those 2 are the only use cases that I can observe. Most people will use RaiseWrapper if they're just defining an intersection type (and we can deprecate it when contexts are stable). TransformingRaise is another pattern where one wants to change the error type right then and there (used in RaiseAccumulate and SingletonRaise), and it's slightly more performance than withError.

Instead of this change, would you prefer we force implementors to provide a val underlyingRaise: Raise<*> or something along those lines? I didn't want to go through that approach because one can do val underlyingRaise get() = this which I heavily dislike

[nomisRev]

I agree with @serras. I understand it's still open for extension, but not in a straightforward way.

Did not yet have the time to properly dive into this, but perhaps we can find a different solution. Perhaps going the other-way around might be better. Something like CoroutineStackFrame which you can optionally implement, if you need behavior like this one.

We can dynamically check if a Raise interface implements RaiseFrame, or similar and then we'd get access to additional properties. That would probably even be possible to add in 1.x

No idea if any of this would work, just spitting some ideas ☺️ Will make some time asap to look into this issue.

[serras]

I've head of teams using CoroutineContext to pass this information, but unfortunately we don't have suspend in our raise function.

[kyay10]

How about introducing some form of interface RaiseWrapper<Error>: Raise<Error> { val underlyingRaise: Raise<*> } and encouraging users to gradually use it? We can support this feature then for those implementors. Alternatively, we can add an val underlyingRaise into Raise directly, with a default impl of returning this, although this would be a 2.0 feature.

[kyay10]

Maybe I'm also getting distracted by thinking too far into the future. With contexts, we don't want "delegating" implementors of Raise, because they can always be replaced by composing Raise and the class in question. Instead, we want "transforming" implementors that actually do something interesting with the error value. Maybe that, as a plan, is unnecessarily ambitious. I'll experiment here with simplifying this, focusing on requiring as little migration as possible.