From 9dd1d9f386c5db532c7ddfd578bb2f980ea83bc4 Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 11:18:12 -0400 Subject: [PATCH 01/11] Catch errors when handling a failed job Handles errors when calling the failed method on a job. Could be ReflectionException or an exception with unserializing the payload. --- src/Illuminate/Queue/Worker.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index 4af98fe42153..005c800488bf 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -234,6 +234,8 @@ protected function handleJobException($connectionName, $job, WorkerOptions $opti $this->raiseExceptionOccurredJobEvent( $connectionName, $job, $e ); + } catch(Exception $exception) { + $this->raiseFailedJobEvent($connectionName, $job, $exception); } finally { if (! $job->isDeleted()) { $job->release($options->delay); From 914ba3474d690e26985ba3b72a760ec6985c2dae Mon Sep 17 00:00:00 2001 From: Brian Date: Tue, 1 Nov 2016 11:25:54 -0400 Subject: [PATCH 02/11] Forgot a space --- src/Illuminate/Queue/Worker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index 005c800488bf..402dbfbfe959 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -234,7 +234,7 @@ protected function handleJobException($connectionName, $job, WorkerOptions $opti $this->raiseExceptionOccurredJobEvent( $connectionName, $job, $e ); - } catch(Exception $exception) { + } catch (Exception $exception) { $this->raiseFailedJobEvent($connectionName, $job, $exception); } finally { if (! $job->isDeleted()) { From c1de4911bba6ad3b273106f3af26b14bafc8a204 Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 16:35:23 -0400 Subject: [PATCH 03/11] Try catch failJob failJob gets called from 2 separate places. Better to put this code around a try catch instead of in the try catch in handleJobException --- src/Illuminate/Queue/Worker.php | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index 005c800488bf..9119af50f94c 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -3,6 +3,7 @@ namespace Illuminate\Queue; use Exception; +use RuntimeException; use Throwable; use Illuminate\Contracts\Queue\Job; use Illuminate\Contracts\Events\Dispatcher; @@ -234,8 +235,6 @@ protected function handleJobException($connectionName, $job, WorkerOptions $opti $this->raiseExceptionOccurredJobEvent( $connectionName, $job, $e ); - } catch(Exception $exception) { - $this->raiseFailedJobEvent($connectionName, $job, $exception); } finally { if (! $job->isDeleted()) { $job->release($options->delay); @@ -266,8 +265,6 @@ protected function markJobAsFailedIfAlreadyExceedsMaxAttempts($connectionName, $ ); $this->failJob($connectionName, $job, $e); - - throw $e; } /** @@ -303,14 +300,20 @@ protected function failJob($connectionName, $job, $e) return; } - // If the job has failed, we will delete it, call the "failed" method and then call - // an event indicating the job has failed so it can be logged if needed. This is - // to allow every developer to better keep monitor of their failed queue jobs. - $job->delete(); + try { + // If the job has failed, we will delete it, call the "failed" method and then call + // an event indicating the job has failed so it can be logged if needed. This is + // to allow every developer to better keep monitor of their failed queue jobs. + $job->delete(); - $job->failed($e); + $job->failed($e); + } catch(Exception $exception) { + $e = new RuntimeException($exception->getMessage(), $exception->getCode(), $e); + } $this->raiseFailedJobEvent($connectionName, $job, $e); + + throw $e; } /** From 5d8d7bab941e19894400a5cda5a61b81828b98ae Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 16:38:33 -0400 Subject: [PATCH 04/11] Remove merge comments --- src/Illuminate/Queue/Worker.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index dc4de063ae14..9119af50f94c 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -235,11 +235,6 @@ protected function handleJobException($connectionName, $job, WorkerOptions $opti $this->raiseExceptionOccurredJobEvent( $connectionName, $job, $e ); -<<<<<<< HEAD -======= - } catch (Exception $exception) { - $this->raiseFailedJobEvent($connectionName, $job, $exception); ->>>>>>> origin/fix-queue-failed-job-event } finally { if (! $job->isDeleted()) { $job->release($options->delay); From 1cfcbe2a44ff66c0733be17c5529961e52f60606 Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 16:40:48 -0400 Subject: [PATCH 05/11] Forgot a space --- src/Illuminate/Queue/Worker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index 9119af50f94c..82726d8a5e24 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -307,7 +307,7 @@ protected function failJob($connectionName, $job, $e) $job->delete(); $job->failed($e); - } catch(Exception $exception) { + } catch (Exception $exception) { $e = new RuntimeException($exception->getMessage(), $exception->getCode(), $e); } From f86278bc903cacbff027d598e2f549d10dd3993a Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 16:51:58 -0400 Subject: [PATCH 06/11] Catch exception --- src/Illuminate/Queue/Worker.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index 82726d8a5e24..28eb5981f1e5 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -293,6 +293,8 @@ protected function markJobAsFailedIfHasExceededMaxAttempts( * @param \Illuminate\Contracts\Queue\Job $job * @param \Exception $e * @return void + * + * @throws \Exception */ protected function failJob($connectionName, $job, $e) { @@ -307,9 +309,7 @@ protected function failJob($connectionName, $job, $e) $job->delete(); $job->failed($e); - } catch (Exception $exception) { - $e = new RuntimeException($exception->getMessage(), $exception->getCode(), $e); - } + } catch (Exception $e) {} $this->raiseFailedJobEvent($connectionName, $job, $e); From bc931b01629282da678341644bf1b6ba651a9325 Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 16:53:25 -0400 Subject: [PATCH 07/11] Style changes --- src/Illuminate/Queue/Worker.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index 28eb5981f1e5..c78afaad6fa8 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -3,7 +3,6 @@ namespace Illuminate\Queue; use Exception; -use RuntimeException; use Throwable; use Illuminate\Contracts\Queue\Job; use Illuminate\Contracts\Events\Dispatcher; @@ -309,7 +308,8 @@ protected function failJob($connectionName, $job, $e) $job->delete(); $job->failed($e); - } catch (Exception $e) {} + } catch (Exception $e) { + } $this->raiseFailedJobEvent($connectionName, $job, $e); From b5e4f2882ccc403ff11650e96fae114aaa2926c1 Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 18:03:54 -0400 Subject: [PATCH 08/11] Catch exception --- src/Illuminate/Queue/Worker.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index c78afaad6fa8..3956fa10498c 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -309,6 +309,7 @@ protected function failJob($connectionName, $job, $e) $job->failed($e); } catch (Exception $e) { + } $this->raiseFailedJobEvent($connectionName, $job, $e); From cc03e83e22680fedf7ba34f711b175bf2d5a8104 Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Tue, 1 Nov 2016 18:06:54 -0400 Subject: [PATCH 09/11] Remove line --- src/Illuminate/Queue/Worker.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index 3956fa10498c..c78afaad6fa8 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -309,7 +309,6 @@ protected function failJob($connectionName, $job, $e) $job->failed($e); } catch (Exception $e) { - } $this->raiseFailedJobEvent($connectionName, $job, $e); From 1191786e6c1f1cf9be5559cbf14398e7647dccea Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Wed, 2 Nov 2016 08:55:40 -0400 Subject: [PATCH 10/11] Fix tests --- src/Illuminate/Queue/Worker.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index c78afaad6fa8..b50960b5ed02 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -264,6 +264,8 @@ protected function markJobAsFailedIfAlreadyExceedsMaxAttempts($connectionName, $ ); $this->failJob($connectionName, $job, $e); + + throw $e; } /** @@ -292,8 +294,6 @@ protected function markJobAsFailedIfHasExceededMaxAttempts( * @param \Illuminate\Contracts\Queue\Job $job * @param \Exception $e * @return void - * - * @throws \Exception */ protected function failJob($connectionName, $job, $e) { @@ -308,12 +308,10 @@ protected function failJob($connectionName, $job, $e) $job->delete(); $job->failed($e); - } catch (Exception $e) { + } catch (Exception $exception) { } $this->raiseFailedJobEvent($connectionName, $job, $e); - - throw $e; } /** From f052310dc2d73058d9a614c9cd67fa91b3864bf3 Mon Sep 17 00:00:00 2001 From: Brian McLachlin Date: Wed, 2 Nov 2016 09:29:23 -0400 Subject: [PATCH 11/11] Replaced empty catch with finally --- src/Illuminate/Queue/Worker.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Queue/Worker.php b/src/Illuminate/Queue/Worker.php index b50960b5ed02..7fa28f9b27aa 100644 --- a/src/Illuminate/Queue/Worker.php +++ b/src/Illuminate/Queue/Worker.php @@ -308,10 +308,9 @@ protected function failJob($connectionName, $job, $e) $job->delete(); $job->failed($e); - } catch (Exception $exception) { + } finally { + $this->raiseFailedJobEvent($connectionName, $job, $e); } - - $this->raiseFailedJobEvent($connectionName, $job, $e); } /**