allow asyncio.CancelledError to bubble out of handle_exceptions

[embray]

in this case a JSON-RPC response will never be returned, but in the case of an RPC server task being cancelled while generating an RPC response we can't guarantee the response will be sent anyways

[embray]

FWIW another workaround is to check when calling async_dispatch if the response is an ExceptionResponse with exc of type CancelledError and re-raise it from the function which called async_dispatch. E.g.:

message = await websocket.recv()
response = await async_dispatch(message)
if isinstance(response, ExceptionResponse) as isinstance(response.exc, asyncio.CancelledError):
    raise response.exc
elif response.wanted:
    await websocket.send(str(response))

This workaround works for me; the only downside the the logging.exception() call in handle_exceptions still logs the CancelledError.

[bcb]

Hi @embray, can you tell me what is happening now that this PR fixes?

[embray]

Sure, here's an example that reproduces the problem (and I'm not saying this PR is necessarily the best solution).

Say you have a simple "RPC server" loop like:

>>> from jsonrpcserver import method, async_dispatch
>>> async def rpc_server(in_queue, out_queue):
...     while True:
...         message = await in_queue.get()
...         response = await async_dispatch(message)
...         print(response)
...         if response.wanted:
...             await out_queue.put(str(response))
... 

and a long-running method like:

>>> @method
... async def long_running_method():
...     await asyncio.sleep(100)
... 

Then given the following test code:

>>> from jsonrpcclient.requests import Request
>>> async def main():
...     in_queue = asyncio.Queue()
...     out_queue = asyncio.Queue()
...     # start the rpc_server as a task
...     rpc_server_task = asyncio.create_task(rpc_server(in_queue, out_queue))
...     # send an RPC call
...     await in_queue.put(str(Request('long_running_method')))
...     # here we might wait for a response on out_queue, but maybe time out
...     # before it finishes
...     await asyncio.sleep(10)
...     # some time later, while the request is still being handled, we try
...     # to cancel the server task and wait for it to cancel
...     rpc_server_task.cancel()
...     try:
...         print('waiting for server task to cancel')
...         await rpc_server_task  # never returns
...     except asyncio.CancelledError:
...         pass
...     print('done')
...
>>> asyncio.run(main())

results in the following output:

waiting for server task to cancel
ERROR:root:
Traceback (most recent call last):
  File "jsonrpcserver/dispatcher.py", line 123, in handle_exceptions
    yield handler
  File "jsonrpcserver/async_dispatcher.py", line 40, in safe_call
    lookup(methods, request.method), *request.args, **request.kwargs
  File "jsonrpcserver/async_dispatcher.py", line 34, in call
    return await validate_args(method, *args, **kwargs)(*args, **kwargs)
  File "<ipython-input-173-2416184294da>", line 3, in long_running_method
    await asyncio.sleep(100)
  File "/usr/lib/python3.7/asyncio/tasks.py", line 595, in sleep
    return await future
concurrent.futures._base.CancelledError
{"jsonrpc": "2.0", "error": {"code": -32000, "message": "Server error"}, "id": 9}
<hangs here, main() never returns>

This is because the CancelledError is caught and handled, resulting in a "Server error" response object from the dispatcher, but because the CancelledError doesn't bubble up the rpc_server task doesn't exit.

[st31ny]

I agree that we should not catch CancelledError — indeed since Python 3.8 CancelledError is a subclass of BaseException, so catching Exception won't catch CancelledError anymore. So, I guess it is the right approach to also avoid catching CancelledError in earlier versions of Python.

[embray]

@st31ny Thanks, I didn't know about that change in Python 3.8. It seems like a good call exactly to avoid this kind of problem.

[bcb]

Ok, I’ll merge this today

[bcb]

Thanks @embray !

[embray]

Thanks!