status-im / nim-chronos

Chronos - An efficient library for asynchronous programming
https://status-im.github.io/nim-chronos/docs/chronos
Apache License 2.0
352 stars 51 forks source link

What is the intended method for catching exceptions raised from asyncCheck'd Futures? #541

Closed PhilippMDoerner closed 1 month ago

PhilippMDoerner commented 1 month ago

Heyho! I'd have posted this in a Discussion section on this repo but it appears to not be enabled. So in order to ask the question I opened this issue. If this is undesired, please just close it and we can move this to the nim forum instead.

Introduction

While playing around with chronos in an attempt at a library I noticed that essentially any uncaught exception raised after an await block (so a block that gets executed on the dispatcher via runForever) gets transformed into a FutureDefect which will naturally immediately proceed to crash the thread the program is on.

Example

The below will raise FutureDefect and thus exit the program without being caught and executing echo "Done normally"

import chronos
proc asyncProc() {.async.} =
  await sleepAsync(10)
  raise newException(ValueError, "Bad stuff")

proc someProc() =
  asyncCheck asyncProc()

someProc()
try:
  runForever()
except CatchableError as e:
  echo "Caught a catchable error"

echo "Done normally"

The problem with that

I would like to be able to catch ValueError (or an exception containing it) outside of asyncProc, as "normal" errors such as this do not represent a sufficiently erroneous case to crash the thread to me.

However, Chronos escalating the ValueError to a Defect means I cannot do so (unless I catch Exception which is meh for obvious reasons).

Why do I want this?

I've been playing around with chronos as part of a multithreading library that I am writing (built on top of taskpools).

What I would like to enable is essentially a server running on a single thread with its own while loop.

The loop essentially consists of:

That looks roughly like this:

proc processMessages(server: ServerActor) =
  ... loads of code ...
  for msg in mailbox.messages:
    process(msg) # <-- User defined proc! May call an async-proc via: asyncCheck anAsyncProc()

proc runServerLoop(server: ServerActor) {.gcsafe.} =  
  block serverLoop: 
    while isRunning():
      {.gcsafe}: 
        try:
          if not server.hasMessages():
            server.waitForSendSignal()

          server.processMessages()

        except CatchableError as e:
          error "Message caused exception", error = e[]

Users may define process procs to handle individual message types. I would like to basically save my users from themselves if possible. If their process proc throws an Exception and they do not deal with it, then I want globally deal with it (essentially catch it, log it and throw it away). I do not want it to bring the entire Thread down as it currently happens due to FutureDefect.

So my question is: For these kinds of "Global error-catching mechanisms" that I want - What is the ideal/intended way to do those? Is it maybe even impossible because of the exception?

arnetheduck commented 1 month ago

you can:

proc process() {.async.} =
  raise (ref ValueError)()

proc ignoreExceptions(fut: Future[void]): Future[void] {.async.} =
  try:
    await fut
  except CatchableError as exc:
    debugEcho "fail"

asyncSpawn ignoreExceptions process()

asyncSpawn/asyncCheck exist to deliberately check for uncaught exceptions - just like raising in a thread without catching would terminate the program. discard process(...) will also make chronos ignore exceptions though it's poor style.

arnetheduck commented 1 month ago

if the user uses asyncSpawn, that's nothing you can control - there's no way to prevent that from raising a Defect - this is by design, the program would be in an inconsistent state at that point (because it would try to raise an exception through the processing loop which leads to resource leaks et al. This is a contributing reason why asyncdispatch has so many leaks and problems for example.

PhilippMDoerner commented 1 month ago

Check, so this sentence of mine is incorrect:

I would like to be able to catch ValueError (or an exception containing it) outside of asyncProc, as "normal" errors such as this do not represent a sufficiently erroneous case to crash the thread to me.

Exceptions thrown by running the dispatcher instead of by some awaited future are a sufficiently erroneous case as you pointed out, due to potential resource leaks etc.

If I want to protect the user therefore my best choice is likely to mandate them not raise an exception, e.g via using raises: []

arnetheduck commented 1 month ago

mandate them not raise an exception,

indeed - it is likely asyncSpawn will start showing a compile-time warning whenever someone uses it without raises: []. asyncCheck is deprecated already for similar reasons.