From 4e2f841855bb5defd208b763b657a543b9665ca4 Mon Sep 17 00:00:00 2001 From: Malcolm Fell Date: Thu, 7 May 2020 12:27:16 +1200 Subject: [PATCH 1/6] Capture exceptions thrown by all messenger handlers --- src/EventListener/MessengerListener.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/EventListener/MessengerListener.php b/src/EventListener/MessengerListener.php index bc3d6e68..9eb1c026 100644 --- a/src/EventListener/MessengerListener.php +++ b/src/EventListener/MessengerListener.php @@ -41,12 +41,16 @@ public function onWorkerMessageFailed(WorkerMessageFailedEvent $event): void $error = $event->getThrowable(); - if ($error instanceof HandlerFailedException && null !== $error->getPrevious()) { - // Unwrap the messenger exception to get the original error - $error = $error->getPrevious(); + if (!$error instanceof HandlerFailedException) { + // All errors thrown during handling a command are captured by the HandleMessageMiddleware, + // and are raised inside a HandlerFailedException. Type check for safety. + return; + } + + foreach ($error->getNestedExceptions() as $nestedException) { + $this->client->captureException($nestedException); } - $this->client->captureException($error); $this->client->flush(); } From 6ebf1d9e5683de6566a6b6dfe3cfe852c10a3e14 Mon Sep 17 00:00:00 2001 From: Malcolm Fell Date: Thu, 7 May 2020 12:27:43 +1200 Subject: [PATCH 2/6] Standardise flush calls on messenger listener --- src/EventListener/MessengerListener.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/EventListener/MessengerListener.php b/src/EventListener/MessengerListener.php index 9eb1c026..7926ec7a 100644 --- a/src/EventListener/MessengerListener.php +++ b/src/EventListener/MessengerListener.php @@ -51,7 +51,7 @@ public function onWorkerMessageFailed(WorkerMessageFailedEvent $event): void $this->client->captureException($nestedException); } - $this->client->flush(); + $this->flush(); } /** @@ -61,6 +61,11 @@ public function onWorkerMessageHandled(WorkerMessageHandledEvent $event): void { // Flush normally happens at shutdown... which only happens in the worker if it is run with a lifecycle limit // such as --time=X or --limit=Y. Flush immediately in a background worker. + $this->flush(); + } + + private function flush(): void + { $this->client->flush(); } } From be5e940c44963e1a3709ae71284e01164799a535 Mon Sep 17 00:00:00 2001 From: Malcolm Fell Date: Thu, 7 May 2020 12:40:35 +1200 Subject: [PATCH 3/6] Wrap all test errors in HandlerFailed --- test/EventListener/MessengerListenerTest.php | 32 ++++++++++++-------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/test/EventListener/MessengerListenerTest.php b/test/EventListener/MessengerListenerTest.php index 5e85f663..19186398 100644 --- a/test/EventListener/MessengerListenerTest.php +++ b/test/EventListener/MessengerListenerTest.php @@ -28,15 +28,17 @@ public function testSoftFailsAreRecorded(): void self::markTestSkipped('Messenger not supported in this environment.'); } + $message = (object) ['foo' => 'bar']; + $envelope = Envelope::wrap($message); + $error = new \RuntimeException(); + $wrappedError = new HandlerFailedException($envelope, [$error]); $this->client->captureException($error)->shouldBeCalled(); $this->client->flush()->shouldBeCalled(); $listener = new MessengerListener($this->client->reveal(), true); - $message = (object) ['foo' => 'bar']; - $envelope = Envelope::wrap($message); - $event = $this->getMessageFailedEvent($envelope, 'receiver', $error, true); + $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, true); $listener->onWorkerMessageFailed($event); } @@ -47,15 +49,17 @@ public function testHardFailsAreRecorded(): void self::markTestSkipped('Messenger not supported in this environment.'); } + $message = (object) ['foo' => 'bar']; + $envelope = Envelope::wrap($message); + $error = new \RuntimeException(); + $wrappedError = new HandlerFailedException($envelope, [$error]); $this->client->captureException($error)->shouldBeCalled(); $this->client->flush()->shouldBeCalled(); $listener = new MessengerListener($this->client->reveal(), true); - $message = (object) ['foo' => 'bar']; - $envelope = Envelope::wrap($message); - $event = $this->getMessageFailedEvent($envelope, 'receiver', $error, false); + $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, false); $listener->onWorkerMessageFailed($event); } @@ -66,15 +70,17 @@ public function testSoftFailsAreNotRecorded(): void self::markTestSkipped('Messenger not supported in this environment.'); } + $message = (object) ['foo' => 'bar']; + $envelope = Envelope::wrap($message); + $error = new \RuntimeException(); + $wrappedError = new HandlerFailedException($envelope, [$error]); $this->client->captureException($error)->shouldNotBeCalled(); $this->client->flush()->shouldNotBeCalled(); $listener = new MessengerListener($this->client->reveal(), false); - $message = (object) ['foo' => 'bar']; - $envelope = Envelope::wrap($message); - $event = $this->getMessageFailedEvent($envelope, 'receiver', $error, true); + $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, true); $listener->onWorkerMessageFailed($event); } @@ -85,15 +91,17 @@ public function testHardFailsAreRecordedWithCaptureSoftDisabled(): void self::markTestSkipped('Messenger not supported in this environment.'); } + $message = (object) ['foo' => 'bar']; + $envelope = Envelope::wrap($message); + $error = new \RuntimeException(); + $wrappedError = new HandlerFailedException($envelope, [$error]); $this->client->captureException($error)->shouldBeCalled(); $this->client->flush()->shouldBeCalled(); $listener = new MessengerListener($this->client->reveal(), false); - $message = (object) ['foo' => 'bar']; - $envelope = Envelope::wrap($message); - $event = $this->getMessageFailedEvent($envelope, 'receiver', $error, false); + $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, false); $listener->onWorkerMessageFailed($event); } From f732f81143269b2bc35257a39091054ffb7f370d Mon Sep 17 00:00:00 2001 From: Malcolm Fell Date: Thu, 7 May 2020 12:41:39 +1200 Subject: [PATCH 4/6] Ensure all raised errors are captured --- test/EventListener/MessengerListenerTest.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/EventListener/MessengerListenerTest.php b/test/EventListener/MessengerListenerTest.php index 19186398..6a680f11 100644 --- a/test/EventListener/MessengerListenerTest.php +++ b/test/EventListener/MessengerListenerTest.php @@ -114,12 +114,14 @@ public function testHandlerFailedExceptionIsUnwrapped(): void $message = (object) ['foo' => 'bar']; $envelope = Envelope::wrap($message); - $error = new \RuntimeException(); - $wrappedError = new HandlerFailedException($envelope, [$error]); + $error1 = new \RuntimeException(); + $error2 = new \RuntimeException(); + $wrappedError = new HandlerFailedException($envelope, [$error1, $error2]); $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, false); - $this->client->captureException($error)->shouldBeCalled(); + $this->client->captureException($error1)->shouldBeCalled(); + $this->client->captureException($error2)->shouldBeCalled(); $this->client->flush()->shouldBeCalled(); $listener = new MessengerListener($this->client->reveal()); From 9037f470344085f4f585318afbeea5587a1aa86f Mon Sep 17 00:00:00 2001 From: Malcolm Fell Date: Thu, 7 May 2020 12:41:52 +1200 Subject: [PATCH 5/6] CS space --- src/EventListener/MessengerListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EventListener/MessengerListener.php b/src/EventListener/MessengerListener.php index 7926ec7a..051bc88a 100644 --- a/src/EventListener/MessengerListener.php +++ b/src/EventListener/MessengerListener.php @@ -41,7 +41,7 @@ public function onWorkerMessageFailed(WorkerMessageFailedEvent $event): void $error = $event->getThrowable(); - if (!$error instanceof HandlerFailedException) { + if (! $error instanceof HandlerFailedException) { // All errors thrown during handling a command are captured by the HandleMessageMiddleware, // and are raised inside a HandlerFailedException. Type check for safety. return; From 719adfabd0e3afeb7b3242fbf8d0116efe7a6628 Mon Sep 17 00:00:00 2001 From: Malcolm Fell Date: Thu, 7 May 2020 12:55:28 +1200 Subject: [PATCH 6/6] Capture all errors, not just HandlerFailedException --- src/EventListener/MessengerListener.php | 14 ++++++-------- test/EventListener/MessengerListenerTest.php | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/EventListener/MessengerListener.php b/src/EventListener/MessengerListener.php index 051bc88a..7ee534d5 100644 --- a/src/EventListener/MessengerListener.php +++ b/src/EventListener/MessengerListener.php @@ -41,14 +41,12 @@ public function onWorkerMessageFailed(WorkerMessageFailedEvent $event): void $error = $event->getThrowable(); - if (! $error instanceof HandlerFailedException) { - // All errors thrown during handling a command are captured by the HandleMessageMiddleware, - // and are raised inside a HandlerFailedException. Type check for safety. - return; - } - - foreach ($error->getNestedExceptions() as $nestedException) { - $this->client->captureException($nestedException); + if ($error instanceof HandlerFailedException) { + foreach ($error->getNestedExceptions() as $nestedException) { + $this->client->captureException($nestedException); + } + } else { + $this->client->captureException($error); } $this->flush(); diff --git a/test/EventListener/MessengerListenerTest.php b/test/EventListener/MessengerListenerTest.php index 6a680f11..18fa54c4 100644 --- a/test/EventListener/MessengerListenerTest.php +++ b/test/EventListener/MessengerListenerTest.php @@ -43,6 +43,26 @@ public function testSoftFailsAreRecorded(): void $listener->onWorkerMessageFailed($event); } + public function testNonMessengerErrorsAreRecorded(): void + { + if (! $this->supportsMessenger()) { + self::markTestSkipped('Messenger not supported in this environment.'); + } + + $message = (object) ['foo' => 'bar']; + $envelope = Envelope::wrap($message); + + $error = new \RuntimeException(); + + $this->client->captureException($error)->shouldBeCalled(); + $this->client->flush()->shouldBeCalled(); + + $listener = new MessengerListener($this->client->reveal(), true); + $event = $this->getMessageFailedEvent($envelope, 'receiver', $error, false); + + $listener->onWorkerMessageFailed($event); + } + public function testHardFailsAreRecorded(): void { if (! $this->supportsMessenger()) {