Skip to content

Add exception message for easier debugging with Cancelled #3232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Zac-HD opened this issue Mar 27, 2025 · 7 comments · May be fixed by #3256
Open

Add exception message for easier debugging with Cancelled #3232

Zac-HD opened this issue Mar 27, 2025 · 7 comments · May be fixed by #3256

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Mar 27, 2025

Occasionally we end up digging through logs trying to work out why something was cancelled, and eventually it occurred to me that Trio could help with this. I don't have a complete design proposal, but it would be great to handle each of three cases:

  • a deadline was reached.
  • a sibling task in the Nursery raised an exception, including the name of the erroring task
  • someone called CancelScope.cancel(), including the name of the task in which this was called
    • add a new reason: str | None = None param, so that users can explicitly annotate why they cancelled something

I think we could represent this as a string argument to Cancelled like any other exception message; the only tricky part will be threading it through all the cancellation-delivery machinery 🙂

Comments and design input most welcome, of course!

@TeamSpen210
Copy link
Contributor

Might be better to not use a string argument, instead pass like an enum + task name, then have __str__ build the arg. That way it's introspectable if need be, and it might be better performant since it doesn't need to build the string 99% of the time.

@A5rocks
Copy link
Contributor

A5rocks commented Mar 27, 2025

I'm not so sure being introspectable is that important for figuring out why something was cancelled. Probably being more varied in what information we show (setting __context__ for case 2? Maybe stitching together more stack frames? I'm not sure.) is better especially for debugging. I guess that doesn't detract from providing this as an enum though.

@Zac-HD
Copy link
Member Author

Zac-HD commented Mar 27, 2025

I'm somewhat inclined against setting __context__, because it's so easy for this to be lost/overwritten as exceptions are caught and re-raised.

An example usecase here is "I have dozens of Cancelled in an ExceptionGroup, does anything stand out?" -> set aside those cancelled due to a sibling task, inspect remainder. That does argue for @TeamSpen210's idea of passing multiple arguments and assembling the __str__ later - we could also store them as attributes, and then easily split exception groups.

I really love "collect and show everything" as an error-handling default, but when you get to 150k line tracebacks, programmatically determining what we can safely omit becomes very very important 😅

@A5rocks
Copy link
Contributor

A5rocks commented Mar 27, 2025

Alright, that makes sense as to why not to add more context for case 2, probably case 3 too. Too bad since a stack trace of the task that called .cancel would function as a reason without having to opt in.

I think providing at least an attribute with an enum is a good idea. I'm not sure what the best strategy for sum types in Python is (because some enum members sound like they would need extra context), but maybe we can get away with .reason: tuple[Literal[CancelReason.DeadlineReached]] | tuple[Literal[CancelReason.NurseryShutdown], str] | .... Or just have an extra_information attribute that takes in whatever.

@Zac-HD
Copy link
Member Author

Zac-HD commented Mar 27, 2025

I'm now thinking of having three attributes source: Literal["deadline", "nursery", "explicit"], source_task: str, and reason: str | None; I also tend to stick to Literal over enums these days.

Storing tracebacks makes me pretty nervous; it's easy to blow up memory use due to delayed garbage collection of all the extra objects they keep references to. It does sound nice to have as an opt-in debug mode, but I don't actually have a specific case in mind where it'd help.

@mikenerone
Copy link
Member

Storing tracebacks makes me pretty nervous; it's easy to blow up memory use due to delayed garbage collection of all the extra objects they keep references to. It does sound nice to have as an opt-in debug mode, but I don't actually have a specific case in mind where it'd help.

There are also use cases that rely on the GC so object finalizers get fired (including one of my own apps).

@Zac-HD
Copy link
Member Author

Zac-HD commented Mar 31, 2025

There are also use cases that rely on the GC so object finalizers get fired (including one of my own apps).

This is not strictly speaking supported behavior for Python, and risky to rely on - you can get broken by CPython updates, or library changes, or PyPy having different GC semantics, etc. - but I agree that it's reasonable for Trio to avoid breaking such use-cases if we can.

@jakkdl jakkdl linked a pull request Apr 22, 2025 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants