temporalio / temporalio/sdk-python
[Feature Request] temporalio.CancelledError should inherit from BaseException
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1.2k
- Forks
- 241
- Avg merge
- 3d 21h
- Merged PRs (30d)
- 55
Description
Cancelled Error Deriving from Exception makes it easy to inadvertently ignore Cancellation Attempts
Currently temporalio.CancelledError inherits from temporalio.FailureError which in turn inherits from Exception.
I would like to suggest that the semantics of this type of error would be more appropriate if it derived from BaseException, primarily because of expectations and patterns of how existing application code might be handling those various cases.
Basically it is very common in user/app code to have fairly broad exception cases like except Exception as e: etc. This is arguably not "best practice" (you should really catch more specific individual exception types), but it happens a lot.
My understanding is that Temporal Cancellation errors are implemented via a fairly low-level Python C-extension API, which allows them to forcibly preempt arbitrary user code. This means those cancellation errors can surface at arbitrary points in the call-graph, i.e. they pop up in random places in User code, not just in the "outer layer" of Temporal workflow code.
This creates a scenario where it's easy for error handling logic in random application code to unintentionally ignore Temporal Cancellation attempts, which leads to lots of confusion when workflows that should have been cancelled keep running.
I believe this type of semantics is why the core Python runtime typically uses BaseException for things like hard system interrupts, cancellation errors, OOMs, etc. And you can actually see that the stdlib changed the base class of asyncio.CancelledError
from Exception to BaseException, I believe for similar reasons.
I realize that while this change is simple to implement it would have meaningful consequences for existing user code, but I wanted to propose the idea and see if this is something that has come up in discussion before, or if the community has any other suggestions on how to handle temporal CancelledError when working with existing code that may be handling Exception broadly.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in temporalio/exceptions.py around the CancelledError definition at line 145, then review how cancellation is raised and handled across the SDK. Confirm the impact on existing Exception handlers and define the required compatibility behavior before changing the inheritance; done means cancellation is not inadvertently caught by broad Exception handlers and the relevant behavior is covered by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100