Move exception logger to a dedicated class (#4)

* Move exception logger to a dedicated class * minor * cs
Happyr · Jun 3, 2020 · 9d714d9 · 9d714d9
1 parent b9222cd
commit 9d714d9
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 92 deletions.
diff --git a/Readme.md b/Readme.md
@@ -1,7 +1,7 @@
 # Bref Messenger failure strategies
 
 So you have fallen in love with [Bref](https://bref.sh) and you really want to use
-Symfony's excellent Messenger component. You've probably also installed the 
+Symfony's excellent Messenger component. You've probably also installed the
 [Bref Symfony Messenger bundle](https://github.com/brefphp/symfony-messenger)
 that allows you to publish messages on SQS and SNS etc. But you are missing something...
 You want to be able to use Symfony Messenger retry strategies, right?
@@ -15,13 +15,13 @@ composer require happyr/bref-messenger-failure-strategies
 ```
 
 Now you have a class called `Happyr\BrefMessenger\SymfonyBusDriver` that implements
-`Bref\Symfony\Messenger\Service\BusDriver`. Feel free to configure your consumers with this 
-new class. 
+`Bref\Symfony\Messenger\Service\BusDriver`. Feel free to configure your consumers with this
+new class.
 
 ## Example
 
 On each consumer you can choose to let Symfony handle failures as described in
-[the documentation](https://symfony.com/doc/current/messenger.html#retries-failures). 
+[the documentation](https://symfony.com/doc/current/messenger.html#retries-failures).
 
 
 ```yaml
@@ -42,7 +42,11 @@ framework:
                   max_delay: 60
 
 services:
-    Happyr\BrefMessenger\SymfonyBusDriver: 
+    Happyr\BrefMessenger\ExceptionLogger:
+        autowire: true
+        autoconfigure: true
+
+    Happyr\BrefMessenger\SymfonyBusDriver:
         autowire: true
 
     Bref\Symfony\Messenger\Service\Sqs\SqsConsumer:
@@ -76,9 +80,9 @@ services:
 
 ```
 
-Make sure you re-run the failure queue time to time. The following config will 
+Make sure you re-run the failure queue time to time. The following config will
 run a script for 5 seconds every 30 minutes. It will run for 5 seconds even though
-no messages has failed. 
+no messages has failed.
 
 ```yaml
 # serverless.yml

diff --git a/src/ExceptionLogger.php b/src/ExceptionLogger.php
@@ -0,0 +1,81 @@
+<?php
+
+namespace Happyr\BrefMessenger;
+
+use Psr\Log\LoggerInterface;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\Messenger\Envelope;
+use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
+use Symfony\Component\Messenger\Exception\HandlerFailedException;
+use Symfony\Component\Messenger\Exception\ValidationFailedException;
+use Symfony\Component\Validator\ConstraintViolationInterface;
+
+class ExceptionLogger implements EventSubscriberInterface
+{
+    private $logger;
+
+    public function __construct(LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+    }
+
+    public static function getSubscribedEvents()
+    {
+        return [
+            WorkerMessageFailedEvent::class => ['onException', 20],
+        ];
+    }
+
+    public function onException(WorkerMessageFailedEvent $event)
+    {
+        $envelope = $event->getEnvelope();
+        $throwable = $event->getThrowable();
+        $firstNestedException = null;
+        if ($throwable instanceof HandlerFailedException) {
+            $envelope = $throwable->getEnvelope();
+            $nestedExceptions = $throwable->getNestedExceptions();
+            $firstNestedException = $nestedExceptions[array_key_first($nestedExceptions)];
+        }
+
+        if ($throwable instanceof ValidationFailedException) {
+            $this->logValidationException($throwable);
+        } else {
+            $this->logException($envelope, $throwable, $event->getReceiverName(), $firstNestedException);
+        }
+    }
+
+    private function logValidationException(ValidationFailedException $exception): void
+    {
+        $violations = $exception->getViolations();
+        $violationMessages = [];
+        /** @var ConstraintViolationInterface $v */
+        foreach ($violations as $v) {
+            $violationMessages[] = \sprintf('%s: %s', $v->getPropertyPath(), (string) $v->getMessage());
+        }
+
+        $this->logger->error('{class} did failed validation.', [
+            'class' => get_class($exception->getViolatingMessage()),
+            'violations' => \json_encode($violationMessages),
+        ]);
+    }
+
+    private function logException(Envelope $envelope, \Throwable $throwable, string $transportName, ?\Throwable $firstNestedException): void
+    {
+        $message = $envelope->getMessage();
+        $context = [
+            'exception' => $throwable,
+            'message' => $message,
+            'transport' => $transportName,
+            'class' => \get_class($message),
+        ];
+
+        if (null === $firstNestedException) {
+            $logMessage = 'Dispatching {class} caused an exception: '.$throwable->getMessage();
+        } else {
+            $logMessage = 'Handling {class} caused an HandlerFailedException: '.$throwable->getMessage();
+            $context['first_exception'] = $firstNestedException;
+        }
+
+        $this->logger->error($logMessage, $context);
+    }
+}
diff --git a/src/SymfonyBusDriver.php b/src/SymfonyBusDriver.php
@@ -10,12 +10,9 @@
 use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent;
-use Symfony\Component\Messenger\Exception\HandlerFailedException;
-use Symfony\Component\Messenger\Exception\ValidationFailedException;
 use Symfony\Component\Messenger\MessageBusInterface;
 use Symfony\Component\Messenger\Stamp\ConsumedByWorkerStamp;
 use Symfony\Component\Messenger\Stamp\ReceivedStamp;
-use Symfony\Component\Validator\ConstraintViolationInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 
 /**
@@ -44,19 +41,6 @@ public function putEnvelopeOnBus(MessageBusInterface $bus, Envelope $envelope, s
         try {
             $envelope = $bus->dispatch($envelope->with(new ReceivedStamp($transportName), new ConsumedByWorkerStamp()));
         } catch (\Throwable $throwable) {
-            $firstNestedException = null;
-            if ($throwable instanceof HandlerFailedException) {
-                $envelope = $throwable->getEnvelope();
-                $nestedExceptions = $throwable->getNestedExceptions();
-                $firstNestedException = $nestedExceptions[array_key_first($nestedExceptions)];
-            }
-
-            if ($throwable instanceof ValidationFailedException) {
-                $this->logValidationException($throwable);
-            } else {
-                $this->logException($envelope, $throwable, $transportName, $firstNestedException);
-            }
-
             $this->eventDispatcher->dispatch(new WorkerMessageFailedEvent($envelope, $transportName, $throwable));
 
             return;
@@ -71,39 +55,4 @@ public function putEnvelopeOnBus(MessageBusInterface $bus, Envelope $envelope, s
             'class' => \get_class($message),
         ]);
     }
-
-    private function logValidationException(ValidationFailedException $exception): void
-    {
-        $violations = $exception->getViolations();
-        $violationMessages = [];
-        /** @var ConstraintViolationInterface $v */
-        foreach ($violations as $v) {
-            $violationMessages[] = \sprintf('%s: %s', $v->getPropertyPath(), (string) $v->getMessage());
-        }
-
-        $this->logger->error('{class} did failed validation.', [
-            'class' => get_class($exception->getViolatingMessage()),
-            'violations' => \json_encode($violationMessages),
-        ]);
-    }
-
-    private function logException(Envelope $envelope, \Throwable $throwable, string $transportName, ?\Throwable $firstNestedException): void
-    {
-        $message = $envelope->getMessage();
-        $context = [
-            'exception' => $throwable,
-            'message' => $message,
-            'transport' => $transportName,
-            'class' => \get_class($message),
-        ];
-
-        if (null === $firstNestedException) {
-            $logMessage = 'Dispatching {class} caused an exception: '.$throwable->getMessage();
-        } else {
-            $logMessage = 'Handling {class} caused an HandlerFailedException: '.$throwable->getMessage();
-            $context['first_exception'] = $firstNestedException;
-        }
-
-        $this->logger->error($logMessage, $context);
-    }
 }
diff --git a/tests/SymfonyBusDriverTest.php b/tests/SymfonyBusDriverTest.php
@@ -8,13 +8,10 @@
 use Happyr\BrefMessenger\Test\Fixture\DummyEventDispatcher;
 use Happyr\BrefMessenger\Test\Fixture\DummyMessage;
 use PHPUnit\Framework\TestCase;
-use Psr\Log\AbstractLogger;
 use Psr\Log\NullLogger;
 use Symfony\Component\Messenger\Envelope;
-use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
 use Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent;
-use Symfony\Component\Messenger\Exception\HandlerFailedException;
 use Symfony\Component\Messenger\MessageBusInterface;
 use Symfony\Component\Messenger\Stamp\ConsumedByWorkerStamp;
 use Symfony\Component\Messenger\Stamp\ReceivedStamp;
@@ -49,35 +46,4 @@ public function testBusIsDispatchingMessages()
         $this->assertInstanceOf(WorkerMessageReceivedEvent::class, $events[2]);
         $this->assertInstanceOf(WorkerMessageHandledEvent::class, $events[3]);
     }
-
-    public function testLogHandlerFailedException()
-    {
-        $fooMessage = new DummyMessage('Foo');
-        $bus = $this->getMockBuilder(MessageBusInterface::class)->getMock();
-
-        $envelope = new Envelope($fooMessage, [new ReceivedStamp('transport'), new ConsumedByWorkerStamp()]);
-        $bus->expects($this->at(0))->method('dispatch')
-            ->with($envelope)
-            ->willThrowException(new HandlerFailedException($envelope, [new \RuntimeException('Foo')]));
-
-        $logger = $this->getMockBuilder(AbstractLogger::class)
-            ->disableOriginalConstructor()
-            ->onlyMethods(['log'])
-            ->getMock();
-        $logger->expects($this->once())->method('log')
-            ->with('error', $this->callback(function ($message) {
-                $this->assertEquals('Handling {class} caused an HandlerFailedException: Foo', $message);
-
-                return true;
-            }), $this->anything());
-
-        $dispatcher = new DummyEventDispatcher();
-        $worker = new SymfonyBusDriver($logger, $dispatcher);
-        $worker->putEnvelopeOnBus($bus, new Envelope($fooMessage), 'transport');
-
-        $events = $dispatcher->getEvents();
-        $this->assertCount(2, $events);
-        $this->assertInstanceOf(WorkerMessageReceivedEvent::class, $events[0]);
-        $this->assertInstanceOf(WorkerMessageFailedEvent::class, $events[1]);
-    }
 }