From 458fda07eb3544a478aed77e91bd9ea58274adde Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 8 Jan 2024 19:38:53 +0200 Subject: [PATCH 1/2] gh-96037: Always insert TimeoutError when exit an expired asyncio.timeout() block If other exception was raised during exiting an expired asyncio.timeout() block, insert TimeoutError in the exception context just above the CancelledError. --- Lib/asyncio/timeouts.py | 20 +++- Lib/test/test_asyncio/test_timeouts.py | 103 ++++++++++++++++-- ...4-01-08-19-38-42.gh-issue-96037.Yr2Y1C.rst | 2 + 3 files changed, 112 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-01-08-19-38-42.gh-issue-96037.Yr2Y1C.rst diff --git a/Lib/asyncio/timeouts.py b/Lib/asyncio/timeouts.py index 30042abb3ad804..fc432ff1823ae2 100644 --- a/Lib/asyncio/timeouts.py +++ b/Lib/asyncio/timeouts.py @@ -109,10 +109,16 @@ async def __aexit__( if self._state is _State.EXPIRING: self._state = _State.EXPIRED - if self._task.uncancel() <= self._cancelling and exc_type is exceptions.CancelledError: + if self._task.uncancel() <= self._cancelling: # Since there are no new cancel requests, we're # handling this. - raise TimeoutError from exc_val + if exc_type is exceptions.CancelledError: + raise TimeoutError from exc_val + elif 10 and exc_val is not None: + self._insert_timeout_error(exc_val) + if isinstance(exc_val, ExceptionGroup): + for exc in exc_val.exceptions: + self._insert_timeout_error(exc) elif self._state is _State.ENTERED: self._state = _State.EXITED @@ -125,6 +131,16 @@ def _on_timeout(self) -> None: # drop the reference early self._timeout_handler = None + @staticmethod + def _insert_timeout_error(exc_val: BaseException) -> None: + while exc_val.__context__ is not None: + if type(exc_val.__context__) is exceptions.CancelledError: + te = TimeoutError() + te.__context__ = te.__cause__ = exc_val.__context__ + exc_val.__context__ = te + break + exc_val = exc_val.__context__ + def timeout(delay: Optional[float]) -> Timeout: """Timeout async context manager. diff --git a/Lib/test/test_asyncio/test_timeouts.py b/Lib/test/test_asyncio/test_timeouts.py index f54e79e4d8e600..ccbb43991bce90 100644 --- a/Lib/test/test_asyncio/test_timeouts.py +++ b/Lib/test/test_asyncio/test_timeouts.py @@ -8,6 +8,9 @@ from test.test_asyncio.utils import await_without_task +class CustomError(Exception): + pass + def tearDownModule(): asyncio.set_event_loop_policy(None) @@ -116,15 +119,57 @@ async def test_foreign_exception_passed(self): raise KeyError self.assertFalse(cm.expired()) + async def test_timeout_exception_context(self): + with self.assertRaises(TimeoutError) as cm: + async with asyncio.timeout(0.01): + try: + 1/0 + finally: + await asyncio.sleep(1) + e = cm.exception + self.assertIsInstance(e.__context__, asyncio.CancelledError) + self.assertIs(e.__cause__, e.__context__) + self.assertIsInstance(e.__context__.__context__, ZeroDivisionError) + self.assertIsNone(e.__context__.__cause__) + async def test_foreign_exception_on_timeout(self): async def crash(): try: await asyncio.sleep(1) finally: 1/0 - with self.assertRaises(ZeroDivisionError): + with self.assertRaises(ZeroDivisionError) as cm: async with asyncio.timeout(0.01): await crash() + e = cm.exception + self.assertIsInstance(e.__context__, TimeoutError) + self.assertIsInstance(e.__context__.__context__, asyncio.CancelledError) + self.assertIs(e.__context__.__cause__, e.__context__.__context__) + self.assertIsNone(e.__cause__) + + async def test_foreign_exception_on_timeout_2(self): + with self.assertRaises(ZeroDivisionError) as cm: + async with asyncio.timeout(0.01): + try: + try: + raise ValueError + finally: + await asyncio.sleep(1) + finally: + try: + raise KeyError + finally: + 1/0 + e = cm.exception + self.assertIsInstance(e.__context__, KeyError) + e2 = e.__context__.__context__ + self.assertIsInstance(e2, TimeoutError) + self.assertIsInstance(e2.__context__, asyncio.CancelledError) + self.assertIsInstance(e2.__context__.__context__, ValueError) + self.assertIsNone(e2.__context__.__cause__) + self.assertIsNone(e.__context__.__cause__) + self.assertIs(e2.__cause__, e2.__context__) + self.assertIsNone(e.__cause__) async def test_foreign_cancel_doesnt_timeout_if_not_expired(self): with self.assertRaises(asyncio.CancelledError): @@ -219,14 +264,24 @@ async def test_repr_disabled(self): self.assertEqual(repr(cm), r"") async def test_nested_timeout_in_finally(self): - with self.assertRaises(TimeoutError): + with self.assertRaises(TimeoutError) as cm1: async with asyncio.timeout(0.01): try: await asyncio.sleep(1) finally: - with self.assertRaises(TimeoutError): + with self.assertRaises(TimeoutError) as cm2: async with asyncio.timeout(0.01): await asyncio.sleep(10) + e1 = cm1.exception + self.assertIsInstance(e1.__context__, asyncio.CancelledError) + self.assertIsNone(e1.__context__.__context__) + self.assertIsNone(e1.__context__.__cause__) + self.assertIs(e1.__cause__, e1.__context__) + e2 = cm2.exception + self.assertIsInstance(e2.__context__, asyncio.CancelledError) + self.assertIs(e2.__context__.__context__, e1.__context__) + self.assertIsNone(e2.__context__.__cause__) + self.assertIs(e2.__cause__, e2.__context__) async def test_timeout_after_cancellation(self): try: @@ -235,7 +290,7 @@ async def test_timeout_after_cancellation(self): except asyncio.CancelledError: pass finally: - with self.assertRaises(TimeoutError): + with self.assertRaises(TimeoutError) as cm: async with asyncio.timeout(0.0): await asyncio.sleep(1) # some cleanup @@ -251,13 +306,6 @@ async def test_cancel_in_timeout_after_cancellation(self): asyncio.current_task().cancel() await asyncio.sleep(2) # some cleanup - async def test_timeout_exception_cause (self): - with self.assertRaises(asyncio.TimeoutError) as exc: - async with asyncio.timeout(0): - await asyncio.sleep(1) - cause = exc.exception.__cause__ - assert isinstance(cause, asyncio.CancelledError) - async def test_timeout_already_entered(self): async with asyncio.timeout(0.01) as cm: with self.assertRaisesRegex(RuntimeError, "has already been entered"): @@ -303,6 +351,39 @@ async def test_timeout_without_task(self): with self.assertRaisesRegex(RuntimeError, "has not been entered"): cm.reschedule(0.02) + async def test_timeout_taskgroup(self): + async def task(): + try: + await asyncio.sleep(2) # Will be interrupted after 0.01 second + finally: + 1/0 # Crash in cleanup + + with self.assertRaises(ExceptionGroup) as cm: + async with asyncio.timeout(0.01): + async with asyncio.TaskGroup() as tg: + tg.create_task(task()) + try: + raise ValueError + finally: + await asyncio.sleep(1) + eg = cm.exception + self.assertIsInstance(eg.__context__, TimeoutError) + self.assertIsInstance(eg.__context__.__context__, asyncio.CancelledError) + self.assertIsInstance(eg.__context__.__context__.__context__, ValueError) + self.assertIsNone(eg.__context__.__context__.__cause__) + self.assertIs(eg.__context__.__cause__, eg.__context__.__context__) + self.assertIsNone(eg.__cause__) + + self.assertEqual(len(eg.exceptions), 1, eg) + e = eg.exceptions[0] + self.assertIsInstance(e, ZeroDivisionError) + self.assertIsInstance(e.__context__, TimeoutError) + self.assertIsInstance(e.__context__.__context__, asyncio.CancelledError) + self.assertIsNone(e.__context__.__context__.__context__) + self.assertIsNone(e.__context__.__context__.__cause__) + self.assertIs(e.__context__.__cause__, e.__context__.__context__) + self.assertIsNone(e.__cause__) + if __name__ == '__main__': unittest.main() diff --git a/Misc/NEWS.d/next/Library/2024-01-08-19-38-42.gh-issue-96037.Yr2Y1C.rst b/Misc/NEWS.d/next/Library/2024-01-08-19-38-42.gh-issue-96037.Yr2Y1C.rst new file mode 100644 index 00000000000000..525925b08230ed --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-01-08-19-38-42.gh-issue-96037.Yr2Y1C.rst @@ -0,0 +1,2 @@ +Insert :exc:`TimeoutError` in the context of the exception that was raised +during exiting an expired :func:`asyncio.timeout` block. From 4fc39e122ad50b0602b2d9b4768fedb3317f5a12 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 8 Jan 2024 22:35:53 +0200 Subject: [PATCH 2/2] Address review comments. --- Lib/asyncio/timeouts.py | 2 +- Lib/test/test_asyncio/test_timeouts.py | 100 +++++++++++++++---------- 2 files changed, 62 insertions(+), 40 deletions(-) diff --git a/Lib/asyncio/timeouts.py b/Lib/asyncio/timeouts.py index fc432ff1823ae2..caecf6882854e2 100644 --- a/Lib/asyncio/timeouts.py +++ b/Lib/asyncio/timeouts.py @@ -114,7 +114,7 @@ async def __aexit__( # handling this. if exc_type is exceptions.CancelledError: raise TimeoutError from exc_val - elif 10 and exc_val is not None: + elif exc_val is not None: self._insert_timeout_error(exc_val) if isinstance(exc_val, ExceptionGroup): for exc in exc_val.exceptions: diff --git a/Lib/test/test_asyncio/test_timeouts.py b/Lib/test/test_asyncio/test_timeouts.py index ccbb43991bce90..1f7f9ee696a525 100644 --- a/Lib/test/test_asyncio/test_timeouts.py +++ b/Lib/test/test_asyncio/test_timeouts.py @@ -8,9 +8,6 @@ from test.test_asyncio.utils import await_without_task -class CustomError(Exception): - pass - def tearDownModule(): asyncio.set_event_loop_policy(None) @@ -127,10 +124,13 @@ async def test_timeout_exception_context(self): finally: await asyncio.sleep(1) e = cm.exception - self.assertIsInstance(e.__context__, asyncio.CancelledError) - self.assertIs(e.__cause__, e.__context__) - self.assertIsInstance(e.__context__.__context__, ZeroDivisionError) - self.assertIsNone(e.__context__.__cause__) + # Expect TimeoutError caused by CancelledError raised during handling + # of ZeroDivisionError. + e2 = e.__cause__ + self.assertIsInstance(e2, asyncio.CancelledError) + self.assertIs(e.__context__, e2) + self.assertIsNone(e2.__cause__) + self.assertIsInstance(e2.__context__, ZeroDivisionError) async def test_foreign_exception_on_timeout(self): async def crash(): @@ -142,10 +142,14 @@ async def crash(): async with asyncio.timeout(0.01): await crash() e = cm.exception - self.assertIsInstance(e.__context__, TimeoutError) - self.assertIsInstance(e.__context__.__context__, asyncio.CancelledError) - self.assertIs(e.__context__.__cause__, e.__context__.__context__) + # Expect ZeroDivisionError raised during handling of TimeoutError + # caused by CancelledError. self.assertIsNone(e.__cause__) + e2 = e.__context__ + self.assertIsInstance(e2, TimeoutError) + e3 = e2.__cause__ + self.assertIsInstance(e3, asyncio.CancelledError) + self.assertIs(e2.__context__, e3) async def test_foreign_exception_on_timeout_2(self): with self.assertRaises(ZeroDivisionError) as cm: @@ -161,15 +165,19 @@ async def test_foreign_exception_on_timeout_2(self): finally: 1/0 e = cm.exception - self.assertIsInstance(e.__context__, KeyError) - e2 = e.__context__.__context__ - self.assertIsInstance(e2, TimeoutError) - self.assertIsInstance(e2.__context__, asyncio.CancelledError) - self.assertIsInstance(e2.__context__.__context__, ValueError) - self.assertIsNone(e2.__context__.__cause__) - self.assertIsNone(e.__context__.__cause__) - self.assertIs(e2.__cause__, e2.__context__) + # Expect ZeroDivisionError raised during handling of KeyError + # raised during handling of TimeoutError caused by CancelledError. self.assertIsNone(e.__cause__) + e2 = e.__context__ + self.assertIsInstance(e2, KeyError) + self.assertIsNone(e2.__cause__) + e3 = e2.__context__ + self.assertIsInstance(e3, TimeoutError) + e4 = e3.__cause__ + self.assertIsInstance(e4, asyncio.CancelledError) + self.assertIsNone(e4.__cause__) + self.assertIsInstance(e4.__context__, ValueError) + self.assertIs(e3.__context__, e4) async def test_foreign_cancel_doesnt_timeout_if_not_expired(self): with self.assertRaises(asyncio.CancelledError): @@ -273,15 +281,21 @@ async def test_nested_timeout_in_finally(self): async with asyncio.timeout(0.01): await asyncio.sleep(10) e1 = cm1.exception - self.assertIsInstance(e1.__context__, asyncio.CancelledError) - self.assertIsNone(e1.__context__.__context__) - self.assertIsNone(e1.__context__.__cause__) - self.assertIs(e1.__cause__, e1.__context__) + # Expect TimeoutError caused by CancelledError. + e12 = e1.__cause__ + self.assertIsInstance(e12, asyncio.CancelledError) + self.assertIsNone(e12.__cause__) + self.assertIsNone(e12.__context__) + self.assertIs(e1.__context__, e12) e2 = cm2.exception - self.assertIsInstance(e2.__context__, asyncio.CancelledError) - self.assertIs(e2.__context__.__context__, e1.__context__) - self.assertIsNone(e2.__context__.__cause__) - self.assertIs(e2.__cause__, e2.__context__) + # Expect TimeoutError caused by CancelledError raised during + # handling of other CancelledError (which is the same as in + # the above chain). + e22 = e2.__cause__ + self.assertIsInstance(e22, asyncio.CancelledError) + self.assertIsNone(e22.__cause__) + self.assertIs(e22.__context__, e12) + self.assertIs(e2.__context__, e22) async def test_timeout_after_cancellation(self): try: @@ -367,22 +381,30 @@ async def task(): finally: await asyncio.sleep(1) eg = cm.exception - self.assertIsInstance(eg.__context__, TimeoutError) - self.assertIsInstance(eg.__context__.__context__, asyncio.CancelledError) - self.assertIsInstance(eg.__context__.__context__.__context__, ValueError) - self.assertIsNone(eg.__context__.__context__.__cause__) - self.assertIs(eg.__context__.__cause__, eg.__context__.__context__) + # Expect ExceptionGroup raised during handling of TimeoutError caused + # by CancelledError raised during handling of ValueError. self.assertIsNone(eg.__cause__) + e_1 = eg.__context__ + self.assertIsInstance(e_1, TimeoutError) + e_2 = e_1.__cause__ + self.assertIsInstance(e_2, asyncio.CancelledError) + self.assertIsNone(e_2.__cause__) + self.assertIsInstance(e_2.__context__, ValueError) + self.assertIs(e_1.__context__, e_2) self.assertEqual(len(eg.exceptions), 1, eg) - e = eg.exceptions[0] - self.assertIsInstance(e, ZeroDivisionError) - self.assertIsInstance(e.__context__, TimeoutError) - self.assertIsInstance(e.__context__.__context__, asyncio.CancelledError) - self.assertIsNone(e.__context__.__context__.__context__) - self.assertIsNone(e.__context__.__context__.__cause__) - self.assertIs(e.__context__.__cause__, e.__context__.__context__) - self.assertIsNone(e.__cause__) + e1 = eg.exceptions[0] + # Expect ZeroDivisionError raised during handling of TimeoutError + # caused by CancelledError (it is a different CancelledError). + self.assertIsInstance(e1, ZeroDivisionError) + self.assertIsNone(e1.__cause__) + e2 = e1.__context__ + self.assertIsInstance(e2, TimeoutError) + e3 = e2.__cause__ + self.assertIsInstance(e3, asyncio.CancelledError) + self.assertIsNone(e3.__context__) + self.assertIsNone(e3.__cause__) + self.assertIs(e2.__context__, e3) if __name__ == '__main__':