Fix cancellation being swallowed in tasks

[bdraco]

asyncio.CancelledError was being consumed in the task which means task.cancelled() can never be true

https://superfastpython.com/asyncio-task-cancellation-best-practices/#Consume_a_CancelledError_Inside_The_Task

Also nothing in this lib ever cares if the task is actually cancelled or not so its fine to close this PR as its only a problem if in the future something would check or care.

[mdonoughe]

This way of handling cancellation exceptions seems very un-pythonic.

SmartBridge cancels these internal tasks and discards them. If the tasks reraise the cancellation exception it will be a never-retrieved exception.

If the exception is caught and reraised like this instead of being ignored at the bottom of a task stack, nobody can observe it except for the never retrieved exception handler. If the cancellation is supposed to be the result of task returned from the connect function, it needs to be propagated like Exception instead of reraised.

[bdraco]

If the tasks reraise the cancellation exception it will be a never-retrieved exception.

I was under the impression that calling .cancel() turns off log_traceback

https://github.com/python/cpython/blob/0ff16115741aeaaaf7f963f68d5c575efb960277/Lib/asyncio/tasks.py#L205
https://github.com/python/cpython/blob/0ff16115741aeaaaf7f963f68d5c575efb960277/Lib/asyncio/futures.py#L95

[bdraco]

If cancellation is swallowed inside the task, task.cancelled() will never be true

Example

import asyncio
import gc
import logging

logging.basicConfig(level=logging.DEBUG)


async def test_task_that_swallowed_cancel():
    try:
        await asyncio.Event().wait()
    except asyncio.CancelledError:
        pass


async def swallow_test():
    task = asyncio.create_task(test_task_that_swallowed_cancel())
    await asyncio.sleep(0)
    task.cancel()
    assert task.cancelling()
    await asyncio.sleep(0)
    # cancelled will never be true because cancellation
    # was swallowed
    assert not task.cancelled()
    del task
    gc.collect()
    # Should not log future never retrieved


async def test_task_that_raises_cancel():
    try:
        await asyncio.Event().wait()
    except asyncio.CancelledError:
        raise


async def not_swallow_test():
    task = asyncio.create_task(test_task_that_raises_cancel())
    await asyncio.sleep(0)
    task.cancel()
    assert task.cancelling()
    await asyncio.sleep(0)
    # cancelled will be true because cancellation
    # was not swallowed
    assert task.cancelled()
    del task
    gc.collect()
    # Should not log future never retrieved


asyncio.run(swallow_test())
asyncio.run(not_swallow_test())

[bdraco]

I think this is a good description of the issue https://superfastpython.com/asyncio-task-cancellation-best-practices/#Consume_a_CancelledError_Inside_The_Task

[bdraco]

Also nothing ever cares if the task is actually cancelled or not so its fine to close this PR as its only a problem if in the future something would check or care.

[bdraco]

@mdonoughe Thanks for looking at the PRs. I'm fine with closing this one out as I don't think its strictly needed. I'm much more concerned with getting a release done so we can address @jonoberheide 's issue, which I think also means #176 can be closed out as well.