[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Guido van Rossum]

Guido van Rossum  added the comment:

Fixed by deprecating the message argument to cancel(). It will be removed in 
3.13.

--
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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Guido van Rossum]

Guido van Rossum  added the comment:


New changeset 0360e9f34659e7d7f3dae021b82f78452db8c714 by Andrew Svetlov in 
branch 'main':
bpo-46829: Deprecate passing a message into Future.cancel() and Task.cancel() 
(GH-31840)
https://github.com/python/cpython/commit/0360e9f34659e7d7f3dae021b82f78452db8c714


--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Yury Selivanov]

Yury Selivanov  added the comment:

IOW I think that supporting custom messages is a needless complication of our 
API. Given how complex task trees can become with TaskGroups collecting those 
messages and presenting them all to the user isn't trivial, showing just 
first/last defeats the purpose. So i'm in favor of dropping it.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Yury Selivanov]

Yury Selivanov  added the comment:

Andrew asked me for my opinion on the matter --  I think we should get rid of 
the message. Exception messages for "signals" can be easily lost if an 
exception was re-raised. If the reason of why something is being cancelled is 
important (in my experience it never is) it should be recorded elsewhere.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Andrew Svetlov]

Andrew Svetlov  added the comment:

If the cancellation message should be kept it needs improvements anyway, the 
single message doesn't work well with multiple `.cancel()` calls.

I can imagine a 'CancelledError(*msgs)' and 'raise exc.drop_msg(msg)' as a 
function equivalent of task cancellation counter and `.cancel()` / 
`.uncancel()` pairing. A similar to MultiError, some sort of.

The counter is easier to understand I guess.

Both multi-message and counter require new APIs, timeout() can be adapted to 
any solution.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Guido van Rossum]

Guido van Rossum  added the comment:

Before we land GH-31840 we should have a somewhat more public discussion (e.g. 
on python-dev or maybe in Async-SIG, https://discuss.python.org/c/async-sig/20; 
or at least here) about deprecating the cancel message. I'm all for it but 
certainly Chris Jerdonek (who wrote the original code, see bpo-31033) needs to 
have a say, and from a comment on GH-19951 it looks Yury Selivanov also really 
liked it.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Andrew Svetlov]

Change by Andrew Svetlov :


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

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

For reference, the msg parameter of Task.cancel() was added in issue31033.

It seems that the initial use case was for debugging. I do not see how it 
differs from the following example:

r = random.random()
if r < 0.5:
x = 0
else:
x = 0
1/x

In the traceback we see the line where an error occurred but we do not see a 
line which lead to this error.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Guido van Rossum]

Guido van Rossum  added the comment:

But that example is made-up. Is there a real-world situation where you need to 
know the call site, and it wouldn't be obvious from other log messages?

Directly cancelling a task without also somehow catching the cancellation (like 
in the timeout or task group cases) feels like an odd practice to me.

And passing the cancel message through is complex (as we've seen in recent PRs).

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Chris Jerdonek]

Chris Jerdonek  added the comment:

I don't really understand all the hate around the idea of a cancel message. One 
reason it's useful is that it provides a simple way to say *why* something is 
being cancelled or *what* is cancelling it. It provides additional context to 
the exception, in the same way that a normal exception's message can provide 
additional helpful context.

Take for example this chunk of code:

import asyncio
import random

async def job():
await asyncio.sleep(5)

async def main():
task = asyncio.create_task(job())
await asyncio.sleep(1)
r = random.random()
if r < 0.5:
task.cancel()
else:
task.cancel()
await task

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

Without passing a message, there's no way to tell which of the two lines called 
cancel, or why. The tracebacks are identical in both cases. This is even in the 
simplest case where only one cancellation is occurring. Passing a message lets 
one disambiguate the two call sites. And if there is truly a race condition 
where it's up in the air between two things cancelling it, I think it would be 
okay for the chosen message to be indeterminate, as either would have 
instigated the cancel in the absence of the other.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Andrew Svetlov]

Andrew Svetlov  added the comment:

Deprecation is a good answer. 
Let's not forget to apply it to 3.11 then.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Guido van Rossum]

Guido van Rossum  added the comment:

I would like to go on record (again) as preferring to get rid of the 
cancel-message parameter altogether. Let's deprecate it. When we initially 
designed things without a cancel message we did it on purpose -- "cancellation" 
is a single flag, not a collection of data.

--

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

What about `CancelledError(*msg_list)` or 
`CancelledError(*reversed(msg_list))`? It is backward compatible and all 
messages are uniformely represented.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue46829] Confusing CancelError message if multiple cancellations are scheduled

[Andrew Svetlov]

New submission from Andrew Svetlov :

Suppose multiple `task.cancel(msg)` with different messages are called on the 
same event loop iteration.

What message (`cancel_exc.args[0]`) should be sent on the next loop iteration?

As of Python 3.10 it is the message from the *last* `task.cancel(msg)` call. 
The main branch changed it to the *first* message (it is a subject for 
discussion still).
Both choices are equally bad. The order of tasks execution at the same loop 
iteration is weak and depends on very many circumstances. Effectively, 
first-cancelled-message and last-cancelled-message are equal to random-message.

This makes use of cancellation message not robust: a task can be cancelled by 
many sources, task-groups adds even more mess.

Guido van Rossum suggested that messages should be collected in a list and 
raised altogether. There is a possibility to do it in a backward-compatible 
way: construct the exception as `CancelledError(last_msg, tuple(msg_list))`. 
args[0] is args[1][-1]. Weird but works.

`.cancel()` should add `None` to the list of cancelled messages.
The message list should be cleared when a new CancelledError is constructed and 
thrown into cancelling task.

Working with exc.args[0] / exc.args[1] is tedious and error-prone. I propose 
adding `exc.msgs` property. Not sure if the last added message is worth a 
separate attribute, a robust code should not rely on messages order as I 
described above.

The single message is not very useful but a list of messages can be used in 
timeouts implementation as cancellation count alternative.

I don't have a strong preference now but want to carefully discuss possible 
opportunities before making the final decision.

--
components: asyncio
messages: 413749
nosy: ajoino, alex.gronholm, asvetlov, chris.jerdonek, dreamsorcerer, 
gvanrossum, iritkatriel, jab, njs, tinchester, yselivanov
priority: normal
severity: normal
status: open
title: Confusing CancelError message if multiple cancellations are scheduled
versions: Python 3.11

___
Python tracker 

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