[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Guido van Rossum]

Guido van Rossum  added the comment:

We should really stop appending to a closed issue.

Anyway, raising ExceptionGroup is backwards incompatible, since "except 
CancelledError" wouldn't cancel the group.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Thomas Grainger]

Thomas Grainger  added the comment:

there could be multiple messages here

perhaps it could be:

```
finally:
# Must reacquire lock even if wait is cancelled
cancelled = []
while True:
try:
await self.acquire()
break
except exceptions.CancelledError as e:
cancelled.append(e)

if len(cancelled) > 1:
raise ExceptionGroup("Cancelled", cancelled)
if cancelled:
raise cancelled[0]
```

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Also Future.result() and Future.exception() can raise a CancelledError. So a 
CancelledError raised in a task may not contain a message passed to 
Task.cancel().

import asyncio
import random

async def main():
fut = asyncio.Future()
fut.cancel()
async def job():
if random.random() < 0.5:
await asyncio.sleep(2)
fut.result()
await asyncio.sleep(5)
task = asyncio.create_task(job())
await asyncio.sleep(1)
task.cancel("cancel task")
await task

asyncio.run(main())

You need to catch a CancelledError raised in a coroutine and re-raise a new 
CancelledError with the specified cancel message if the task was cancelled.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Andrew Svetlov]

Andrew Svetlov  added the comment:

Serhiy is right, Condition.wait() has the following code:

finally:
# Must reacquire lock even if wait is cancelled
cancelled = False
while True:
try:
await self.acquire()
break
except exceptions.CancelledError:
cancelled = True

if cancelled:
raise exceptions.CancelledError

It swallows CancelledError exceptions from waiters and re-raises CancelledError 
without the cancellation message.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Chris Jerdonek]

Chris Jerdonek  added the comment:

For future reference, with Andrew's change merged above, the traceback for the 
example snippet in my message above:
https://bugs.python.org/issue45390#msg403570
is now the following. Observe that (1) the call to sleep() continues to be 
present, but (2) without introducing two new intermediate CancelledErrors, 
which increase the verbosity of the traceback:

Traceback (most recent call last):
  File "/home/andrew/projects/cpython/exc_traceback.py", line 14, in 
asyncio.run(main())
^^^
  File "/home/andrew/projects/cpython/Lib/asyncio/runners.py", line 44, in run
return loop.run_until_complete(main)
   ^
  File "/home/andrew/projects/cpython/Lib/asyncio/base_events.py", line 640, in 
run_until_complete
return future.result()
   ^^^
  File "/home/andrew/projects/cpython/exc_traceback.py", line 11, in main
await task
^^
  File "/home/andrew/projects/cpython/exc_traceback.py", line 5, in job
await asyncio.sleep(5)
^^
  File "/home/andrew/projects/cpython/Lib/asyncio/tasks.py", line 619, in sleep
return await future
   
asyncio.exceptions.CancelledError: cancel job

(This is copied from Andrew's comment in the PR here:
https://github.com/python/cpython/pull/31383#issuecomment-1046822899 )

Serhiy, can you provide a sample snippet for your case with output, like I did 
in my message linked above?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Seems a CancelledError message can be lost also in Condition.wait().

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Andrew Svetlov]

Change by Andrew Svetlov :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Andrew Svetlov]

Andrew Svetlov  added the comment:


New changeset 4140bcb1cd76dec5cf8d398f4d0e86c438c987d0 by Andrew Svetlov in 
branch 'main':
bpo-45390: Propagate CancelledError's message from cancelled task to its 
awaiter (GH-31383)
https://github.com/python/cpython/commit/4140bcb1cd76dec5cf8d398f4d0e86c438c987d0


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Guido van Rossum]

Change by Guido van Rossum :


--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Chris Jerdonek]

Chris Jerdonek  added the comment:

Andrew, the approach I described would I feel be much better. It would result 
in more concise, less verbose tracebacks, as opposed to more verbose -- not 
just because the message won't be repeated, but also because it eliminates the 
unneeded creation of intermediate exceptions. It would also cause is checks to 
work, which is what we want since behaviorally it should be the same exception.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Marco Pagliaricci]

Marco Pagliaricci  added the comment:

Andrew,
many thanks for your time, solving this issue.
I think your solution is the best to fix this little problem and I agree
with you on backporting.

My Best Regards,
and thanks again.

Marco

On Thu, Feb 17, 2022 at 10:29 AM Andrew Svetlov 
wrote:

>
> Andrew Svetlov  added the comment:
>
> I have a pull request for the issue.
> It doesn't use `Future.set_exception()` but creates a new CancelledError()
> with propagated message.
> The result is the same, except raised exceptions are not comparable by
> `is` check.
> As a benefit, `_cancelled_exc` works with the patch, exc.__context__ is
> correctly set.
>
> The patch is not backported because it changes existing behavior a little.
> I'd like to avoid a situation when third-party code works with Python
> 3.11+, 3.10.3+, and 3.9.11+ only.
>
> --
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Andrew Svetlov]

Andrew Svetlov  added the comment:

I have a pull request for the issue.
It doesn't use `Future.set_exception()` but creates a new CancelledError() with 
propagated message.
The result is the same, except raised exceptions are not comparable by `is` 
check.
As a benefit, `_cancelled_exc` works with the patch, exc.__context__ is 
correctly set.

The patch is not backported because it changes existing behavior a little. I'd 
like to avoid a situation when third-party code works with Python 3.11+, 
3.10.3+, and 3.9.11+ only.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Andrew Svetlov]

Change by Andrew Svetlov :


--
keywords: +patch
pull_requests: +29531
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/31383

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Andrew Svetlov]

Change by Andrew Svetlov :


--
versions: +Python 3.11 -Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Chris Jerdonek]

Chris Jerdonek  added the comment:

Here's a simplification of Marco's snippet to focus the discussion.

import asyncio

async def job():
# raise RuntimeError('error!')
await asyncio.sleep(5)

async def main():
task = asyncio.create_task(job())
await asyncio.sleep(1)
task.cancel('cancel job')
await task

if __name__=="__main__":
asyncio.run(main())


Running this pre-Python 3.9 gives something like this--

Traceback (most recent call last):
  File "test.py", line 15, in 
asyncio.run(main())
  File "/.../python3.7/asyncio/runners.py", line 43, in run
return loop.run_until_complete(main)
  File "/.../python3.7/asyncio/base_events.py", line 579, in run_until_complete
return future.result()
concurrent.futures._base.CancelledError


Running this with Python 3.9+ gives something like the following. The 
difference is that the traceback now starts at the sleep() call:

Traceback (most recent call last):
  File "/.../test.py", line 6, in job
await asyncio.sleep(5)
  File "/.../python3.9/asyncio/tasks.py", line 654, in sleep
return await future
asyncio.exceptions.CancelledError: cancel job

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/.../test.py", line 12, in main
await task
asyncio.exceptions.CancelledError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/.../test.py", line 15, in 
asyncio.run(main())
  File "/.../python3.9/asyncio/runners.py", line 44, in run
return loop.run_until_complete(main)
  File "/.../python3.9/asyncio/base_events.py", line 642, in run_until_complete
return future.result()
asyncio.exceptions.CancelledError


Uncommenting the RuntimeError turns it into this--

Traceback (most recent call last):
  File "/.../test.py", line 15, in 
asyncio.run(main())
  File "/.../python3.9/asyncio/runners.py", line 44, in run
return loop.run_until_complete(main)
  File "/.../python3.9/asyncio/base_events.py", line 642, in run_until_complete
return future.result()
  File "/.../test.py", line 12, in main
await task
  File "/.../test.py", line 5, in job
raise RuntimeError('error!')
RuntimeError: error!


I agree it would be a lot nicer if the original CancelledError('cancel job') 
could bubble up just like the RuntimeError does, instead of creating a new 
CancelledError at each await and chaining it to the previous CancelledError. 
asyncio's creation of a new CancelledError at each stage predates the PR that 
added the chaining, so this could be viewed as an evolution of the change that 
added the chaining.

I haven't checked to be sure, but the difference in behavior between 
CancelledError and other exceptions might be explained by the following lines:
https://github.com/python/cpython/blob/3d1ca867ed0e3ae343166806f8ddd9739e568ab4/Lib/asyncio/tasks.py#L242-L250
You can see that for exceptions other than CancelledError, the exception is 
propagated by calling super().set_exception(exc), whereas with CancelledError, 
it is propagated by calling super().cancel() again.

Maybe this would even be an easy change to make. Instead of asyncio creating a 
new CancelledError and chaining it to the previous, asyncio can just raise the 
existing one. For the pure Python implementation at least, it may be as simple 
as making a change here, inside _make_cancelled_error():
https://github.com/python/cpython/blob/3d1ca867ed0e3ae343166806f8ddd9739e568ab4/Lib/asyncio/futures.py#L135-L142

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Marco Pagliaricci]

Marco Pagliaricci  added the comment:

Chris,
ok, I have modified the snippet of code to better show what I mean.
Still here, the message of the CancelledError exception is lost, but if I
comment line 10, and uncomment line 11, so I throw a ValueError("TEST"),
that "TEST" string will be printed, so the message is not lost.
Again, I just find this behavior very counter-intuitive, and should be VERY
WELL documented in the docs.

Thanks,
M.

On Sat, Oct 9, 2021 at 3:06 PM Chris Jerdonek 
wrote:

>
> Chris Jerdonek  added the comment:
>
> I still don't see you calling asyncio.Task.exception() in your new
> attachment...
>
> --
>
> ___
> Python tracker 
> 
> ___
>

--
Added file: https://bugs.python.org/file50334/task_bug.py

___
Python tracker 

___import asyncio


async def job():
print("job(): START...")
try:
await asyncio.sleep(5.0)
except asyncio.CancelledError as e:
print("job(): CANCELLED!", e)
raise asyncio.CancelledError("TEST")
#raise ValueError("TEST")
print("job(): DONE.")


async def cancel_task_after(task, time):
try:
await asyncio.sleep(time)
except asyncio.CancelledError:
print("cancel_task_after(): CANCELLED!")
except Exception as e:
print("cancel_task_after(): Exception!", e.__class__.__name__, 
e)
task.cancel("Hello!")


async def main():
task = asyncio.create_task(job())
# RUN/CANCEL task.
try:
await asyncio.gather(task, cancel_task_after(task, 1.0))
except asyncio.CancelledError as e:
try:
task_exc = task.exception()
except BaseException as be:
task_exc = be
print("In running task, we encountered a cancellation! 
Excpetion message is: ", e)
print("   ^ Task exc is:", task_exc.__class__.__name__, 
task_exc)
except Exception as e:
print("In running task, we got a generic Exception:", 
e.__class__.__name__, e)
# GET result.
try:
result = task.result()
except asyncio.CancelledError as e:
print("Task has been cancelled, exception message is: ", e)
except Exception as e:
try:
task_exc = task.exception()
except BaseException as be:
task_exc = be
print("Task raised generic exception", e.__class__.__name__, e)
print("  ^ Task exc is:", task_exc.__class__.__name__, task_exc)
else:
print("Task result is: ", result)


if __name__=="__main__":
asyncio.run(main())

___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Chris Jerdonek]

Chris Jerdonek  added the comment:

I still don't see you calling asyncio.Task.exception() in your new attachment...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Marco Pagliaricci]

Marco Pagliaricci  added the comment:

Chris,
I'm attaching to this e-mail the code I'm referring to.
As you can see, in line 10, I re-raise the asyncio.CancelledError exception
with a message "TEST".
That message is lost, due to the reasons we've talked about.

My point is that, if we substitute that line 10, with the commented line
11, and we comment the line 10, so we raise a ValueError("TEST") exception,
as you can see, the message "TEST" is NOT LOST.
I just find this counter-intuitive, and error-prone.

AT LEAST should be very well specified in the docs.

Regards,
M.

On Sat, Oct 9, 2021 at 2:51 PM Chris Jerdonek 
wrote:

>
> Chris Jerdonek  added the comment:
>
> > 2) Now: if I re-raise the asyncio.CancelledError as-is, I lose the
> message,
> if I call the `asyncio.Task.exception()` function.
>
> Re-raise asyncio.CancelledError where? (And what do you mean by
> "re-raise"?) Call asyncio.Task.exception() where? This isn't part of your
> example, so it's not clear what you mean exactly.
>
> --
>
> ___
> Python tracker 
> 
> ___
>

--
Added file: https://bugs.python.org/file50333/task_bug.py

___
Python tracker 

___import asyncio


async def job():
print("job(): START...")
try:
await asyncio.sleep(5.0)
except asyncio.CancelledError as e:
print("job(): CANCELLED!", e)
#raise asyncio.CancelledError("TEST")
raise ValueError("TEST")
print("job(): DONE.")


async def cancel_task_after(task, time):
try:
await asyncio.sleep(time)
except asyncio.CancelledError:
print("cancel_task_after(): CANCELLED!")
except Exception as e:
print("cancel_task_after(): Exception!", e.__class__.__name__, 
e)
task.cancel("Hello!")


async def main():
task = asyncio.create_task(job())
# RUN/CANCEL task.
try:
await asyncio.gather(task, cancel_task_after(task, 1.0))
except asyncio.CancelledError as e:
print("In running task, we encountered a cancellation! 
Excpetion message is: ", e)
except Exception as e:
print("In running task, we got a generic Exception:", 
e.__class__.__name__, e)
# GET result.
try:
result = task.result()
except asyncio.CancelledError as e:
print("Task has been cancelled, exception message is: ", e)
except Exception as e:
print("Task raised generic exception", e.__class__.__name__, e)
else:
print("Task result is: ", result)


if __name__=="__main__":
asyncio.run(main())

___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Chris Jerdonek]

Chris Jerdonek  added the comment:

> 2) Now: if I re-raise the asyncio.CancelledError as-is, I lose the message,
if I call the `asyncio.Task.exception()` function.

Re-raise asyncio.CancelledError where? (And what do you mean by "re-raise"?) 
Call asyncio.Task.exception() where? This isn't part of your example, so it's 
not clear what you mean exactly.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Marco Pagliaricci]

Marco Pagliaricci  added the comment:

OK, I see your point.
But I still can't understand one thing and I think it's very confusing:

1) if you see my example, inside the job() coroutine, I get correctly
cancelled with an `asyncio.CancelledError` exception containing my message.
2) Now: if I re-raise the asyncio.CancelledError as-is, I lose the message,
if I call the `asyncio.Task.exception()` function.
3) If I raise a *new* asyncio.CancelledError with a new message, inside the
job() coroutine's `except asyncio.CancelledError:` block, I still lose the
message if I call `asyncio.Task.exception()`.
4) But if I raise another exception, say `raise ValueError("TEST")`, always
from the `except asyncio.CancelledError:` block of the job() coroutine, I
*get* the message!
I get `ValueError("TEST")` by calling `asyncio.Task.exception()`, while I
don't with the `asyncio.CancelledError()` one.

Is this really wanted? Sorry, but I still find this a lot confusing.
Shouldn't it be better to return from the `asyncio.Task.exception()` the
old one (containing the message) ?
Or, otherwise, create a new instance of the exception for *ALL* the
exception classes?

Thank you for your time,
My Best Regards,

M.

On Thu, Oct 7, 2021 at 10:25 AM Thomas Grainger 
wrote:

>
> Thomas Grainger  added the comment:
>
> afaik this is intentional https://bugs.python.org/issue31033
>
> --
> nosy: +graingert
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Thomas Grainger]

Thomas Grainger  added the comment:

afaik this is intentional https://bugs.python.org/issue31033

--
nosy: +graingert

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Chris Jerdonek]

Chris Jerdonek  added the comment:

> But, once the asyncio.Task is cancelled, is impossible to retrieve that 
> original asyncio.CancelledError(msg) exception with the message, because it 
> seems that *a new* asyncio.CancelledError() [without the message] is raised 
> when asyncio.Task.result() or asyncio.Task.exception() methods are called.

You say it's "impossible", but isn't the message accessible via the exception 
chain (and visible in the traceback)? One benefit of not duplicating the 
message on the internal call to cancel() is that it makes it easier to pinpoint 
which CancelledError object is associated with the user's call to cancel(), and 
which is associated with the call done by asyncio internals, which is a 
different cancellation. Another benefit is that it prevents info from being 
duplicated in the traceback.

--
nosy: +chris.jerdonek

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Ben]

Ben  added the comment:

This seems to be present in both the Python implementation as well as the 
accelerated C _asyncio module.

It looks like that when a Task awaits a cancelled future,
the task itself is cancelled but the cancellation message is not propagated to 
the task.
https://github.com/python/cpython/blob/main/Lib/asyncio/tasks.py#L242

--
nosy: +bjs

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45390] asyncio.Task doesn't propagate CancelledError() exception correctly.

[Marco Pagliaricci]

New submission from Marco Pagliaricci :

I've spotted a little bug in how asyncio.CancelledError() exception is 
propagated inside an asyncio.Task.

Since python 3.9 the asyncio.Task.cancel() method has a new 'msg' parameter, 
that will create an asyncio.CancelledError(msg) exception incorporating that 
message.

The exception is successfully propagated to the coroutine the asyncio.Task is 
running, so the coroutine successfully gets raised an 
asyncio.CancelledError(msg) with the specified message in 
asyncio.Task.cancel(msg) method.

But, once the asyncio.Task is cancelled, is impossible to retrieve that 
original asyncio.CancelledError(msg) exception with the message, because it 
seems that *a new* asyncio.CancelledError() [without the message] is raised 
when asyncio.Task.result() or asyncio.Task.exception() methods are called.

I have the feeling that this is just wrong, and that the original message 
specified in asyncio.Task.cancel(msg) should be propagated even also 
asyncio.Task.result() is called.

I'm including a little snippet of code that clearly shows this bug.

I'm using python 3.9.6, in particular:
Python 3.9.6 (default, Aug 21 2021, 09:02:49) 
[GCC 10.2.1 20210110] on linux

--
components: asyncio
files: task_bug.py
messages: 403294
nosy: asvetlov, pagliaricci.m, yselivanov
priority: normal
severity: normal
status: open
title: asyncio.Task doesn't propagate CancelledError() exception correctly.
type: behavior
versions: Python 3.9
Added file: https://bugs.python.org/file50328/task_bug.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com