Reinventing `CancelledError` for ease of use

[emizzle]

The DX problem

Handling, or not handling, CancelledError is one of the biggest pain points in chronos as a developer. It's one of the hardest concepts to grasp as a beginner, it's the easiest to miss when crafting new code, and it's very difficult to debug if you get it wrong. Most people will write code like this:

proc myProc() {.async: raises: [].} = # or just {.async.}
  try:
    await someAsyncProc()
  except CatchableError as e:
    error "something bad happened", error = e.msg

and inadvertently swallow cancellation propagation. This has bitten us countless times in Codex, and caused many days of debugging effort as a result.

Workaround idea

1. Make CancelledError "special", ie not derived from CatchableError and internally managed by chronos, so that the dev would almost never need to interfere with cancellation propagation (but can, if needed, see last point). Similar to how Defect is no longer needed to be declared in raises:[], maybe CancelledError could inherit from Exception, so that it will not need to be managed inside the list of async: (raises: []).

2. Modify asyncSpawn to not to raise Defect when a CancelledError is encountered. 99% of use cases (in our case) for needing to prevent cancellation propagation come from not wanting asyncSpawn to raise Defect. What other uses cases exist where a dev would want to stop cancellation propagation?

3. If the above two changes were applied, then we could allow the developer to control cancellations in the rare cases by adding bubbleCancellation: false to async pragma properties. IOW,

proc myProc() {.async: (bubbleCancellation=true, raises: []).} # default behaviour
proc myProc() {.async: (bubbleCancellation=false, raises: []).}
# both would transform to a return type of
proc myProc(): InternalRaisesFuture[void, void] {.stackTrace: false, raises: [], gcsafe.} =

The default behaviour is typically to propagate cancellations, so as a convenience to the dev, bubbleCancellations=true could be the default, such that:

proc myProc() {.async: (raises: []).}
# would be equivalent to
proc myProc() {.async: (bubbleCancellation=true, raises: []).}

Naive request?

This may be a naive request in that CancelledError may not be inheritable from Exception and also catchable at the same time. In that case, we could change it so that bubbleCancellation=true adds CancelledError to the InternalRaisesFuture, whereas bubbleCancellation=false would catch and discard CancelledError. IOW,

proc myProc() {.async: (bubbleCancellation = true, raises: []).}
# would transform to a return type of
proc myProc(): InternalRaisesFuture[void, (CancelledError)] {.stackTrace: false, raises: [], gcsafe.} =

whereas when bubbleCancellation=false:

proc myProc() {.async: (bubbleCancellation=false, raises: []).}
# would transform to a return type of
proc myProc(): InternalRaisesFuture[void, void] {.stackTrace: false, raises: [], gcsafe.} =
  try:
    # body
  except CancelledError:
    discard # do not propagate

This version would not allow us to have a default alias of

proc myProc() {.async: (raises: []).}

to be equivalent to:

proc myProc() {.async: (bubbleCancellation = true, raises: []).}

since the CancelledError would still need to be managed in the list of async: (raises: []) by the dev.

[arnetheduck]

Without addressing the other points, inheriting from anything but CatchableError is not well defined and was discarded as an idea in earlier conversations.

[cheatfate]

Just some comments from your example:
You should know that

proc myProc() {.async: raises: [].}

is not equal to

proc myProc() {.async.}

In first case procedure declared as not raising any exceptions, in second you are declaring raises: [CatchableError].

Problem you describing will disappear as soon as you start handling errors not in the way

try:
  ...
except CatchableError:
 ...

This approach will create pain for everything and for everybody and not only because of cancellation.

So declare exceptions you raising as YourNewAppException and use (async: raises). Derive all your exceptions from this one, handle all the stdlib exceptions right in place and you will see how easy is to handle CancelledError in such way. It will just disappear from your exception handlers...