Skip to content

Commit 41300c6

Browse files
authored
[8.x] Prevent double throwing chained exception on sync queue (#42950)
* prevent double throwing chained exception on sync queue * decrease job count after processing
1 parent 33ef96e commit 41300c6

File tree

2 files changed

+67
-7
lines changed

2 files changed

+67
-7
lines changed

src/Illuminate/Queue/SyncQueue.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
class SyncQueue extends Queue implements QueueContract
1414
{
15+
protected $jobsCount = 0;
16+
1517
/**
1618
* Get the size of the queue.
1719
*
@@ -37,6 +39,8 @@ public function push($job, $data = '', $queue = null)
3739
{
3840
$queueJob = $this->resolveJob($this->createPayload($job, $queue, $data), $queue);
3941

42+
$this->jobsCount++;
43+
4044
try {
4145
$this->raiseBeforeJobEvent($queueJob);
4246

@@ -45,6 +49,8 @@ public function push($job, $data = '', $queue = null)
4549
$this->raiseAfterJobEvent($queueJob);
4650
} catch (Throwable $e) {
4751
$this->handleException($queueJob, $e);
52+
} finally {
53+
$this->jobsCount--;
4854
}
4955

5056
return 0;
@@ -113,11 +119,19 @@ protected function raiseExceptionOccurredJobEvent(Job $job, Throwable $e)
113119
*/
114120
protected function handleException(Job $queueJob, Throwable $e)
115121
{
116-
$this->raiseExceptionOccurredJobEvent($queueJob, $e);
122+
static $isGuarded = false;
123+
124+
if ($isGuarded) {
125+
$isGuarded = false;
126+
} else {
127+
$isGuarded = $this->jobsCount > 1;
117128

118-
$queueJob->fail($e);
129+
$this->raiseExceptionOccurredJobEvent($queueJob, $e);
119130

120-
throw $e;
131+
$queueJob->fail($e);
132+
133+
throw $e;
134+
}
121135
}
122136

123137
/**

tests/Integration/Queue/JobChainingTest.php

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
class JobChainingTest extends TestCase
1414
{
15-
public static $catchCallbackRan = false;
15+
public static $catchCallbackCount = 0;
1616

1717
protected function getEnvironmentSetUp($app)
1818
{
@@ -30,7 +30,6 @@ protected function tearDown(): void
3030
JobChainingTestFirstJob::$ran = false;
3131
JobChainingTestSecondJob::$ran = false;
3232
JobChainingTestThirdJob::$ran = false;
33-
static::$catchCallbackRan = false;
3433
}
3534

3635
public function testJobsCanBeChainedOnSuccess()
@@ -148,19 +147,56 @@ public function testThirdJobIsNotFiredIfSecondFails()
148147

149148
public function testCatchCallbackIsCalledOnFailure()
150149
{
150+
self::$catchCallbackCount = 0;
151+
151152
Bus::chain([
152153
new JobChainingTestFirstJob,
153154
new JobChainingTestFailingJob,
154155
new JobChainingTestSecondJob,
155156
])->catch(static function () {
156-
self::$catchCallbackRan = true;
157+
self::$catchCallbackCount++;
157158
})->dispatch();
158159

159160
$this->assertTrue(JobChainingTestFirstJob::$ran);
160-
$this->assertTrue(static::$catchCallbackRan);
161+
$this->assertSame(1, static::$catchCallbackCount);
161162
$this->assertFalse(JobChainingTestSecondJob::$ran);
162163
}
163164

165+
public function testCatchCallbackIsCalledOnceOnSyncQueue()
166+
{
167+
self::$catchCallbackCount = 0;
168+
169+
try {
170+
Bus::chain([
171+
new JobChainingTestFirstJob(),
172+
new JobChainingTestThrowJob(),
173+
new JobChainingTestSecondJob(),
174+
])->catch(function () {
175+
self::$catchCallbackCount++;
176+
})->onConnection('sync')->dispatch();
177+
} finally {
178+
$this->assertTrue(JobChainingTestFirstJob::$ran);
179+
$this->assertSame(1, static::$catchCallbackCount);
180+
$this->assertFalse(JobChainingTestSecondJob::$ran);
181+
}
182+
183+
self::$catchCallbackCount = 0;
184+
185+
try {
186+
Bus::chain([
187+
new JobChainingTestFirstJob(),
188+
new JobChainingTestThrowJob(),
189+
new JobChainingTestSecondJob(),
190+
])->catch(function () {
191+
self::$catchCallbackCount++;
192+
})->onConnection('sync')->dispatch();
193+
} finally {
194+
$this->assertTrue(JobChainingTestFirstJob::$ran);
195+
$this->assertSame(1, static::$catchCallbackCount);
196+
$this->assertFalse(JobChainingTestSecondJob::$ran);
197+
}
198+
}
199+
164200
public function testChainJobsUseSameConfig()
165201
{
166202
JobChainingTestFirstJob::dispatch()->allOnQueue('some_queue')->allOnConnection('sync1')->chain([
@@ -293,3 +329,13 @@ public function handle()
293329
$this->fail();
294330
}
295331
}
332+
333+
class JobChainingTestThrowJob implements ShouldQueue
334+
{
335+
use Dispatchable, InteractsWithQueue, Queueable;
336+
337+
public function handle()
338+
{
339+
throw new \Exception();
340+
}
341+
}

0 commit comments

Comments
 (0)