bug symfony#53157 [Mailer] Throw TransportException when unable to read from socket (xdanik)

nicolas-grekas · nicolas-grekas · commit 4eb23d84e42a · 2024-01-29T08:33:37.000+01:00
This PR was merged into the 5.4 branch. Discussion ---------- [Mailer] Throw `TransportException` when unable to read from socket | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? |no | Issues | None | License | MIT We are seeing error `fgets(): SSL: Connection reset by peer` multiple times a day from connection to Office 365 SMTP server (smtp.office365.com:587). It's certainly related to some kind of timeout as we are sending emails from long running queue dispatcher and error shows up only occasionally and never with the first message. We are not seeing this issue with any other SMTP server, but we have not tested much past smtp.mandrillapp.com and local MailHog. We have tried adjusting the `$pingThreshold` and `$restartThreshold` options, but without much success (well `$restartThreshold = 1` resolves the issue, but it also forces the transport to close connection after each message). Stack trace: ``` #0 /var/www/vendor/symfony/mailer/Transport/Smtp/Stream/AbstractStream.php(77): fgets(Resource(stream)) #1 /var/www/vendor/symfony/mailer/Transport/Smtp/SmtpTransport.php(315): Symfony\Component\Mailer\Transport\Smtp\Stream\AbstractStream->readLine() #2 /var/www/vendor/symfony/mailer/Transport/Smtp/SmtpTransport.php(181): Symfony\Component\Mailer\Transport\Smtp\SmtpTransport->getFullResponse() #3 /var/www/vendor/symfony/mailer/Transport/Smtp/SmtpTransport.php(140): Symfony\Component\Mailer\Transport\Smtp\SmtpTransport->executeCommand("RSET ", Array(1)) #4 /var/www/vendor/symfony/mailer/Mailer.php(45): Symfony\Component\Mailer\Transport\Smtp\SmtpTransport->send(Object(Symfony\Component\Mime\Email), Null) symfony#5 (our queue dispatcher): Symfony\Component\Mailer\Mailer->send(Object(Symfony\Component\Mime\Email)) ``` App is running on PHP 8.0.28 on Debian Linux x64, Mailer v5.4.22. I would gladly written some tests for this, but I don't know how to simulate calls to low-level stream functions like fgets. Commits ------- 44d5b57 [Mailer] Throw TransportException when unable to read from socket
diff --git a/src/Symfony/Component/Mailer/Transport/Smtp/Stream/AbstractStream.php b/src/Symfony/Component/Mailer/Transport/Smtp/Stream/AbstractStream.php
@@ -74,7 +74,7 @@ public function readLine(): string
             return '';
         }
 
-        $line = fgets($this->out);
+        $line = @fgets($this->out);
         if ('' === $line || false === $line) {
             $metas = stream_get_meta_data($this->out);
             if ($metas['timed_out']) {
@@ -83,6 +83,9 @@ public function readLine(): string
             if ($metas['eof']) {
                 throw new TransportException(sprintf('Connection to "%s" has been closed unexpectedly.', $this->getReadConnectionDescription()));
             }
+            if (false === $line) {
+                throw new TransportException(sprintf('Unable to read from connection to "%s": ', $this->getReadConnectionDescription()).error_get_last()['message']);
+            }
         }
 
         $this->debug .= sprintf('< %s', $line);

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ public function readLine(): string`
`74`	`74`	`return '';`
`75`	`75`	`}`
`76`	`76`
`77`		`- $line = fgets($this->out);`
	`77`	`+ $line = @fgets($this->out);`
`78`	`78`	`if ('' === $line \|\| false === $line) {`
`79`	`79`	`$metas = stream_get_meta_data($this->out);`
`80`	`80`	`if ($metas['timed_out']) {`
`@@ -83,6 +83,9 @@ public function readLine(): string`
`83`	`83`	`if ($metas['eof']) {`
`84`	`84`	`throw new TransportException(sprintf('Connection to "%s" has been closed unexpectedly.', $this->getReadConnectionDescription()));`
`85`	`85`	`}`
	`86`	`+ if (false === $line) {`
	`87`	`+ throw new TransportException(sprintf('Unable to read from connection to "%s": ', $this->getReadConnectionDescription()).error_get_last()['message']);`
	`88`	`+ }`
`86`	`89`	`}`
`87`	`90`
`88`	`91`	`$this->debug .= sprintf('< %s', $line);`