feature symfony#60110 [Mailer] [Transport] Allow exception logging for RoundRobinTransport mailer (jnoordsij)

This PR was squashed before being merged into the 7.4 branch.

Discussion
----------

[Mailer] [Transport] Allow exception logging for `RoundRobinTransport` mailer

| Q             | A
| ------------- | ---
| Branch?       | 7.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | N/A <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

The `RoundRobinTransport` class (and by extension the `FailoverTransport` class) allow one to mail using different mail transports, with automatic failover in case a mailer fails. In case of such a failure, exceptions are caught (and grouped in case of multiple failures) and the next transport is attempted. This ensures that as long as one transport succeeds, the mail will be delivered.

However, the current way of exception handling also means that exceptions on failing transports will never bubble up, unless all of them fail at once. This means that in case of a prolonged failure of a transport, e.g. due to misconfiguration, the only way to detect such a failure is by noting that a intended mailer is never sending mail, rather then be warned by actual exceptions of your application. If this is not detected, then one will only discover once all senders are failing, that one of them might have been failing all of the time already.

To allow one to be notified of all exceptions happening within the handling of sending a mail, I propose to add an `$logger` property to the class that implements `LoggerInterface`, that would allow one log exceptions from individual mailers used. For example, this would allow one to manually send the exception to some logging service in a structured manner, so one can address the failing transport, while sending would still continue properly as long as there's still a succeeding transport left.

Commits
-------

834f876 [Mailer] [Transport] Allow exception logging for `RoundRobinTransport` mailer

Author: fabpot

src/Symfony/Component/Mailer/CHANGELOG.md

@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+7.4
+---
+
+ * Add `logger` (constructor) property to `RoundRobinTransport`
+
 7.3
 ---
 

src/Symfony/Component/Mailer/Tests/Transport/RoundRobinTransportTest.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\Mailer\Tests\Transport;
 
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Mailer\Exception\TransportException;
 use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
 use Symfony\Component\Mailer\Transport\RoundRobinTransport;
@@ -167,19 +168,45 @@ public function testSendOneDeadMessageAlterationsDoNotPersist()
         $this->assertFalse($message->getHeaders()->has('X-Transport-1'));
     }
 
+    public function testLoggerReceivesExceptions()
+    {
+        $t1 = $this->createMock(TransportInterface::class);
+        $t1->expects($this->exactly(2))->method('send');
+
+        $ex = new TransportException();
+        $t2 = $this->createMock(TransportInterface::class);
+        $t2->expects($this->exactly(1))
+            ->method('send')
+            ->willReturnCallback(fn () => throw $ex);
+        $t2->expects($this->atLeast(1))->method('__toString')->willReturn('t2');
+
+        $log = $this->createMock(LoggerInterface::class);
+        $log->expects($this->exactly(1))
+            ->method('error')
+            ->with('Transport "t2" failed.', ['exception' => $ex]);
+
+        $t = new RoundRobinTransport([$t1, $t2], logger: $log);
+        $p = new \ReflectionProperty($t, 'cursor');
+        $p->setValue($t, 0);
+        $t->send(new RawMessage(''));
+        $this->assertTransports($t, 1, []);
+        $t->send(new RawMessage(''));
+        $this->assertTransports($t, 1, [$t2]);
+    }
+
     public function testFailureDebugInformation()
     {
         $t1 = $this->createMock(TransportInterface::class);
         $e1 = new TransportException();
         $e1->appendDebug('Debug message 1');
         $t1->expects($this->once())->method('send')->willThrowException($e1);
-        $t1->expects($this->once())->method('__toString')->willReturn('t1');
+        $t1->expects($this->atLeast(1))->method('__toString')->willReturn('t1');
 
         $t2 = $this->createMock(TransportInterface::class);
         $e2 = new TransportException();
         $e2->appendDebug('Debug message 2');
         $t2->expects($this->once())->method('send')->willThrowException($e2);
-        $t2->expects($this->once())->method('__toString')->willReturn('t2');
+        $t2->expects($this->atLeast(1))->method('__toString')->willReturn('t2');
 
         $t = new RoundRobinTransport([$t1, $t2]);
 

src/Symfony/Component/Mailer/Transport/RoundRobinTransport.php

@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Mailer\Transport;
 
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 use Symfony\Component\Mailer\Envelope;
 use Symfony\Component\Mailer\Exception\TransportException;
 use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
@@ -36,6 +38,7 @@ class RoundRobinTransport implements TransportInterface
     public function __construct(
         private array $transports,
         private int $retryPeriod = 60,
+        private LoggerInterface $logger = new NullLogger(),
     ) {
         if (!$transports) {
             throw new TransportException(\sprintf('"%s" must have at least one transport configured.', static::class));
@@ -54,6 +57,7 @@ public function send(RawMessage $message, ?Envelope $envelope = null): ?SentMess
             } catch (TransportExceptionInterface $e) {
                 $exception ??= new TransportException('All transports failed.');
                 $exception->appendDebug(\sprintf("Transport \"%s\": %s\n", $transport, $e->getDebug()));
+                $this->logger->error(\sprintf("Transport \"%s\" failed.", $transport), ['exception' => $e]);
                 $this->deadTransports[$transport] = microtime(true);
             }
         }