[issue39027] run_coroutine_threadsafe uses wrong TimeoutError

[Nick Gaya]

Nick Gaya <nicholasgaya+github at gmail.com> added the comment:

@msg358296:

> IMO, it seems rather counter-intuitive to have to specify `concurrent.futures.TimeoutError` when using a timeout for the future returned by `run_coroutine_threadsafe()`.

I think that's expected, given that the function returns a `concurrent.futures.Future` object. The `Future.result()` method behaves the same regardless of the executor.

@avsetlov:

> futures._chain_future() should convert exceptions. Seems _convert_future_exc() does this work already but maybe it fails somehow. We need to investigate more.

The `_convert_future_exc()` only converts `concurrent.futures` exceptions to `asyncio` exceptions, not the reverse. I'm not sure if this is intentional or not.

With the current behavior there are two types of timeout errors that can occur in the example snippet:

* If the coroutine itself throws an `asyncio.exceptions.TimeoutError`, this will be propagated as-is to the `concurrent.futures.Future` object and thrown by `future.result()`.

* If the coroutine does not terminate within the timeout supplied to `future.result()`, then the method will throw a `concurrent.futures.TimeoutError` without changing the state of the future or the associated coroutine.

It's only necessary to cancel the future in the second case, as in the first case it's already in a finished state. So the example should catch `concurrent.futures.TimeoutError` rather than `asyncio.TimeoutError`.

----------
nosy: +nickgaya

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue39027>
_______________________________________