gh-96037: Always insert TimeoutError when exit an expired asyncio.timeout() block#113819
Conversation
…io.timeout() block If other exception was raised during exiting an expired asyncio.timeout() block, insert TimeoutError in the exception context just above the CancelledError.
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
I like this. Hopefully @iritkatriel does too. I got a little lost following the tests, hence my request for comments there. And there seems to be some debugging code "10" left?
| # Since there are no new cancel requests, we're | ||
| # handling this. | ||
| raise TimeoutError from exc_val | ||
| if exc_type is exceptions.CancelledError: |
There was a problem hiding this comment.
I don't think CancelledError is final. Maybe we should switch to isinstance()?
There was a problem hiding this comment.
Some code uses isinstance(), other code (in taskgroups.py and futures.py) uses is.
Are we going to backport this PR? If not, then perhaps it is better to change is to isinstance() in other PR before merging this PR. If yes, then we can include this change in this PR.
There was a problem hiding this comment.
I think it's a perf hack, but it makes things harder to reason about (what will happen if I raise a subclass of CancelledError? Who knows?).
I don't think we should backport this, such changes always trip some folks over.
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Cool. Go ahead and merge, let's not backport.
| # Since there are no new cancel requests, we're | ||
| # handling this. | ||
| raise TimeoutError from exc_val | ||
| if exc_type is exceptions.CancelledError: |
There was a problem hiding this comment.
I think it's a perf hack, but it makes things harder to reason about (what will happen if I raise a subclass of CancelledError? Who knows?).
I don't think we should backport this, such changes always trip some folks over.
…io.timeout() block (pythonGH-113819) If other exception was raised during exiting an expired asyncio.timeout() block, insert TimeoutError in the exception context just above the CancelledError.
…io.timeout() block (pythonGH-113819) If other exception was raised during exiting an expired asyncio.timeout() block, insert TimeoutError in the exception context just above the CancelledError.
…io.timeout() block (pythonGH-113819) If other exception was raised during exiting an expired asyncio.timeout() block, insert TimeoutError in the exception context just above the CancelledError.
3 participants
If other exception was raised during exiting an expired asyncio.timeout() block, insert TimeoutError in the exception context just above the CancelledError.