fix(temporal): an activity that times out fails its workflow instead of leaving it Running (#544)

[gplanchat]

Closes #544. Found while writing #518 (#545).

What was wrong

On Temporal, an activity whose last attempt timed out left its workflow Running forever. TemporalExecutionHistory read ACTIVITY_TASK_COMPLETED, FAILED and CANCELED, but not ACTIVITY_TASK_TIMED_OUT. The replayed workflow found neither a result nor a failure for that activity, so it suspended again on every workflow task.

The fix

TIMED_OUT now settles the activity slot with a DurableActivityFailedException whose envelope is \RuntimeException and whose message names the timeout, for instance Activity start-to-close timeout exceeded. The journal backends raise exactly that for their own timeouts (ActivityMessageProcessor, then ActivityFailureEventFactory), so a workflow catches the same failure on every backend. No new envelope type was needed.

The timeout type comes from the server's TimeoutFailureInfo:

• start-to-close, schedule-to-start and schedule-to-close use the journal's wording;
• heartbeat, which the journal backends don't enforce, reads Activity heartbeat timeout exceeded., with the same type.

The server records TIMED_OUT only after the last attempt, since retries leave no event, so the failure is terminal.

Done when (each test failing first)

• A real server fails the workflow: WorkflowFailurePathsTest::testAnActivityThatTimesOutFailsTheWorkflowOnTheServer runs a 3 s activity under a 1 s start-to-close timeout with RetryLimit::once(). With the fix reverted: Event EVENT_TYPE_WORKFLOW_EXECUTION_FAILED missing from the history … after 30 s. With it: OK in 1.4 s. Private temporalio/temporal:1.9.1 server.
• TemporalExecutionHistory raises the journal's failure: AnActivityTimeoutFailsItsSlotTest covers all four timeout types, all red before the fix (the slot was null). It also compares against the journal directly: the same exception class, envelope class and message as DurableActivityFailedException::toThrowable() on the journal's own start-to-close timeout.
• A run the bug left stuck fails on its next task: WorkflowTaskRunnerTest::testARunStuckOnAnUnreadActivityTimeoutFailsOnItsNextTask replays the exact stuck history: the activity scheduled, then TIMED_OUT, then the workflow task the bug completed with no command, then a new task. Before the fix the runner returned 0 commands (still stuck). Now it returns one FailWorkflowExecution carrying the timeout message. The history is unchanged, only the reading of TIMED_OUT changed, so replay does not diverge.

Checks

• Temporal unit tests: 255. Unit suite: 2074. PHPStan, Psalm and php-cs-fixer clean.
• Root integration and Temporal suites on the same server: 120 tests, all green except the 2 SearchAttributesTest cases, because my container lacks CI's --search-attribute flags.

[gplanchat]

Second review of b890a89 (e3a53fd plus e395f8b). I checked parity with the journal backends and alice's replay condition. Verdict: OK, conditional on the two Temporal integration jobs going green (both are pending as I write). There is no blocking finding, only two non-blocking notes.

Checked

• Tests fail without the fix. With TemporalExecutionHistory.php set back to origin/main, all six new tests fail: the four timeout kinds, the parity test, and the stuck-run replay. With the fix, the two files pass (20 tests).
• Parity with the journal backends.
  • For its own timeouts, ActivityMessageProcessor:71-126 raises a RuntimeException ("Activity {schedule-to-close|schedule-to-start|start-to-close} timeout exceeded."), which becomes a DurableActivityFailedException. The new branch builds the same exception class, envelope class and message.
  • testTheFailureIsTheOneTheJournalBackendsRaise pins this through ActivityFailureEventFactory + toThrowable, not a hand-copied string.
  • The journal's ActivityRetryState::Timeout never reaches the exception, which has no retry state on any backend, so nothing is lost there.
  • The heartbeat kind has no journal counterpart, since the journal does not enforce heartbeat timeouts. Its message follows the same pattern.
• The kind comes from TimeoutFailureInfo. getFailure()?->getTimeoutFailureInfo()?->getTimeoutType() is mapped for all four kinds, and each kind has its own data-provider row.
• Replay of a stuck run (alice's condition). testARunStuckOnAnUnreadActivityTimeoutFailsOnItsNextTask replays the exact shape from the issue's trace: ActivityTaskTimedOut, then a workflow task completed with no command, then a new task.
  • It asserts exactly one command, and that command is FAIL_WORKFLOW_EXECUTION carrying the timeout message.
  • A reschedule or a nondeterminism outcome would break the assertCount(1) or the command type, so both are ruled out.
  • The history is unchanged; only the reading changed. On a real server, the next task's FailWorkflowExecution is legal after an earlier task that completed with no command.
• The late completion in the integration fixture. The 3 s sleep attempt finishes after the server has timed it out. TemporalActivityWorker:189 sends that completion through ignoringStaleTask, so the rejected late completion is dropped and the worker keeps running for the tests that follow.

Non-blocking

1. default => 'start-to-close' (TemporalExecutionHistory.php, the new match). A TIMED_OUT event with no failure, or with TIMEOUT_TYPE_UNSPECIFIED, is reported as a start-to-close timeout. A real server always sets the type, so this only matters for a malformed or synthetic history. Naming it generically ("Activity timeout exceeded.") would avoid a wrong label; the journal never produces that message, so parity is unaffected.
2. attempt is hard-coded to 1, as in the existing ACTIVITY_TASK_FAILED branch. With retries, the journal's attempt() is the last attempt number. ActivityTaskStarted carries the attempt if you want to read it. This predates the PR and fits a separate follow-up.

The integration test lands in a commit after the fix (e395f8b after e3a53fd), so its red-first run can't be seen in the history. The unit tests sit in the fix commit, and they are red without it, as shown above.

[gplanchat]

Re-check of 227794e: OK. Point 1 is applied. A kind the server does not name (UNSPECIFIED, or a missing failure, which falls into default too) now reads "Activity timeout exceeded.", and the unspecified row pins it. The four known messages are unchanged. Both files pass (21 tests) and cs is clean. Point 2 stays a follow-up, as agreed. My OK is still conditional on the two Temporal integration jobs.

[gplanchat]

Review at 227794e (antoine). Verdict: OK, no finding. One non-blocking note, which predates this PR.

(1) The same failure the journal backends raise.

• The journal's ActivityMessageProcessor raises RuntimeException('Activity start-to-close timeout exceeded.') (:127), and appends the same shape for schedule-to-close and schedule-to-start (:72, :77).
• The bridge now builds a DurableActivityFailedException around a RuntimeException envelope with the same three messages. The parity test compares them against the journal's own toThrowable().
• Heartbeat has no journal counterpart, since only Temporal enforces it. A kind the server does not name now reads "Activity timeout exceeded." (227794e) rather than guessing start-to-close.
• Mutation: removing the ACTIVITY_TASK_TIMED_OUT case turns all 5 AnActivityTimeoutFailsItsSlotTest cases red, and the stuck-history replay test too.

(2) Replay of a run already stuck.

• Your test covers the uncaught path: the history after the empty completion gives one FailWorkflowExecution.
• I probed the caught path too, with a throwaway test, not committed. It uses the same stuck history, but the workflow catches DurableActivityFailedException and schedules another activity. Result: exactly one COMMAND_TYPE_SCHEDULE_ACTIVITY_TASK, and no divergence error. The empty-completed task in the history is not re-judged, and the new command lands on the current task.

(3) TIMED_OUT is terminal. Temporal keeps retries inside the activity's state and writes ACTIVITY_TASK_TIMED_OUT only once no attempt is left, or for schedule-to-close and schedule-to-start, which are never retried. So filling the slot's failure on that event cannot pre-empt a retry.

On a real server (my own temporalio/temporal:1.9.1), testAnActivityThatTimesOutFailsTheWorkflowOnTheServer:

• passes in about 1 s;
• with the case removed, fails after about 30 s instead of hanging.

Non-blocking, predates this PR: attempt is hardcoded to 1, here as in the existing ACTIVITY_TASK_FAILED branch. The journal reports the real attempt, from the activity message. So $e->attempt() and the exception message's attempt differ between backends for an activity that timed out after retries. The last attempt is readable from the ActivityTaskStarted event that startedEventId points to. That is worth a follow-up issue covering both branches, not this PR.

Gates: full PHPUnit 2,336 OK (110 skips); PHPStan, Psalm and cs:check 0.