Skip to content

Commit 6005bf2

Browse files
errors: make WorkflowCancelled/ActivityCancelled uncatchable by except Exception
Inherit from BaseException (not DurableWorkflowError/Exception) so a generic except Exception: block in activity bodies or result handlers cannot silently swallow cancellation. Mirrors asyncio.CancelledError / KeyboardInterrupt. Pre-1.0 shape fix for zorporation/durable-workflow#441 — upstream lesson from temporalio/sdk-python#1292. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4e4b9c8 commit 6005bf2

4 files changed

Lines changed: 153 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
66

77
## [Unreleased]
88

9+
### Changed
10+
- **Breaking (pre-1.0):** `WorkflowCancelled` and `ActivityCancelled` now inherit
11+
from `BaseException` (not `DurableWorkflowError` / `Exception`), so a generic
12+
`except Exception:` block in activity code or result handlers no longer
13+
silently swallows cancellation. Callers that relied on catching cancellation
14+
via `except Exception:` or `except DurableWorkflowError:` must now either
15+
catch the class by name (e.g. `except (ActivityCancelled, WorkflowCancelled):`)
16+
or catch `BaseException`. Mirrors the standard-library precedent set by
17+
`asyncio.CancelledError` and `KeyboardInterrupt`. Rationale and upstream
18+
lesson: [zorporation/durable-workflow#441](https://github.com/zorporation/durable-workflow/issues/441).
19+
920
## [0.3.0] — 2026-04-19
1021

1122
### Added

src/durable_workflow/errors.py

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,19 @@
11
"""Typed exceptions raised by the Durable Workflow client and worker.
22
3-
Every exception inherits from :class:`DurableWorkflowError`, so callers that
4-
only want to distinguish SDK errors from unrelated failures can catch that
5-
base. More specific subclasses let callers react to particular outcomes —
6-
workflow-not-found, update-rejected, schedule-already-exists — without
7-
parsing server response bodies.
3+
Every *error* exception inherits from :class:`DurableWorkflowError`, so
4+
callers that only want to distinguish SDK errors from unrelated failures can
5+
catch that base. More specific subclasses let callers react to particular
6+
outcomes — workflow-not-found, update-rejected, schedule-already-exists —
7+
without parsing server response bodies.
8+
9+
Cancellation is intentionally *not* in that hierarchy. :class:`WorkflowCancelled`
10+
and :class:`ActivityCancelled` inherit from :class:`BaseException` directly,
11+
so a generic ``except Exception:`` block cannot accidentally swallow a
12+
cancellation signal. Callers that want to handle cancellation must name the
13+
class explicitly (``except (ActivityCancelled, ...):``). This mirrors the way
14+
:class:`asyncio.CancelledError` and :class:`KeyboardInterrupt` behave in the
15+
standard library and avoids the historical mistake called out in
16+
https://github.com/temporalio/sdk-python/issues/1292.
817
"""
918

1019
from __future__ import annotations
@@ -140,19 +149,39 @@ def __init__(self, message: str = "workflow was terminated") -> None:
140149
super().__init__(message)
141150

142151

143-
class WorkflowCancelled(DurableWorkflowError):
144-
"""A workflow was cancelled and finished in the ``cancelled`` state."""
152+
class WorkflowCancelled(BaseException):
153+
"""A workflow was cancelled and finished in the ``cancelled`` state.
154+
155+
Inherits from :class:`BaseException` — not :class:`Exception` — so that a
156+
generic ``except Exception:`` block cannot accidentally swallow the
157+
cancellation outcome. Callers that want to treat a cancelled workflow
158+
differently from a failed one (e.g. to skip alerting) must catch this
159+
class by name.
160+
"""
145161

146162
def __init__(self, message: str = "workflow was cancelled") -> None:
147163
super().__init__(message)
148164

149165

150-
class ActivityCancelled(DurableWorkflowError):
166+
class ActivityCancelled(BaseException):
151167
"""An in-flight activity was cancelled.
152168
153169
Raised inside :meth:`durable_workflow.ActivityContext.heartbeat` when the
154170
server reports that the owning workflow has asked for cancellation, so the
155171
activity can exit cleanly on its next heartbeat.
172+
173+
Inherits from :class:`BaseException` — not :class:`Exception` — so that a
174+
user ``except Exception:`` block inside the activity function cannot
175+
accidentally swallow the cancellation signal. Activities that need to run
176+
cleanup on cancellation should catch this class by name and re-raise:
177+
178+
.. code-block:: python
179+
180+
try:
181+
await activity.context().heartbeat()
182+
except ActivityCancelled:
183+
cleanup()
184+
raise
156185
"""
157186

158187
def __init__(self, message: str = "activity was cancelled") -> None:

tests/test_activity_context.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,45 @@ async def heartbeat_activity() -> str:
199199
assert call_kwargs["failure_type"] == "ActivityCancelled"
200200
assert call_kwargs["non_retryable"] is True
201201

202+
@pytest.mark.asyncio
203+
async def test_cancellation_survives_user_generic_except(self, mock_client: AsyncMock) -> None:
204+
"""A user activity with ``except Exception:`` must not swallow cancellation.
205+
206+
Regression guard for zorporation/durable-workflow#441: when the server
207+
signals cancellation via the heartbeat response, the activity function
208+
may have a broad ``except Exception:`` block (for logging, retry, etc.).
209+
That block must *not* catch ``ActivityCancelled``, so the worker still
210+
reports the activity as cancelled rather than completing normally.
211+
"""
212+
mock_client.heartbeat_activity_task = AsyncMock(return_value={"cancel_requested": True})
213+
214+
@activity.defn(name="hb-swallow-act")
215+
async def activity_with_broad_except() -> str:
216+
ctx = activity.context()
217+
try:
218+
await ctx.heartbeat()
219+
except Exception: # noqa: BLE001 — intentional: simulates user mistake
220+
return "swallowed cancellation"
221+
return "no cancel"
222+
223+
worker = Worker(
224+
mock_client, task_queue="q1", workflows=[], activities=[activity_with_broad_except]
225+
)
226+
task = {
227+
"task_id": "at-swallow",
228+
"activity_attempt_id": "aa-swallow",
229+
"activity_type": "hb-swallow-act",
230+
"arguments": "[]",
231+
"payload_codec": "json",
232+
}
233+
outcome = await worker._run_activity_task(task)
234+
235+
assert outcome == "cancelled"
236+
mock_client.fail_activity_task.assert_called_once()
237+
call_kwargs = mock_client.fail_activity_task.call_args.kwargs
238+
assert call_kwargs["failure_type"] == "ActivityCancelled"
239+
mock_client.complete_activity_task.assert_not_called()
240+
202241
@pytest.mark.asyncio
203242
async def test_non_retryable_error(self, mock_client: AsyncMock) -> None:
204243
@activity.defn(name="nr-act")

tests/test_errors.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@
33
import pytest
44

55
from durable_workflow.errors import (
6+
ActivityCancelled,
7+
DurableWorkflowError,
68
InvalidArgument,
79
NamespaceNotFound,
810
QueryFailed,
911
ServerError,
1012
Unauthorized,
1113
UpdateRejected,
1214
WorkflowAlreadyStarted,
15+
WorkflowCancelled,
1316
WorkflowNotFound,
1417
_raise_for_status,
1518
)
@@ -78,3 +81,66 @@ def test_reason_from_dict(self) -> None:
7881
def test_reason_from_str(self) -> None:
7982
e = ServerError(500, "plain text")
8083
assert e.reason() is None
84+
85+
86+
class TestCancellationContract:
87+
"""Cancellation exceptions must not be caught by generic ``except Exception:``.
88+
89+
Guards the pre-1.0 design decision that ``WorkflowCancelled`` and
90+
``ActivityCancelled`` inherit from :class:`BaseException` directly, so that
91+
user worker/activity code cannot accidentally swallow a cancellation signal
92+
in a catch-all. See zorporation/durable-workflow#441.
93+
"""
94+
95+
def test_workflow_cancelled_not_caught_by_exception(self) -> None:
96+
try:
97+
raise WorkflowCancelled("stop")
98+
except Exception: # noqa: BLE001 — intentional: proves it does NOT catch
99+
pytest.fail("WorkflowCancelled must not be catchable via except Exception")
100+
except WorkflowCancelled:
101+
pass
102+
103+
def test_activity_cancelled_not_caught_by_exception(self) -> None:
104+
try:
105+
raise ActivityCancelled("stop")
106+
except Exception: # noqa: BLE001 — intentional: proves it does NOT catch
107+
pytest.fail("ActivityCancelled must not be catchable via except Exception")
108+
except ActivityCancelled:
109+
pass
110+
111+
def test_workflow_cancelled_not_caught_by_durableworkflowerror(self) -> None:
112+
"""Belt-and-braces: DurableWorkflowError is also an Exception subclass."""
113+
try:
114+
raise WorkflowCancelled("stop")
115+
except DurableWorkflowError:
116+
pytest.fail(
117+
"WorkflowCancelled must not be catchable via except DurableWorkflowError"
118+
)
119+
except WorkflowCancelled:
120+
pass
121+
122+
def test_activity_cancelled_not_caught_by_durableworkflowerror(self) -> None:
123+
try:
124+
raise ActivityCancelled("stop")
125+
except DurableWorkflowError:
126+
pytest.fail(
127+
"ActivityCancelled must not be catchable via except DurableWorkflowError"
128+
)
129+
except ActivityCancelled:
130+
pass
131+
132+
def test_cancellation_explicitly_caught_by_baseexception(self) -> None:
133+
"""Callers that really want to catch everything still can — by naming BaseException."""
134+
for exc_cls in (WorkflowCancelled, ActivityCancelled):
135+
try:
136+
raise exc_cls("stop")
137+
except BaseException as e: # noqa: BLE001 — intentional
138+
assert isinstance(e, exc_cls)
139+
140+
def test_cancellation_inheritance_shape(self) -> None:
141+
assert issubclass(WorkflowCancelled, BaseException)
142+
assert issubclass(ActivityCancelled, BaseException)
143+
assert not issubclass(WorkflowCancelled, Exception)
144+
assert not issubclass(ActivityCancelled, Exception)
145+
assert not issubclass(WorkflowCancelled, DurableWorkflowError)
146+
assert not issubclass(ActivityCancelled, DurableWorkflowError)

0 commit comments

Comments
 (0)