From: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> Date: Wed, 17 Aug 2022 17:04:59 +0000 (-0700) Subject: GH-95704: Don't suppress errors from tasks when TG is cancelled (GH-95761) X-Git-Tag: v3.11.0rc2~103 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=36c114ab11d478c9ede246035606c12fc080e6ff;p=thirdparty%2FPython%2Fcpython.git 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 --- diff --git a/Lib/asyncio/taskgroups.py b/Lib/asyncio/taskgroups.py index 9be4838e3c7a..5d5e2a8a85dd 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 74bae06af8e7..6a0231f2859a 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 000000000000..31f9fc6547d9 --- /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.