diff options
author | Guido van Rossum <guido@python.org> | 2022-08-17 01:23:06 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-17 01:23:06 (GMT) |
commit | f51f54f39d384da63be622bcdc9cf4cfb43bad3d (patch) | |
tree | bdedc65a5f961e9aa330f7d8b929d8df0c5a4bb6 | |
parent | 9b30b965f0c1da216397b495faef4d93ff7a5328 (diff) | |
download | cpython-f51f54f39d384da63be622bcdc9cf4cfb43bad3d.zip cpython-f51f54f39d384da63be622bcdc9cf4cfb43bad3d.tar.gz cpython-f51f54f39d384da63be622bcdc9cf4cfb43bad3d.tar.bz2 |
GH-95704: Don't suppress errors from tasks when TG is cancelled (#95761)
When a task catches CancelledError and raises some other error,
the other error should not silently be suppressed.
Any scenario where a task crashes in cleanup upon cancellation
will now result in an ExceptionGroup wrapping the crash(es)
instead of propagating CancelledError and ignoring the side errors.
NOTE: This represents a change in behavior (hence the need to
change several tests). But it is only an edge case.
Co-authored-by: Thomas Grainger <tagrain@gmail.com>
-rw-r--r-- | Lib/asyncio/taskgroups.py | 7 | ||||
-rw-r--r-- | Lib/test/test_asyncio/test_taskgroups.py | 54 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst | 2 |
3 files changed, 35 insertions, 28 deletions
diff --git a/Lib/asyncio/taskgroups.py b/Lib/asyncio/taskgroups.py index 9be4838..5d5e2a8 100644 --- a/Lib/asyncio/taskgroups.py +++ b/Lib/asyncio/taskgroups.py @@ -116,10 +116,9 @@ class TaskGroup: if self._base_error is not None: raise self._base_error - if propagate_cancellation_error is not None: - # The wrapping task was cancelled; since we're done with - # closing all child tasks, just propagate the cancellation - # request now. + # Propagate CancelledError if there is one, except if there + # are other errors -- those have priority. + if propagate_cancellation_error and not self._errors: raise propagate_cancellation_error if et is not None and et is not exceptions.CancelledError: diff --git a/Lib/test/test_asyncio/test_taskgroups.py b/Lib/test/test_asyncio/test_taskgroups.py index 74bae06..6a0231f 100644 --- a/Lib/test/test_asyncio/test_taskgroups.py +++ b/Lib/test/test_asyncio/test_taskgroups.py @@ -230,29 +230,29 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase): self.assertEqual(NUM, 15) - async def test_cancellation_in_body(self): + async def test_taskgroup_08(self): async def foo(): - await asyncio.sleep(0.1) - 1 / 0 + try: + await asyncio.sleep(10) + finally: + 1 / 0 async def runner(): async with taskgroups.TaskGroup() as g: for _ in range(5): g.create_task(foo()) - try: - await asyncio.sleep(10) - except asyncio.CancelledError: - raise + await asyncio.sleep(10) r = asyncio.create_task(runner()) await asyncio.sleep(0.1) self.assertFalse(r.done()) r.cancel() - with self.assertRaises(asyncio.CancelledError) as cm: + with self.assertRaises(ExceptionGroup) as cm: await r + self.assertEqual(get_error_types(cm.exception), {ZeroDivisionError}) async def test_taskgroup_09(self): @@ -316,8 +316,10 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase): async def test_taskgroup_11(self): async def foo(): - await asyncio.sleep(0.1) - 1 / 0 + try: + await asyncio.sleep(10) + finally: + 1 / 0 async def runner(): async with taskgroups.TaskGroup(): @@ -325,24 +327,26 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase): for _ in range(5): g2.create_task(foo()) - try: - await asyncio.sleep(10) - except asyncio.CancelledError: - raise + await asyncio.sleep(10) r = asyncio.create_task(runner()) await asyncio.sleep(0.1) self.assertFalse(r.done()) r.cancel() - with self.assertRaises(asyncio.CancelledError): + with self.assertRaises(ExceptionGroup) as cm: await r + self.assertEqual(get_error_types(cm.exception), {ExceptionGroup}) + self.assertEqual(get_error_types(cm.exception.exceptions[0]), {ZeroDivisionError}) + async def test_taskgroup_12(self): async def foo(): - await asyncio.sleep(0.1) - 1 / 0 + try: + await asyncio.sleep(10) + finally: + 1 / 0 async def runner(): async with taskgroups.TaskGroup() as g1: @@ -352,19 +356,19 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase): for _ in range(5): g2.create_task(foo()) - try: - await asyncio.sleep(10) - except asyncio.CancelledError: - raise + await asyncio.sleep(10) r = asyncio.create_task(runner()) await asyncio.sleep(0.1) self.assertFalse(r.done()) r.cancel() - with self.assertRaises(asyncio.CancelledError): + with self.assertRaises(ExceptionGroup) as cm: await r + self.assertEqual(get_error_types(cm.exception), {ExceptionGroup}) + self.assertEqual(get_error_types(cm.exception.exceptions[0]), {ZeroDivisionError}) + async def test_taskgroup_13(self): async def crash_after(t): @@ -424,8 +428,9 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase): self.assertFalse(r.done()) r.cancel() - with self.assertRaises(asyncio.CancelledError): + with self.assertRaises(ExceptionGroup) as cm: await r + self.assertEqual(get_error_types(cm.exception), {ZeroDivisionError}) async def test_taskgroup_16(self): @@ -451,8 +456,9 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase): self.assertFalse(r.done()) r.cancel() - with self.assertRaises(asyncio.CancelledError): + with self.assertRaises(ExceptionGroup) as cm: await r + self.assertEqual(get_error_types(cm.exception), {ZeroDivisionError}) async def test_taskgroup_17(self): NUM = 0 diff --git a/Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst b/Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst new file mode 100644 index 0000000..31f9fc6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst @@ -0,0 +1,2 @@ +When a task catches :exc:`asyncio.CancelledError` and raises some other error,
+the other error should generally not silently be suppressed.
|