Skip to content

Commit 030f3ea

Browse files
committed
fix: add exception handling to _run_handle and fix mypy errors
Address PR review comments: - Wrap _run_handle in try/except to match CPython Handle._run() behavior, preventing unhandled exceptions from crashing the event loop - Fix mypy no-any-return in signal.py params_from_payload - Fix mypy attr-defined errors for Handle private attributes - Update tests to verify exceptions are logged rather than propagated Signed-off-by: Tim Li <ltim@uber.com> Made-with: Cursor
1 parent dc45265 commit 030f3ea

File tree

3 files changed

+25
-8
lines changed

3 files changed

+25
-8
lines changed

cadence/_internal/workflow/deterministic_event_loop.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,22 @@ def _run_once(self) -> None:
169169

170170
@staticmethod
171171
def _run_handle(handle: events.Handle) -> None:
172-
handle._context.run(handle._callback, *handle._args)
172+
try:
173+
ctx = handle._context # type: ignore[attr-defined]
174+
cb = handle._callback # type: ignore[attr-defined]
175+
args = handle._args # type: ignore[attr-defined]
176+
if ctx is not None:
177+
ctx.run(cb, *args)
178+
else:
179+
cb(*args)
180+
except (SystemExit, KeyboardInterrupt):
181+
raise
182+
except BaseException as exc:
183+
handle._loop.call_exception_handler({ # type: ignore[attr-defined]
184+
'message': 'Exception in callback %r' % handle,
185+
'exception': exc,
186+
'handle': handle,
187+
})
173188

174189
def _run_forever_setup(self) -> None:
175190
self._check_closed()

cadence/signal.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ def params_from_payload(self, data_converter: Any, payload: Any) -> list[Any]:
8989
type_hints = [p.type_hint for p in self._params]
9090
if not type_hints:
9191
return []
92-
return data_converter.from_data(payload, type_hints)
92+
result: list[Any] = data_converter.from_data(payload, type_hints)
93+
return result
9394

9495
@staticmethod
9596
def wrap(

tests/cadence/_internal/workflow/test_signal_handling.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from datetime import timedelta
44

5-
import pytest
65

76
from cadence.api.v1.common_pb2 import ActivityType, Payload
87
from cadence.api.v1.history_pb2 import (
@@ -340,7 +339,7 @@ def test_signal_with_empty_payload(self):
340339
completion = result.decisions[0].complete_workflow_execution_decision_attributes
341340
assert DATA_CONVERTER.from_data(completion.result, [str]) == ["notified"]
342341

343-
def test_async_signal_handler_raises(self):
342+
def test_async_signal_handler_raises(self, caplog):
344343
engine = make_workflow_engine(AsyncSignalWorkflow)
345344
events = [
346345
HistoryEvent(
@@ -362,8 +361,9 @@ def test_async_signal_handler_raises(self):
362361
),
363362
]
364363

365-
with pytest.raises(NotImplementedError, match="Async signal handlers"):
364+
with caplog.at_level("ERROR"):
366365
engine.process_decision(events)
366+
assert "Async signal handlers" in caplog.text
367367

368368
def test_multi_param_signal(self):
369369
"""Signal handler with multiple typed parameters decodes correctly."""
@@ -422,8 +422,8 @@ def test_unknown_signal_dropped(self):
422422
completion = result.decisions[0].complete_workflow_execution_decision_attributes
423423
assert DATA_CONVERTER.from_data(completion.result, [str]) == ["notified"]
424424

425-
def test_signal_handler_exception_fails_decision(self):
426-
"""Exception in a signal handler propagates and fails the decision task."""
425+
def test_signal_handler_exception_fails_decision(self, caplog):
426+
"""Exception in a signal handler is caught and logged by the event loop."""
427427
engine = make_workflow_engine(FailingSignalWorkflow)
428428
events = [
429429
HistoryEvent(
@@ -445,8 +445,9 @@ def test_signal_handler_exception_fails_decision(self):
445445
),
446446
]
447447

448-
with pytest.raises(ValueError, match="handler failed on: boom"):
448+
with caplog.at_level("ERROR"):
449449
engine.process_decision(events)
450+
assert "handler failed on: boom" in caplog.text
450451

451452

452453
class TestSameBatchOrdering:

0 commit comments

Comments
 (0)