1

In essence my question is when and where is the asyncio.CancelledError exception raised in the coroutine being cancelled?

I have an application with a couple of async tasks that run in a loop. At some point I start those tasks like this:

async def connect(self);
    ...
    t1 = asyncio.create_tasks(task1())
    t2 = asyncio.create_task(task2())
    ...
    self._workers = [t1, t2, ...]

When disconnecting, I cancel the tasks like this:

async def disconnect(self):
    for task in self._workers:
        task.cancel()

This has been working fine. The documentation of Task.cancel says

The coroutine then has a chance to clean up or even deny the request by suppressing the exception with a try … … except CancelledError … finally block. Therefore, unlike Future.cancel(), Task.cancel() does not guarantee that the Task will be cancelled, although suppressing cancellation completely is not common and is actively discouraged.

so in my workers I avoid doing stuff like this:

async def worker():
    while True:
        ...
        try:
            some work
        except:
            continue

but that means that now I have to explicitly put asyncio.CancelledError in the except statement:

async def worker():
    while True:
        ...
        try:
            some work
        except asyncio.CancelledError:
            raise
        except:
            continue

which can be tedious and I also have to make sure that anything that I call from my worker obliges by this rule.

So now I'm not sure if this is a good practice at all. Now that I'm thinking about it, I don't even know when exactly the exception is raised. I was searching for a similar case here in SO and found this question which also raised the same question "When will this exception be thrown? And where?". The answer says

This exception is thrown after task.cancel() is called. It is thrown inside the coroutine, where it is caught in the example, and it is then re-raised to be thrown and caught in the awaiting routine.

And while it make sense, this got me thinking: this is async scheduling, the tasks are not interrupted at any arbitrary place like with threads but they only "give back control" to the event loop when a task does an await. Right? So that means that checking everywhere whether asyncio.CancelledError was raised might not be necessary. For example, let's consider this example:

def worker(interval=1):
    while True:
        try:
            # doing some work and no await is called in this block
            sync_call1()
            sync_call2()
            sync_call3()
        except asyncio.CancelledError:
            raise
        except:
            # deal with error
            pass

        await asyncio.sleep(interval)

So I think here the except asyncio.CancelledError is unnecessary because this error cannot "physically" be raised in the try/block at all since the thread in the try block will never be interrupted by the event loop. The only place where this task gives back the control to the event loop is at the sleep call, that is not even in a try/block and hence it doesn't suppress the exception. Is my train of though correct? If so, does that mean that I only have to account for asyncio.CancelledError when I have an await in the try block? So would this also be OK, knowing that worker() can be cancelled?

def worker(interval=1):
    while True:
        try:
            # doing some work and no await is called in this block
            sync_call1()
            sync_call2()
            sync_call3()
        except:
            # deal with error
            pass

        await asyncio.sleep(interval)

And after reading the answer of the other SO question, I think I should also wait for the cancelled tasks in my disconnect() function, do I? Like this?

async def disconnect(self):
    for task in self._workers:
        task.cancel()

    await asyncio.gather(*self._workers)

Is this correct?

Pablo
  • 13,271
  • 4
  • 39
  • 59
  • I think so. `CancelledException` is raised in the `await` expression so `CancelledException` is not raised (BTW, wildcard catch is not good). Besides, `await` tasks being cancelled will raise `CancelledException`. `asyncio.wait` or loop them one by one to sure the tasks all done before return. – Aaron Jun 08 '20 at 19:08
  • @Aaron aha, ok, thanks. So it would be better to wrap `await asyncio.gather(*self._workers)` in `try/block` so that `disconnect` doesn't raise an exception. That make sense. – Pablo Jun 08 '20 at 19:15
  • 1
    for `asyncio.gather`, the `return_exceptions=True` argument is needed or some tasks may be still scheduled. – Aaron Jun 08 '20 at 19:19
  • @Aaron nice, that makes sense. – Pablo Jun 08 '20 at 19:24
  • I would point out that `try: ... except: continue` is an anti-pattern. You should always catch a more specific exception. If you do catch all exceptions, that should be only to perform some cleanup/logging before re-raising it. If you do so, you won't have a problem with `CancelledError`. Also, in Python 3.8 `CancelledError` derives from `BaseException`, which means `except Exception` won't catch it, also resolving the issue. – user4815162342 Jun 08 '20 at 20:11
  • @user4815162342 thanks for the suggestion. Because my worker task runs in endless loop, if some minor step fails (aka throws an exception) I'd like to "ok, ignore this cycle, let's start a new one), that's why I have the `try: ... except: continue` there. How would you solve this? Simply with `except Exception: continue`? Thanks for the tip of `CancelledError` deriving from `BaseException`. Is this only in py3.8 or does it also apply to py3.7 and py3.6 as well? – Pablo Jun 08 '20 at 20:17
  • In Python 3.8 and later you can use `except Exception: continue`, but I'd still log the exception before continuing rather than swallowing it silently. In Python 3.7 and earlier you need to use the pattern you discovered yourself. Your analysis is correct in that if the code you see doesn't contain an awaiting construct, you can't get a `CancelledError` (at least not from `task.cancel`; someone could still raise it manually, but then you probably _want_ to treat is as any other exception). Note that awaiting constructs include `await`, `async for` and `async with`. – user4815162342 Jun 08 '20 at 20:30
  • 1
    I've now added these comments as an answer. – user4815162342 Jun 10 '20 at 20:27

1 Answers1

2

Your reasoning is correct: if the code doesn't contain an awaiting construct, you can't get a CancelledError (at least not from task.cancel; someone could still raise it manually, but then you probably want to treat is as any other exception). Note that awaiting constructs include await, async for and async with.

Having said that, I would add that try: ... except: continue is an anti-pattern. You should always catch a more specific exception. If you do catch all exceptions, that should be only to perform some cleanup/logging before re-raising it. If you do so, you won't have a problem with CancelledError. If you absolutely must catch all exceptions, consider at least logging the fact that an exception was raised, so that it doesn't pass silently.

Python 3.8 made it much easier to catch exceptions other than CancelledError because it switched to deriving CancelledError from BaseException. In 3.8 except Exception won't catch it, resolving your issue.

To sum it up:

  • If you run Python 3.8 and later, use except Exception: traceback.print_exc(); continue.

  • In Python 3.7 and earlier you need to use the pattern put forth in the question. If it's a lot of typing, you can abstract it into a function, but that will still require some refactoring.

For example, you could define a utility function like this:

def run_safe(thunk):
    try:
        thunk()
        return True
    except asyncio.CancelledError:
        raise
    except:
        traceback.print_exc()
        return False
user4815162342
  • 141,790
  • 18
  • 296
  • 355
  • 1
    Thank you for your suggestions. It's a shame that `CancelledError` is based on `BaseException` from py3.8 onwards, I still have to support running in py36 and py37, so I have to do the long way. – Pablo Jun 10 '20 at 22:15