Fix traced CCE when nested different types.

[kyay10]

When traced is used in a nested manner, a raise call that refers to the outer traced Raise gets intercepted by the inner traced. The result of this is that a CCE gets thrown because the inner traced only expects errors of the type it declares.

I think more generally any method that intercepts a RaiseCancellationException cannot function properly without the chance for CCE. This means that no methods in Arrow should be allowed to reuse a Raise instance and intercept its error with raisedOrRethrow, and instead those methods need to create their own Raise instance inside. The only method that does that is traced, but perhaps an internal note should be added to never use raisedOrRethrow except if you yourself created the Raise instance that you're using it on

[serras]

Looks good to me, except for the new public API function

[serras]

Do we need to make this part of the API? It feels that it can be directly placed in the code.

[kyay10]

It encapsulates doing a check with the RaiseCancellationException, which is private. I can instead make it @PublishedAPI internal

[serras]

Suggested change

	} catch (rethrown: CancellationException) {
	throw rethrown.withCause(e)
	}
	} catch (rethrown: RaiseCancellationException) {
	throw RaiseCancellationException(rethrown.raised, rethrown.raise, e.cause ?: rethrown.cause)
	}

[kyay10]

We can't do this because of #3235 unless we @PublishedApi RaiseCancellationException.

[nomisRev]

I've been contemplating doing this.. but I've been a bit absent last 2 months due to some personal things keeping me busy.

My rationale for making it public is because I've had a scenario or two where I wanted to differentiate between RaiseCancellationException, and (Job) CancellationException. It's in really low-level edge case scenarios though, but it would allow figuring out if the CancellationException comes from Arrow, from KotlinX Coroutines, or from somewhere else.

Perhaps it warrants @DelicateApi or something, like KotlinX does to force @OptIn so it requires "flagging" low-level code.

WDYT?

[kyay10]

I think giving users that option is a good idea, especially because we already have tests confirming that try-catch can recover from raise. We'd have to make both Raise exceptions public thus. It's another question if we should allow inspecting the error, too.

[nomisRev]

Initially I would say don't expose it if we don't have a need for it.

I got a small WIP, that relies on some new functionality and unofficial support of Kotlin 😅
It adds a public sealed class, with expect/actual implementation to disable the stack traces.

I'm going to raise it as a PR targeting this one, so we can discuss further based on a code example ☺️

[nomisRev]

This can now be replaced by the sealed public error for Raise, and then this PR is ready to be merged IMO 🥳 🙌

[nomisRev]

@kyay10 I cannot target the branch of your fork from here, #3349.

Also, I just realised you're not a member of the Arrow org. Would you like to be? Then you can make branches directly on Arrow, officially approve PRs, etc ☺️ You've been extremely helpful over the years, and have been very active member both here and on Slack so it seems waaay overdue 😅

[kyay10]

@nomisRev Absolutely yes!! Thank you so, so much for the kind words! Words can't describe how grateful I am; Arrow has provided a great and welcoming environment for me to learn and grow, and hence I can only be honoured to help out whenever I can!

I'll have a look at the PR now!

[nomisRev]

@kyay10 I'm so glad to hear that! ☺️

You should've received an invitation to join the Arrow org! So feel free to work directly on the Arrow repo now, etc 😉 Finally, officially, Welcome to the team 🙌 🥳

[serras]

@nomisRev @kyay10 please check that I've merged main here correctly. If so, please proceed to squash and merge into main.

[serras]

Well, I've broken the build and I don't know how to fix it... :/

[kyay10]

I'll have a look in a moment! main I think just had significant changes to how RaiseCancellationException works

[nomisRev]

Thank you for the awesome work @kyay10! 🙌 🙏

This looks great, and I really like the clean-up with RaiseCancellationException in foldUnsafe it becomes very clear now how it works.

I would personally only like to apply my suggestion, solely because it avoids having it in the public binary.

[nomisRev]

Oh... right 😓 I missed that. No, this is much better! You definitely made the right call on that one 😉
Great work @kyay10! 👏 👏 👏