Allow use of current hub scope in Messenger Listener (#339)

* Alter MessengerListener to use current hub with methods for capturing exception and accessing client. This allows the scope of the current hub to be updated in a prior event listener. * Inject the hub rather than rely upon singleton instance. * Correctly typehint and remove phpstan exclusions for the MessengerListenerTest properties. * Merge changes from #340. * Fix up new test. Co-authored-by: Steve Hall <steve.hall+gitlab@rg456.co.uk>
getsentry · May 7, 2020 · b64b6c0 · b64b6c0
1 parent 942ae46
commit b64b6c0
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 29 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -270,11 +270,6 @@ parameters:
 			count: 1
 			path: test/EventListener/ConsoleListenerTest.php
 
-		-
-			message: "#^Property Sentry\\\\SentryBundle\\\\Test\\\\EventListener\\\\MessengerListenerTest\\:\\:\\$client has no typehint specified\\.$#"
-			count: 1
-			path: test/EventListener/MessengerListenerTest.php
-
 		-
 			message: "#^Class Symfony\\\\Component\\\\Messenger\\\\Event\\\\WorkerMessageHandledEvent constructor invoked with 3 parameters, 2 required\\.$#"
 			count: 1

diff --git a/src/EventListener/MessengerListener.php b/src/EventListener/MessengerListener.php
@@ -3,29 +3,30 @@
 namespace Sentry\SentryBundle\EventListener;
 
 use Sentry\FlushableClientInterface;
+use Sentry\State\HubInterface;
 use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
 use Symfony\Component\Messenger\Exception\HandlerFailedException;
 
 final class MessengerListener
 {
     /**
-     * @var FlushableClientInterface
+     * @var HubInterface
      */
-    private $client;
+    private $hub;
 
     /**
      * @var bool
      */
     private $captureSoftFails;
 
     /**
-     * @param FlushableClientInterface $client
-     * @param bool                     $captureSoftFails
+     * @param HubInterface $hub
+     * @param bool         $captureSoftFails
      */
-    public function __construct(FlushableClientInterface $client, bool $captureSoftFails = true)
+    public function __construct(HubInterface $hub, bool $captureSoftFails = true)
     {
-        $this->client = $client;
+        $this->hub = $hub;
         $this->captureSoftFails = $captureSoftFails;
     }
 
@@ -43,10 +44,10 @@ public function onWorkerMessageFailed(WorkerMessageFailedEvent $event): void
 
         if ($error instanceof HandlerFailedException) {
             foreach ($error->getNestedExceptions() as $nestedException) {
-                $this->client->captureException($nestedException);
+                $this->hub->captureException($nestedException);
             }
         } else {
-            $this->client->captureException($error);
+            $this->hub->captureException($error);
         }
 
         $this->flush();
@@ -64,6 +65,9 @@ public function onWorkerMessageHandled(WorkerMessageHandledEvent $event): void
 
     private function flush(): void
     {
-        $this->client->flush();
+        $client = $this->hub->getClient();
+        if ($client instanceof FlushableClientInterface) {
+            $client->flush();
+        }
     }
 }
diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml
@@ -53,7 +53,7 @@
         </service>
 
         <service id="Sentry\SentryBundle\EventListener\MessengerListener" class="Sentry\SentryBundle\EventListener\MessengerListener" public="false">
-            <argument type="service" id="Sentry\FlushableClientInterface" />
+            <argument type="service" id="Sentry\State\HubInterface" />
             <tag name="kernel.event_listener" event="Symfony\Component\Messenger\Event\WorkerMessageFailedEvent" method="onWorkerMessageFailed" priority="%sentry.listener_priorities.worker_error%" />
             <tag name="kernel.event_listener" event="Symfony\Component\Messenger\Event\WorkerMessageHandledEvent" method="onWorkerMessageHandled" priority="%sentry.listener_priorities.worker_error%" />
         </service>

diff --git a/test/EventListener/MessengerListenerTest.php b/test/EventListener/MessengerListenerTest.php
@@ -5,6 +5,7 @@
 use Sentry\FlushableClientInterface;
 use Sentry\SentryBundle\EventListener\MessengerListener;
 use Sentry\SentryBundle\Test\BaseTestCase;
+use Sentry\State\HubInterface;
 use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
@@ -13,13 +14,18 @@
 
 class MessengerListenerTest extends BaseTestCase
 {
+    /** @var \Prophecy\Prophecy\ObjectProphecy|FlushableClientInterface */
     private $client;
+    /** @var \Prophecy\Prophecy\ObjectProphecy|HubInterface */
+    private $hub;
 
     protected function setUp(): void
     {
         parent::setUp();
 
         $this->client = $this->prophesize(FlushableClientInterface::class);
+        $this->hub = $this->prophesize(HubInterface::class);
+        $this->hub->getClient()->willReturn($this->client);
     }
 
     public function testSoftFailsAreRecorded(): void
@@ -34,10 +40,10 @@ public function testSoftFailsAreRecorded(): void
         $error = new \RuntimeException();
         $wrappedError = new HandlerFailedException($envelope, [$error]);
 
-        $this->client->captureException($error)->shouldBeCalled();
+        $this->hub->captureException($error)->shouldBeCalled();
         $this->client->flush()->shouldBeCalled();
 
-        $listener = new MessengerListener($this->client->reveal(), true);
+        $listener = new MessengerListener($this->hub->reveal(), true);
         $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, true);
 
         $listener->onWorkerMessageFailed($event);
@@ -54,10 +60,10 @@ public function testNonMessengerErrorsAreRecorded(): void
 
         $error = new \RuntimeException();
 
-        $this->client->captureException($error)->shouldBeCalled();
+        $this->hub->captureException($error)->shouldBeCalled();
         $this->client->flush()->shouldBeCalled();
 
-        $listener = new MessengerListener($this->client->reveal(), true);
+        $listener = new MessengerListener($this->hub->reveal(), true);
         $event = $this->getMessageFailedEvent($envelope, 'receiver', $error, false);
 
         $listener->onWorkerMessageFailed($event);
@@ -75,10 +81,10 @@ public function testHardFailsAreRecorded(): void
         $error = new \RuntimeException();
         $wrappedError = new HandlerFailedException($envelope, [$error]);
 
-        $this->client->captureException($error)->shouldBeCalled();
+        $this->hub->captureException($error)->shouldBeCalled();
         $this->client->flush()->shouldBeCalled();
 
-        $listener = new MessengerListener($this->client->reveal(), true);
+        $listener = new MessengerListener($this->hub->reveal(), true);
         $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, false);
 
         $listener->onWorkerMessageFailed($event);
@@ -96,10 +102,10 @@ public function testSoftFailsAreNotRecorded(): void
         $error = new \RuntimeException();
         $wrappedError = new HandlerFailedException($envelope, [$error]);
 
-        $this->client->captureException($error)->shouldNotBeCalled();
+        $this->hub->captureException($error)->shouldNotBeCalled();
         $this->client->flush()->shouldNotBeCalled();
 
-        $listener = new MessengerListener($this->client->reveal(), false);
+        $listener = new MessengerListener($this->hub->reveal(), false);
         $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, true);
 
         $listener->onWorkerMessageFailed($event);
@@ -117,10 +123,10 @@ public function testHardFailsAreRecordedWithCaptureSoftDisabled(): void
         $error = new \RuntimeException();
         $wrappedError = new HandlerFailedException($envelope, [$error]);
 
-        $this->client->captureException($error)->shouldBeCalled();
+        $this->hub->captureException($error)->shouldBeCalled();
         $this->client->flush()->shouldBeCalled();
 
-        $listener = new MessengerListener($this->client->reveal(), false);
+        $listener = new MessengerListener($this->hub->reveal(), false);
         $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, false);
 
         $listener->onWorkerMessageFailed($event);
@@ -140,11 +146,11 @@ public function testHandlerFailedExceptionIsUnwrapped(): void
 
         $event = $this->getMessageFailedEvent($envelope, 'receiver', $wrappedError, false);
 
-        $this->client->captureException($error1)->shouldBeCalled();
-        $this->client->captureException($error2)->shouldBeCalled();
+        $this->hub->captureException($error1)->shouldBeCalled();
+        $this->hub->captureException($error2)->shouldBeCalled();
         $this->client->flush()->shouldBeCalled();
 
-        $listener = new MessengerListener($this->client->reveal());
+        $listener = new MessengerListener($this->hub->reveal());
         $listener->onWorkerMessageFailed($event);
     }
 
@@ -155,7 +161,7 @@ public function testClientIsFlushedWhenMessageHandled(): void
         }
 
         $this->client->flush()->shouldBeCalled();
-        $listener = new MessengerListener($this->client->reveal());
+        $listener = new MessengerListener($this->hub->reveal());
 
         $message = (object) ['foo' => 'bar'];
         $envelope = Envelope::wrap($message);