From 36c114ab11d478c9ede246035606c12fc080e6ff Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Wed, 17 Aug 2022 10:04:59 -0700 Subject: GH-95704: Don't suppress errors from tasks when TG is cancelled (GH-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 (cherry picked from commit f51f54f39d384da63be622bcdc9cf4cfb43bad3d) Co-authored-by: Guido van Rossum --- Lib/asyncio/taskgroups.py | 7 ++- Lib/test/test_asyncio/test_taskgroups.py | 54 ++++++++++++---------- .../2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst | 2 + 3 files changed, 35 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst 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. -- cgit v0.12