[Messenger] deprecate LoggingMiddleware in favor of providing a logge…

…r to SendMessageMiddleware
symfony · Mar 14, 2019 · e362917 · e362917
1 parent 7ccec2a
commit e362917
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,15 +6,13 @@ CHANGELOG
 
  * Added `PhpSerializer` which uses PHP's native `serialize()` and
    `unserialize()` to serialize messages to a transport
-
  * [BC BREAK] If no serializer were passed, the default serializer
    changed from `Serializer` to `PhpSerializer` inside `AmqpReceiver`,
    `AmqpSender`, `AmqpTransport` and `AmqpTransportFactory`.
-
  * Added `TransportException` to mark an exception transport-related
-
  * [BC BREAK] If listening to exceptions while using `AmqpSender` or `AmqpReceiver`, `\AMQPException` is
    no longer thrown in favor of `TransportException`.
+ * Deprecated `LoggingMiddleware`, pass a logger to `SendMessageMiddleware` instead.
 
 4.2.0
 -----

diff --git a/Command/ConsumeMessagesCommand.php b/Command/ConsumeMessagesCommand.php
@@ -170,8 +170,8 @@ protected function execute(InputInterface $input, OutputInterface $output): void
 
         $io->comment('Quit the worker with CONTROL-C.');
 
-        if (!$output->isDebug()) {
-            $io->comment('Re-run the command with a -vvv option to see logs about consumed messages.');
+        if (OutputInterface::VERBOSITY_VERBOSE > $output->getVerbosity()) {
+            $io->comment('Re-run the command with a -vv option to see logs about consumed messages.');
         }
 
         $worker = new Worker($receiver, $bus);

diff --git a/Middleware/HandleMessageMiddleware.php b/Middleware/HandleMessageMiddleware.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Messenger\Middleware;
 
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
 use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;
 use Symfony\Component\Messenger\Handler\HandlersLocatorInterface;
@@ -23,13 +25,16 @@
  */
 class HandleMessageMiddleware implements MiddlewareInterface
 {
+    use LoggerAwareTrait;
+
     private $handlersLocator;
     private $allowNoHandlers;
 
     public function __construct(HandlersLocatorInterface $handlersLocator, bool $allowNoHandlers = false)
     {
         $this->handlersLocator = $handlersLocator;
         $this->allowNoHandlers = $allowNoHandlers;
+        $this->logger = new NullLogger();
     }
 
     /**
@@ -41,11 +46,24 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
     {
         $handler = null;
         $message = $envelope->getMessage();
+
+        $context = [
+            'message' => $message,
+            'class' => \get_class($message),
+        ];
+
         foreach ($this->handlersLocator->getHandlers($envelope) as $alias => $handler) {
-            $envelope = $envelope->with(HandledStamp::fromCallable($handler, $handler($message), \is_string($alias) ? $alias : null));
+            $handledStamp = HandledStamp::fromCallable($handler, $handler($message), \is_string($alias) ? $alias : null);
+            $envelope = $envelope->with($handledStamp);
+            $this->logger->info('Message "{class}" handled by "{handler}"', $context + ['handler' => $handledStamp->getCallableName()]);
         }
-        if (null === $handler && !$this->allowNoHandlers) {
-            throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', \get_class($envelope->getMessage())));
+
+        if (null === $handler) {
+            if (!$this->allowNoHandlers) {
+                throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $context['class']));
+            }
+
+            $this->logger->info('No handler for message "{class}"', $context);
         }
 
         return $stack->next()->handle($envelope, $stack);

diff --git a/Middleware/LoggingMiddleware.php b/Middleware/LoggingMiddleware.php
@@ -11,13 +11,15 @@
 
 namespace Symfony\Component\Messenger\Middleware;
 
+@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.3, pass a logger to SendMessageMiddleware instead.', LoggingMiddleware::class), E_USER_DEPRECATED);
+
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Messenger\Envelope;
 
 /**
  * @author Samuel Roze <samuel.roze@gmail.com>
  *
- * @experimental in 4.2
+ * @deprecated since 4.3, pass a logger to SendMessageMiddleware instead
  */
 class LoggingMiddleware implements MiddlewareInterface
 {

diff --git a/Middleware/SendMessageMiddleware.php b/Middleware/SendMessageMiddleware.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Messenger\Middleware;
 
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
 use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Stamp\ReceivedStamp;
 use Symfony\Component\Messenger\Stamp\SentStamp;
@@ -24,31 +26,48 @@
  */
 class SendMessageMiddleware implements MiddlewareInterface
 {
+    use LoggerAwareTrait;
+
     private $sendersLocator;
 
     public function __construct(SendersLocatorInterface $sendersLocator)
     {
         $this->sendersLocator = $sendersLocator;
+        $this->logger = new NullLogger();
     }
 
     /**
      * {@inheritdoc}
      */
     public function handle(Envelope $envelope, StackInterface $stack): Envelope
     {
-        if ($envelope->all(ReceivedStamp::class)) {
-            // it's a received message, do not send it back
-            return $stack->next()->handle($envelope, $stack);
-        }
+        $context = [
+            'message' => $envelope->getMessage(),
+            'class' => \get_class($envelope->getMessage()),
+        ];
+
         $handle = false;
         $sender = null;
 
-        foreach ($this->sendersLocator->getSenders($envelope, $handle) as $alias => $sender) {
-            $envelope = $sender->send($envelope)->with(new SentStamp(\get_class($sender), \is_string($alias) ? $alias : null));
-        }
+        try {
+            if ($envelope->all(ReceivedStamp::class)) {
+                // it's a received message, do not send it back
+                $this->logger->info('Received message "{class}"', $context);
+            } else {
+                foreach ($this->sendersLocator->getSenders($envelope, $handle) as $alias => $sender) {
+                    $this->logger->info('Sending message "{class}" with "{sender}"', $context + ['sender' => \get_class($sender)]);
+                    $envelope = $sender->send($envelope)->with(new SentStamp(\get_class($sender), \is_string($alias) ? $alias : null));
+                }
+            }
+
+            if (null === $sender || $handle) {
+                return $stack->next()->handle($envelope, $stack);
+            }
+        } catch (\Throwable $e) {
+            $context['exception'] = $e;
+            $this->logger->warning('An exception occurred while handling message "{class}"', $context);
 
-        if (null === $sender || $handle) {
-            return $stack->next()->handle($envelope, $stack);
+            throw $e;
         }
 
         // message should only be sent and not be handled by the next middleware

diff --git a/Tests/Middleware/LoggingMiddlewareTest.php b/Tests/Middleware/LoggingMiddlewareTest.php
@@ -17,6 +17,9 @@
 use Symfony\Component\Messenger\Test\Middleware\MiddlewareTestCase;
 use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage;
 
+/**
+ * @group legacy
+ */
 class LoggingMiddlewareTest extends MiddlewareTestCase
 {
     public function testDebugLogAndNextMiddleware()

diff --git a/composer.json b/composer.json
@@ -16,10 +16,10 @@
         }
     ],
     "require": {
-        "php": "^7.1.3"
+        "php": "^7.1.3",
+        "psr/log": "~1.0"
     },
     "require-dev": {
-        "psr/log": "~1.0",
         "symfony/console": "~3.4|~4.0",
         "symfony/dependency-injection": "~3.4.19|^4.1.8",
         "symfony/http-kernel": "~3.4|~4.0",