Skip to content

Commit 439ee7b

Browse files
authored
fix: do not abort Actor exit when the terminal status message fails (#1078)
In `_ActorType.__aexit__`, the terminal `set_status_message()` call was the only step in `finalize()` not wrapped in `try`/`except Exception`. A transient API error there propagated out of `__aexit__` and skipped everything after it: the event manager exit, the charging manager exit, `_save_actor_state()` and `sys.exit(self.exit_code)`. The run then died with an unrelated traceback, the wrong exit code, and unpersisted state. The call is now guarded the same way as the three steps that follow it - the failure is logged and cleanup continues. *✍️ Drafted by Claude Code*
1 parent 8905a97 commit 439ee7b

2 files changed

Lines changed: 32 additions & 1 deletion

File tree

src/apify/_actor.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,10 @@ async def __aexit__(
260260

261261
async def finalize() -> None:
262262
if self.status_message is not None:
263-
await self.set_status_message(self.status_message, is_terminal=True)
263+
try:
264+
await self.set_status_message(self.status_message, is_terminal=True)
265+
except Exception:
266+
self.log.exception('Failed to set terminal status message')
264267

265268
# Sleep for a bit so that the listeners have a chance to trigger
266269
await asyncio.sleep(0.1)

tests/unit/actor/test_actor_lifecycle.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,34 @@ async def test_actor_fail_prevents_further_execution(caplog: pytest.LogCaptureFi
410410
assert status_records[0].levelno == logging.INFO
411411

412412

413+
async def test_failing_terminal_status_message_does_not_abort_exit(
414+
caplog: pytest.LogCaptureFixture,
415+
monkeypatch: pytest.MonkeyPatch,
416+
) -> None:
417+
"""A failing terminal status message must be logged and must not skip the rest of the cleanup nor the exit code."""
418+
set_status_message = AsyncMock(side_effect=RuntimeError('Status update failed'))
419+
charging_manager_exit = AsyncMock()
420+
save_actor_state = AsyncMock()
421+
monkeypatch.setattr(_ActorType, 'set_status_message', set_status_message)
422+
monkeypatch.setattr(ChargingManagerImplementation, '__aexit__', charging_manager_exit)
423+
monkeypatch.setattr(_ActorType, '_save_actor_state', save_actor_state)
424+
425+
# Explicitly set exit_process=True since in Pytest env it defaults to False.
426+
actor = Actor(exit_process=True)
427+
await actor.init()
428+
429+
with pytest.raises(SystemExit) as exc_info:
430+
await actor.exit(exit_code=7, status_message='Done')
431+
432+
# The mock replaces a class attribute, so it is not bound and does not receive `self`.
433+
set_status_message.assert_called_once_with('Done', is_terminal=True)
434+
assert [r for r in caplog.records if r.msg == 'Failed to set terminal status message']
435+
assert exc_info.value.code == 7
436+
assert actor.event_manager.active is False
437+
charging_manager_exit.assert_called_once()
438+
save_actor_state.assert_called_once()
439+
440+
413441
@pytest.mark.parametrize(
414442
('first_with_call', 'second_with_call'),
415443
[

0 commit comments

Comments
 (0)