Skip to content

Commit d8d4219

Browse files
Skip stale foreign exception logs during replay
1 parent 1ea7c85 commit d8d4219

5 files changed

Lines changed: 135 additions & 10 deletions

File tree

src/ActivityStub.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ public static function make($activity, ...$arguments): PromiseInterface
6666
++$context->index;
6767
WorkflowStub::setContext($context);
6868
$result = Serializer::unserialize($log->result);
69+
if ($log->class === Exception::class && self::isForeignExceptionResult($result, $activity)) {
70+
return self::make($activity, ...$arguments);
71+
}
6972
if (
7073
is_array($result) &&
7174
array_key_exists('class', $result) &&
@@ -98,4 +101,12 @@ public static function make($activity, ...$arguments): PromiseInterface
98101
WorkflowStub::setContext($context);
99102
return (new Deferred())->promise();
100103
}
104+
105+
private static function isForeignExceptionResult(mixed $result, string $activity): bool
106+
{
107+
return is_array($result)
108+
&& isset($result['sourceClass'])
109+
&& is_string($result['sourceClass'])
110+
&& $result['sourceClass'] !== $activity;
111+
}
101112
}

src/ChildWorkflowStub.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ public static function make($workflow, ...$arguments): PromiseInterface
6060
++$context->index;
6161
WorkflowStub::setContext($context);
6262
$result = Serializer::unserialize($log->result);
63+
if ($log->class === Exception::class && self::isForeignExceptionResult($result, $workflow)) {
64+
return self::make($workflow, ...$arguments);
65+
}
6366
if (
6467
is_array($result)
6568
&& array_key_exists('class', $result)
@@ -114,4 +117,12 @@ public static function make($workflow, ...$arguments): PromiseInterface
114117
WorkflowStub::setContext($context);
115118
return (new Deferred())->promise();
116119
}
120+
121+
private static function isForeignExceptionResult(mixed $result, string $workflow): bool
122+
{
123+
return is_array($result)
124+
&& isset($result['sourceClass'])
125+
&& is_string($result['sourceClass'])
126+
&& $result['sourceClass'] !== $workflow;
127+
}
117128
}

src/Exception.php

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Illuminate\Foundation\Bus\Dispatchable;
1111
use Illuminate\Queue\InteractsWithQueue;
1212
use Illuminate\Queue\SerializesModels;
13+
use Illuminate\Support\Facades\Cache;
1314
use Throwable;
1415
use Workflow\Exceptions\TransitionNotFound;
1516
use Workflow\Middleware\WithoutOverlappingMiddleware;
@@ -51,18 +52,30 @@ public function __construct(
5152

5253
public function handle()
5354
{
54-
$workflow = $this->storedWorkflow->toWorkflow();
55+
$lock = Cache::lock('laravel-workflow-exception:' . $this->storedWorkflow->id, 15);
56+
57+
if (! $lock->get()) {
58+
$this->release();
59+
60+
return;
61+
}
5562

5663
try {
57-
if ($this->storedWorkflow->hasLogByIndex($this->index)) {
58-
$workflow->resume();
59-
} elseif ($this->shouldPersistAfterProbeReplay()) {
60-
$workflow->next($this->index, $this->now, self::class, $this->exception);
61-
}
62-
} catch (TransitionNotFound) {
63-
if ($workflow->running()) {
64-
$this->release();
64+
$workflow = $this->storedWorkflow->toWorkflow();
65+
66+
try {
67+
if ($this->storedWorkflow->hasLogByIndex($this->index)) {
68+
$workflow->resume();
69+
} elseif ($this->shouldPersistAfterProbeReplay()) {
70+
$workflow->next($this->index, $this->now, self::class, $this->exceptionPayload());
71+
}
72+
} catch (TransitionNotFound) {
73+
if ($workflow->running()) {
74+
$this->release();
75+
}
6576
}
77+
} finally {
78+
$lock->release();
6679
}
6780
}
6881

@@ -132,11 +145,22 @@ private function createTentativeWorkflowState(): StoredWorkflow
132145
'index' => $this->index,
133146
'now' => $this->now,
134147
'class' => self::class,
135-
'result' => Serializer::serialize($this->exception),
148+
'result' => Serializer::serialize($this->exceptionPayload()),
136149
]);
137150

138151
$storedWorkflowClass = $this->storedWorkflow::class;
139152

140153
return $storedWorkflowClass::query()->findOrFail($this->storedWorkflow->id);
141154
}
155+
156+
private function exceptionPayload()
157+
{
158+
if (! is_array($this->exception) || $this->sourceClass === null) {
159+
return $this->exception;
160+
}
161+
162+
return array_merge($this->exception, [
163+
'sourceClass' => $this->sourceClass,
164+
]);
165+
}
142166
}

tests/Unit/ActivityStubTest.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
use Exception;
88
use RuntimeException;
99
use Tests\Fixtures\TestActivity;
10+
use Tests\Fixtures\TestOtherActivity;
1011
use Tests\Fixtures\TestWorkflow;
1112
use Tests\TestCase;
1213
use Workflow\ActivityStub;
14+
use Workflow\Exception as WorkflowException;
1315
use Workflow\Models\StoredWorkflow;
1416
use Workflow\Serializers\Serializer;
1517
use Workflow\States\WorkflowPendingStatus;
@@ -123,6 +125,43 @@ public function testLoadsStoredExceptionWithNonStandardConstructor(): void
123125
});
124126
}
125127

128+
public function testSkipsStoredExceptionForDifferentSourceClass(): void
129+
{
130+
$workflow = WorkflowStub::load(WorkflowStub::make(TestWorkflow::class)->id());
131+
$storedWorkflow = StoredWorkflow::findOrFail($workflow->id());
132+
$storedWorkflow->update([
133+
'arguments' => Serializer::serialize([]),
134+
'status' => WorkflowPendingStatus::$name,
135+
]);
136+
$storedWorkflow->logs()
137+
->create([
138+
'index' => 0,
139+
'now' => WorkflowStub::now(),
140+
'class' => WorkflowException::class,
141+
'result' => Serializer::serialize([
142+
'class' => Exception::class,
143+
'message' => 'foreign',
144+
'code' => 0,
145+
'sourceClass' => TestOtherActivity::class,
146+
]),
147+
]);
148+
$storedWorkflow->logs()
149+
->create([
150+
'index' => 1,
151+
'now' => WorkflowStub::now(),
152+
'class' => TestActivity::class,
153+
'result' => Serializer::serialize('test'),
154+
]);
155+
156+
ActivityStub::make(TestActivity::class)
157+
->then(static function ($value) use (&$result) {
158+
$result = $value;
159+
});
160+
161+
$this->assertSame('test', $result);
162+
$this->assertSame(2, WorkflowStub::getContext()->index);
163+
}
164+
126165
public function testAll(): void
127166
{
128167
$workflow = WorkflowStub::load(WorkflowStub::make(TestWorkflow::class)->id());

tests/Unit/ChildWorkflowStubTest.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44

55
namespace Tests\Unit;
66

7+
use Exception;
78
use Mockery;
89
use Tests\Fixtures\TestChildWorkflow;
10+
use Tests\Fixtures\TestExceptionWorkflow;
911
use Tests\Fixtures\TestParentWorkflow;
1012
use Tests\TestCase;
1113
use Workflow\ChildWorkflowStub;
14+
use Workflow\Exception as WorkflowException;
1215
use Workflow\Models\StoredWorkflow;
1316
use Workflow\Serializers\Serializer;
1417
use Workflow\States\WorkflowPendingStatus;
@@ -91,6 +94,43 @@ public function testLoadsChildWorkflow(): void
9194
$this->assertNull($result);
9295
}
9396

97+
public function testSkipsStoredExceptionForDifferentSourceClass(): void
98+
{
99+
$workflow = WorkflowStub::load(WorkflowStub::make(TestParentWorkflow::class)->id());
100+
$storedWorkflow = StoredWorkflow::findOrFail($workflow->id());
101+
$storedWorkflow->update([
102+
'arguments' => Serializer::serialize([]),
103+
'status' => WorkflowPendingStatus::$name,
104+
]);
105+
$storedWorkflow->logs()
106+
->create([
107+
'index' => 0,
108+
'now' => WorkflowStub::now(),
109+
'class' => WorkflowException::class,
110+
'result' => Serializer::serialize([
111+
'class' => Exception::class,
112+
'message' => 'foreign child',
113+
'code' => 0,
114+
'sourceClass' => TestExceptionWorkflow::class,
115+
]),
116+
]);
117+
$storedWorkflow->logs()
118+
->create([
119+
'index' => 1,
120+
'now' => WorkflowStub::now(),
121+
'class' => TestChildWorkflow::class,
122+
'result' => Serializer::serialize('test'),
123+
]);
124+
125+
ChildWorkflowStub::make(TestChildWorkflow::class)
126+
->then(static function ($value) use (&$result) {
127+
$result = $value;
128+
});
129+
130+
$this->assertSame('test', $result);
131+
$this->assertSame(2, WorkflowStub::getContext()->index);
132+
}
133+
94134
public function testDoesNotResumeRunningStartedChildWorkflow(): void
95135
{
96136
$childWorkflow = Mockery::mock();

0 commit comments

Comments
 (0)