Skip to content

Commit 976a77d

Browse files
rmcdanielrmcdaniel
authored
Merge pull request #900 from durable-workflow/fix/878-expired-attempt-view
Preserve lease-loss status and reason in activity attempt views
2 parents 1a5ec33 + e74e475 commit 976a77d

4 files changed

Lines changed: 235 additions & 6 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
- Preserve recorded activity lease expiry and worker deregistration in canonical
6+
attempt views, including exhausted retries and cold history reads. Keep
7+
ordinary failures and unavailable lease-loss reasons distinct.
8+
39
## 2.10.1 - 2026-10-21
410

511
- Maintain eligible embedded activity leases from a fresh metadata-only PHP

‎src/V2/Support/ActivityAttemptSnapshots.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,12 @@ private static function fromEvent(WorkflowHistoryEvent $event): ?array
154154
HistoryEventType::ActivityTimedOut,
155155
HistoryEventType::ActivityCancelled,
156156
], true);
157+
$leaseFailureReason = in_array($event->event_type, [
158+
HistoryEventType::ActivityRetryScheduled,
159+
HistoryEventType::ActivityFailed,
160+
], true) && in_array($payload['failure_reason'] ?? null, ['lease_expired', 'worker_deregistered'], true)
161+
? $payload['failure_reason']
162+
: null;
157163

158164
$snapshot = array_filter([
159165
'id' => $attemptId,
@@ -168,8 +174,9 @@ private static function fromEvent(WorkflowHistoryEvent $event): ?array
168174
?? self::intValue($payload['retry_after_attempt'] ?? null)
169175
?? self::intValue($activity['attempt_count'] ?? null)
170176
?? self::intValue($task['attempt_count'] ?? null),
171-
'status' => self::statusForEvent($event->event_type)
172-
?? self::stringValue($attempt['status'] ?? null),
177+
'status' => $leaseFailureReason !== null
178+
? ActivityAttemptStatus::Expired->value
179+
: (self::statusForEvent($event->event_type) ?? self::stringValue($attempt['status'] ?? null)),
173180
'lease_owner' => self::stringValue($attempt['lease_owner'] ?? null)
174181
?? self::stringValue($task['lease_owner'] ?? null),
175182
'started_at' => self::timestamp($attempt['started_at'] ?? null)
@@ -189,6 +196,7 @@ private static function fromEvent(WorkflowHistoryEvent $event): ?array
189196

190197
if ($terminalEvent) {
191198
$snapshot['lease_expires_at'] = null;
199+
$snapshot['failure_reason'] = $leaseFailureReason;
192200
}
193201

194202
return $snapshot;

‎src/V2/Support/RunActivityView.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ private static function presentAttempt(array $attempt, ?ActivityExecution $execu
400400
'can_continue' => self::attemptStateCanContinue($attempt, $execution),
401401
'cancel_requested' => self::attemptStateCancelRequested($attempt, $execution),
402402
'stop_reason' => self::attemptStateStopReason($attempt, $execution),
403+
'failure_reason' => self::stringValue($attempt['failure_reason'] ?? null),
403404
];
404405
}
405406

@@ -433,6 +434,7 @@ private static function syntheticAttempt(array $state, ?ActivityExecution $execu
433434
'can_continue' => self::syntheticAttemptCanContinue($state, $execution),
434435
'cancel_requested' => self::syntheticAttemptCancelRequested($state, $execution),
435436
'stop_reason' => self::syntheticAttemptStopReason($state, $execution),
437+
'failure_reason' => null,
436438
];
437439
}
438440

@@ -471,12 +473,13 @@ private static function attemptStateStopReason(array $attempt, ?ActivityExecutio
471473
return 'attempt_cancelled';
472474
}
473475

474-
if ($execution?->status === ActivityStatus::Cancelled) {
475-
return 'activity_cancelled';
476+
if ($status === ActivityAttemptStatus::Expired->value) {
477+
return self::stringValue($attempt['failure_reason'] ?? null)
478+
?? ($execution?->status === ActivityStatus::Cancelled ? 'activity_cancelled' : 'attempt_expired');
476479
}
477480

478-
if ($status === ActivityAttemptStatus::Expired->value) {
479-
return 'attempt_expired';
481+
if ($execution?->status === ActivityStatus::Cancelled) {
482+
return 'activity_cancelled';
480483
}
481484

482485
if (in_array($status, [

‎tests/Unit/V2/ActivityAttemptLeaseViewTest.php‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Unit\V2;
6+
7+
use Illuminate\Database\Eloquent\Collection;
8+
use Orchestra\Testbench\TestCase;
9+
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Workflow\V2\Enums\ActivityAttemptStatus;
11+
use Workflow\V2\Enums\HistoryEventType;
12+
use Workflow\V2\Models\ActivityAttempt;
13+
use Workflow\V2\Models\ActivityExecution;
14+
use Workflow\V2\Models\WorkflowHistoryEvent;
15+
use Workflow\V2\Models\WorkflowRun;
16+
use Workflow\V2\Support\RunActivityView;
17+
18+
final class ActivityAttemptLeaseViewTest extends TestCase
19+
{
20+
public static function terminalCases(): array
21+
{
22+
return [
23+
'lease retry' => [HistoryEventType::ActivityRetryScheduled, 'lease_expired', 'expired', 'lease_expired'],
24+
'exhausted lease' => [HistoryEventType::ActivityFailed, 'lease_expired', 'expired', 'lease_expired'],
25+
'deregistered retry' => [
26+
HistoryEventType::ActivityRetryScheduled,
27+
'worker_deregistered',
28+
'expired',
29+
'worker_deregistered',
30+
],
31+
'exhausted deregistration' => [
32+
HistoryEventType::ActivityFailed,
33+
'worker_deregistered',
34+
'expired',
35+
'worker_deregistered',
36+
],
37+
'ordinary retry' => [HistoryEventType::ActivityRetryScheduled, null, 'failed', 'attempt_closed'],
38+
'ordinary failure' => [HistoryEventType::ActivityFailed, null, 'failed', 'attempt_closed'],
39+
'unknown reason' => [HistoryEventType::ActivityFailed, 'unknown_failure', 'failed', 'attempt_closed'],
40+
'authored timeout' => [HistoryEventType::ActivityTimedOut, null, 'expired', 'attempt_expired'],
41+
];
42+
}
43+
44+
#[DataProvider('terminalCases')]
45+
public function testColdHistoryKeepsTheOriginalAttemptOutcome(
46+
HistoryEventType $eventType,
47+
?string $reason,
48+
string $expectedStatus,
49+
string $expectedStopReason,
50+
): void {
51+
$run = $this->createRun($eventType, $reason);
52+
$before = $this->attributes($run);
53+
$rows = RunActivityView::activitiesForRun($run, decodePayloads: false);
54+
55+
self::assertCount(1, $rows);
56+
self::assertSame('completed', $rows[0]['status']);
57+
self::assertSame('typed_history', $rows[0]['history_authority']);
58+
self::assertCount(2, $rows[0]['attempts']);
59+
[$original, $replacement] = $rows[0]['attempts'];
60+
self::assertSame('original-attempt', $original['id']);
61+
self::assertSame('original-task', $original['task_id']);
62+
self::assertSame('original-worker', $original['lease_owner']);
63+
self::assertSame(1, $original['attempt_number']);
64+
self::assertSame($expectedStatus, $original['status']);
65+
self::assertSame($expectedStopReason, $original['stop_reason']);
66+
self::assertSame(
67+
$expectedStatus === 'expired' && $reason !== null ? $reason : null,
68+
$original['failure_reason'] ?? null,
69+
);
70+
self::assertSame('2026-10-22T06:14:54.510928Z', $original['closed_at']->toJSON());
71+
self::assertNull($original['lease_expires_at']);
72+
self::assertFalse($original['can_continue']);
73+
self::assertFalse($original['cancel_requested']);
74+
self::assertSame('replacement-attempt', $replacement['id']);
75+
self::assertSame('completed', $replacement['status']);
76+
self::assertSame('attempt_closed', $replacement['stop_reason']);
77+
self::assertNull($replacement['failure_reason'] ?? null);
78+
self::assertFalse($replacement['can_continue']);
79+
self::assertSame($before, $this->attributes($run));
80+
}
81+
82+
public function testProjectionOnlyReadDoesNotInventALeaseLossReason(): void
83+
{
84+
$run = $this->createRun(HistoryEventType::ActivityRetryScheduled, 'lease_expired');
85+
$run->activityExecutions[0]->attempts[0]->status = ActivityAttemptStatus::Expired;
86+
$before = $this->attributes($run);
87+
$attempt = RunActivityView::activitiesForRun($run, false, false)[0]['attempts'][0];
88+
89+
self::assertSame('expired', $attempt['status']);
90+
self::assertSame('attempt_expired', $attempt['stop_reason']);
91+
self::assertNull($attempt['failure_reason'] ?? null);
92+
self::assertFalse($attempt['can_continue']);
93+
self::assertSame($before, $this->attributes($run));
94+
}
95+
96+
public function testLeaseReasonSurvivesMissingMutableAttemptRows(): void
97+
{
98+
$run = $this->createRun(HistoryEventType::ActivityRetryScheduled, 'lease_expired');
99+
$run->activityExecutions[0]->setRelation('attempts', new Collection());
100+
$rows = RunActivityView::activitiesForRun($run, decodePayloads: false);
101+
102+
self::assertCount(2, $rows[0]['attempts']);
103+
self::assertSame('expired', $rows[0]['attempts'][0]['status']);
104+
self::assertSame('lease_expired', $rows[0]['attempts'][0]['stop_reason']);
105+
self::assertSame('lease_expired', $rows[0]['attempts'][0]['failure_reason']);
106+
self::assertFalse($rows[0]['attempts'][0]['can_continue']);
107+
self::assertCount(0, $run->activityExecutions[0]->attempts);
108+
}
109+
110+
public function testLaterCancellationKeepsTheEarlierAttemptLeaseLoss(): void
111+
{
112+
$run = $this->createRun(HistoryEventType::ActivityRetryScheduled, 'lease_expired', cancelled: true);
113+
$before = $this->attributes($run);
114+
$row = RunActivityView::activitiesForRun($run, decodePayloads: false)[0];
115+
116+
self::assertSame('cancelled', $row['status']);
117+
self::assertSame('expired', $row['attempts'][0]['status']);
118+
self::assertSame('lease_expired', $row['attempts'][0]['stop_reason']);
119+
self::assertSame('lease_expired', $row['attempts'][0]['failure_reason']);
120+
self::assertFalse($row['attempts'][0]['can_continue']);
121+
self::assertSame('cancelled', $row['attempts'][1]['status']);
122+
self::assertSame('attempt_cancelled', $row['attempts'][1]['stop_reason']);
123+
self::assertSame($before, $this->attributes($run));
124+
}
125+
126+
private function createRun(HistoryEventType $eventType, ?string $reason, bool $cancelled = false): WorkflowRun
127+
{
128+
$original = new ActivityAttempt([
129+
'id' => 'original-attempt',
130+
'activity_execution_id' => 'activity',
131+
'workflow_task_id' => 'original-task',
132+
'attempt_number' => 1,
133+
'status' => 'failed',
134+
'lease_owner' => 'original-worker',
135+
'started_at' => '2026-10-22 06:14:49.510928',
136+
'closed_at' => '2026-10-22 06:14:54.510928',
137+
]);
138+
$replacement = new ActivityAttempt([
139+
'id' => 'replacement-attempt',
140+
'activity_execution_id' => 'activity',
141+
'workflow_task_id' => 'replacement-task',
142+
'attempt_number' => 2,
143+
'status' => $cancelled ? 'cancelled' : 'completed',
144+
]);
145+
$execution = new ActivityExecution([
146+
'id' => 'activity',
147+
'status' => $cancelled ? 'cancelled' : 'completed',
148+
'current_attempt_id' => 'replacement-attempt',
149+
'attempt_count' => 2,
150+
]);
151+
$execution->setRelation('attempts', new Collection([$original, $replacement]));
152+
$event = new WorkflowHistoryEvent([
153+
'sequence' => 1,
154+
'event_type' => $eventType,
155+
'workflow_task_id' => 'original-task',
156+
'recorded_at' => '2026-10-22 06:14:54.510928',
157+
'payload' => array_filter([
158+
'activity_execution_id' => 'activity',
159+
'activity_attempt_id' => 'original-attempt',
160+
'attempt_number' => 1,
161+
'failure_reason' => $reason,
162+
'activity' => [
163+
'id' => 'activity',
164+
'status' => 'pending',
165+
'attempt_count' => 1,
166+
],
167+
'activity_attempt' => [
168+
'id' => 'original-attempt',
169+
'lease_owner' => 'original-worker',
170+
],
171+
], static fn (mixed $value): bool => $value !== null),
172+
]);
173+
$completed = new WorkflowHistoryEvent([
174+
'sequence' => 2,
175+
'event_type' => $cancelled ? HistoryEventType::ActivityCancelled : HistoryEventType::ActivityCompleted,
176+
'workflow_task_id' => 'replacement-task',
177+
'recorded_at' => '2026-10-22 06:15:00.123456',
178+
'payload' => [
179+
'activity_execution_id' => 'activity',
180+
'activity_attempt_id' => 'replacement-attempt',
181+
'attempt_number' => 2,
182+
'activity' => [
183+
'id' => 'activity',
184+
'status' => $cancelled ? 'cancelled' : 'completed',
185+
'attempt_count' => 2,
186+
],
187+
],
188+
]);
189+
$run = new WorkflowRun([
190+
'id' => 'run',
191+
'namespace' => 'lease-observation',
192+
]);
193+
$run->setRelation('activityExecutions', new Collection([$execution]));
194+
$run->setRelation('historyEvents', new Collection([$event, $completed]));
195+
196+
return $run;
197+
}
198+
199+
private function attributes(WorkflowRun $run): array
200+
{
201+
return [
202+
$run->getAttributes(),
203+
$run->activityExecutions->map(static fn ($row): array => [
204+
$row->getAttributes(),
205+
$row->attempts->map(static fn ($attempt): array => $attempt->getAttributes())
206+
->all(),
207+
])->all(),
208+
$run->historyEvents->map(static fn ($row): array => $row->getAttributes())
209+
->all(),
210+
];
211+
}
212+
}

0 commit comments

Comments
 (0)