[FIXED] The proper way to use try/except blocks in tasks that can be cancelled

Issue

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?

Solution

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

Answered By - user4815162342