feature #32583 [Mailer] Logger vs debug mailer (fabpot)

This PR was squashed before being merged into the 4.4 branch (closes #32583).

Discussion
----------

[Mailer] Logger vs debug mailer

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Currently, there is no way to get the network data for the HTTP calls done by the HTTP transports (which makes debugging harder). For SMTP, we do have the network data, but as logs (each SMTP command/response is its own log line which means that the logs are "polluted" and the data is not tied with the sent message).

This pull request adds a `getDebug()` method on `SentMessage`. That allows to get the debug data conveniently in a standardized way (for both SMTP and HTTP transports). I have moved the SMTP logs to this new mechanism and added support for HTTP transports.

Commits
-------

fded3cd68c [Mailer] added support ffor debug info when using SMTP
d2f33d2cfe [Mailer] added debug info for HTTP mailers
This commit is contained in:
Fabien Potencier 2019-07-18 15:39:24 +02:00
commit 3849e1c238
14 changed files with 136 additions and 65 deletions

View File

@ -12,12 +12,13 @@
namespace Symfony\Component\Mailer\Bridge\Amazon\Http\Api;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SmtpEnvelope;
use Symfony\Component\Mailer\Transport\Http\Api\AbstractApiTransport;
use Symfony\Component\Mime\Email;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -42,7 +43,7 @@ class SesTransport extends AbstractApiTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSendEmail(Email $email, SmtpEnvelope $envelope): void
protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInterface
{
$date = gmdate('D, d M Y H:i:s e');
$auth = sprintf('AWS3-HTTPS AWSAccessKeyId=%s,Algorithm=HmacSHA256,Signature=%s', $this->accessKey, $this->getSignature($date));
@ -60,8 +61,10 @@ class SesTransport extends AbstractApiTransport
if (200 !== $response->getStatusCode()) {
$error = new \SimpleXMLElement($response->getContent(false));
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', $error->Error->Message, $error->Error->Code));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', $error->Error->Message, $error->Error->Code), $response);
}
return $response;
}
private function getSignature(string $string): string

View File

@ -12,11 +12,12 @@
namespace Symfony\Component\Mailer\Bridge\Amazon\Http;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SentMessage;
use Symfony\Component\Mailer\Transport\Http\AbstractHttpTransport;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -41,7 +42,7 @@ class SesTransport extends AbstractHttpTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSend(SentMessage $message): void
protected function doSendHttp(SentMessage $message): ResponseInterface
{
$date = gmdate('D, d M Y H:i:s e');
$auth = sprintf('AWS3-HTTPS AWSAccessKeyId=%s,Algorithm=HmacSHA256,Signature=%s', $this->accessKey, $this->getSignature($date));
@ -61,8 +62,10 @@ class SesTransport extends AbstractHttpTransport
if (200 !== $response->getStatusCode()) {
$error = new \SimpleXMLElement($response->getContent(false));
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', $error->Error->Message, $error->Error->Code));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', $error->Error->Message, $error->Error->Code), $response);
}
return $response;
}
private function getSignature(string $string): string

View File

@ -12,12 +12,13 @@
namespace Symfony\Component\Mailer\Bridge\Mailchimp\Http\Api;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SmtpEnvelope;
use Symfony\Component\Mailer\Transport\Http\Api\AbstractApiTransport;
use Symfony\Component\Mime\Email;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -35,7 +36,7 @@ class MandrillTransport extends AbstractApiTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSendEmail(Email $email, SmtpEnvelope $envelope): void
protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInterface
{
$response = $this->client->request('POST', self::ENDPOINT, [
'json' => $this->getPayload($email, $envelope),
@ -44,11 +45,13 @@ class MandrillTransport extends AbstractApiTransport
if (200 !== $response->getStatusCode()) {
$result = $response->toArray(false);
if ('error' === ($result['status'] ?? false)) {
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', $result['message'], $result['code']));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', $result['message'], $result['code']), $response);
}
throw new TransportException(sprintf('Unable to send an email (code %s).', $result['code']));
throw new HttpTransportException(sprintf('Unable to send an email (code %s).', $result['code']), $response);
}
return $response;
}
private function getPayload(Email $email, SmtpEnvelope $envelope): array

View File

@ -12,11 +12,12 @@
namespace Symfony\Component\Mailer\Bridge\Mailchimp\Http;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SentMessage;
use Symfony\Component\Mailer\Transport\Http\AbstractHttpTransport;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -33,7 +34,7 @@ class MandrillTransport extends AbstractHttpTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSend(SentMessage $message): void
protected function doSendHttp(SentMessage $message): ResponseInterface
{
$envelope = $message->getEnvelope();
$response = $this->client->request('POST', self::ENDPOINT, [
@ -48,10 +49,12 @@ class MandrillTransport extends AbstractHttpTransport
if (200 !== $response->getStatusCode()) {
$result = $response->toArray(false);
if ('error' === ($result['status'] ?? false)) {
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', $result['message'], $result['code']));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', $result['message'], $result['code']), $response);
}
throw new TransportException(sprintf('Unable to send an email (code %s).', $result['code']));
throw new HttpTransportException(sprintf('Unable to send an email (code %s).', $result['code']), $response);
}
return $response;
}
}

View File

@ -12,13 +12,14 @@
namespace Symfony\Component\Mailer\Bridge\Mailgun\Http\Api;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SmtpEnvelope;
use Symfony\Component\Mailer\Transport\Http\Api\AbstractApiTransport;
use Symfony\Component\Mime\Email;
use Symfony\Component\Mime\Part\Multipart\FormDataPart;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -40,7 +41,7 @@ class MailgunTransport extends AbstractApiTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSendEmail(Email $email, SmtpEnvelope $envelope): void
protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInterface
{
$body = new FormDataPart($this->getPayload($email, $envelope));
$headers = [];
@ -58,8 +59,10 @@ class MailgunTransport extends AbstractApiTransport
if (200 !== $response->getStatusCode()) {
$error = $response->toArray(false);
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', $error['message'], $response->getStatusCode()));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', $error['message'], $response->getStatusCode()), $response);
}
return $response;
}
private function getPayload(Email $email, SmtpEnvelope $envelope): array

View File

@ -12,13 +12,14 @@
namespace Symfony\Component\Mailer\Bridge\Mailgun\Http;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SentMessage;
use Symfony\Component\Mailer\Transport\Http\AbstractHttpTransport;
use Symfony\Component\Mime\Part\DataPart;
use Symfony\Component\Mime\Part\Multipart\FormDataPart;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -39,7 +40,7 @@ class MailgunTransport extends AbstractHttpTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSend(SentMessage $message): void
protected function doSendHttp(SentMessage $message): ResponseInterface
{
$body = new FormDataPart([
'to' => implode(',', $this->stringifyAddresses($message->getEnvelope()->getRecipients())),
@ -59,7 +60,9 @@ class MailgunTransport extends AbstractHttpTransport
if (200 !== $response->getStatusCode()) {
$error = $response->toArray(false);
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', $error['message'], $response->getStatusCode()));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', $error['message'], $response->getStatusCode()), $response);
}
return $response;
}
}

View File

@ -12,12 +12,13 @@
namespace Symfony\Component\Mailer\Bridge\Postmark\Http\Api;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SmtpEnvelope;
use Symfony\Component\Mailer\Transport\Http\Api\AbstractApiTransport;
use Symfony\Component\Mime\Email;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -35,7 +36,7 @@ class PostmarkTransport extends AbstractApiTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSendEmail(Email $email, SmtpEnvelope $envelope): void
protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInterface
{
$response = $this->client->request('POST', self::ENDPOINT, [
'headers' => [
@ -48,8 +49,10 @@ class PostmarkTransport extends AbstractApiTransport
if (200 !== $response->getStatusCode()) {
$error = $response->toArray(false);
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', $error['Message'], $error['ErrorCode']));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', $error['Message'], $error['ErrorCode']), $response);
}
return $response;
}
private function getPayload(Email $email, SmtpEnvelope $envelope): array

View File

@ -12,13 +12,14 @@
namespace Symfony\Component\Mailer\Bridge\Sendgrid\Http\Api;
use Psr\Log\LoggerInterface;
use Symfony\Component\Mailer\Exception\TransportException;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SmtpEnvelope;
use Symfony\Component\Mailer\Transport\Http\Api\AbstractApiTransport;
use Symfony\Component\Mime\Address;
use Symfony\Component\Mime\Email;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Kevin Verschaeve
@ -36,7 +37,7 @@ class SendgridTransport extends AbstractApiTransport
parent::__construct($client, $dispatcher, $logger);
}
protected function doSendEmail(Email $email, SmtpEnvelope $envelope): void
protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInterface
{
$response = $this->client->request('POST', self::ENDPOINT, [
'json' => $this->getPayload($email, $envelope),
@ -46,8 +47,10 @@ class SendgridTransport extends AbstractApiTransport
if (202 !== $response->getStatusCode()) {
$errors = $response->toArray(false);
throw new TransportException(sprintf('Unable to send an email: %s (code %s).', implode('; ', array_column($errors['errors'], 'message')), $response->getStatusCode()));
throw new HttpTransportException(sprintf('Unable to send an email: %s (code %s).', implode('; ', array_column($errors['errors'], 'message')), $response->getStatusCode()), $response);
}
return $response;
}
private function getPayload(Email $email, SmtpEnvelope $envelope): array

View File

@ -11,9 +11,24 @@
namespace Symfony\Component\Mailer\Exception;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Fabien Potencier <fabien@symfony.com>
*/
class HttpTransportException extends TransportException
{
private $response;
public function __construct(string $message = null, ResponseInterface $response, int $code = 0, \Exception $previous = null)
{
parent::__construct($message, $code, $previous);
$this->response = $response;
}
public function getResponse(): ResponseInterface
{
return $this->response;
}
}

View File

@ -22,6 +22,7 @@ class SentMessage
private $original;
private $raw;
private $envelope;
private $debug = '';
/**
* @internal
@ -48,6 +49,16 @@ class SentMessage
return $this->envelope;
}
public function getDebug(): string
{
return $this->debug;
}
public function appendDebug(string $debug): void
{
$this->debug .= $debug;
}
public function toString(): string
{
return $this->raw->toString();

View File

@ -13,9 +13,12 @@ namespace Symfony\Component\Mailer\Transport\Http;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\Mailer\Exception\HttpTransportException;
use Symfony\Component\Mailer\SentMessage;
use Symfony\Component\Mailer\Transport\AbstractTransport;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Victor Bocharsky <victor@symfonycasts.com>
@ -37,4 +40,22 @@ abstract class AbstractHttpTransport extends AbstractTransport
parent::__construct($dispatcher, $logger);
}
abstract protected function doSendHttp(SentMessage $message): ResponseInterface;
protected function doSend(SentMessage $message): void
{
$response = null;
try {
$response = $this->doSendHttp($message);
} catch (HttpTransportException $e) {
$response = $e->getResponse();
throw $e;
} finally {
if (null !== $response) {
$message->appendDebug($response->getInfo('debug'));
}
}
}
}

View File

@ -11,42 +11,23 @@
namespace Symfony\Component\Mailer\Transport\Http\Api;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\Mailer\Exception\RuntimeException;
use Symfony\Component\Mailer\SentMessage;
use Symfony\Component\Mailer\SmtpEnvelope;
use Symfony\Component\Mailer\Transport\AbstractTransport;
use Symfony\Component\Mailer\Transport\Http\AbstractHttpTransport;
use Symfony\Component\Mime\Address;
use Symfony\Component\Mime\Email;
use Symfony\Component\Mime\MessageConverter;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
/**
* @author Fabien Potencier <fabien@symfony.com>
*/
abstract class AbstractApiTransport extends AbstractTransport
abstract class AbstractApiTransport extends AbstractHttpTransport
{
protected $client;
abstract protected function doSendApi(Email $email, SmtpEnvelope $envelope): ResponseInterface;
public function __construct(HttpClientInterface $client = null, EventDispatcherInterface $dispatcher = null, LoggerInterface $logger = null)
{
$this->client = $client;
if (null === $client) {
if (!class_exists(HttpClient::class)) {
throw new \LogicException(sprintf('You cannot use "%s" as the HttpClient component is not installed. Try running "composer require symfony/http-client".', __CLASS__));
}
$this->client = HttpClient::create();
}
parent::__construct($dispatcher, $logger);
}
abstract protected function doSendEmail(Email $email, SmtpEnvelope $envelope): void;
protected function doSend(SentMessage $message): void
protected function doSendHttp(SentMessage $message): ResponseInterface
{
try {
$email = MessageConverter::toEmail($message->getOriginalMessage());
@ -54,7 +35,7 @@ abstract class AbstractApiTransport extends AbstractTransport
throw new RuntimeException(sprintf('Unable to send message with the "%s" transport: %s', __CLASS__, $e->getMessage()), 0, $e);
}
$this->doSendEmail($email, $message->getEnvelope());
return $this->doSendApi($email, $message->getEnvelope());
}
protected function getRecipients(Email $email, SmtpEnvelope $envelope): array

View File

@ -135,7 +135,6 @@ class SmtpTransport extends AbstractTransport
*/
public function executeCommand(string $command, array $codes): string
{
$this->getLogger()->debug(sprintf('Email transport "%s" sent command "%s"', __CLASS__, trim($command)));
$this->stream->write($command);
$response = $this->getFullResponse();
$this->assertResponseCode($response, $codes);
@ -145,18 +144,22 @@ class SmtpTransport extends AbstractTransport
protected function doSend(SentMessage $message): void
{
$envelope = $message->getEnvelope();
$this->doMailFromCommand($envelope->getSender()->toString());
foreach ($envelope->getRecipients() as $recipient) {
$this->doRcptToCommand($recipient->toString());
}
try {
$envelope = $message->getEnvelope();
$this->doMailFromCommand($envelope->getSender()->toString());
foreach ($envelope->getRecipients() as $recipient) {
$this->doRcptToCommand($recipient->toString());
}
$this->executeCommand("DATA\r\n", [354]);
foreach (AbstractStream::replace("\r\n.", "\r\n..", $message->toIterable()) as $chunk) {
$this->stream->write($chunk);
$this->executeCommand("DATA\r\n", [354]);
foreach (AbstractStream::replace("\r\n.", "\r\n..", $message->toIterable()) as $chunk) {
$this->stream->write($chunk, false);
}
$this->stream->flush();
$this->executeCommand("\r\n.\r\n", [250]);
} finally {
$message->appendDebug($this->stream->getDebug());
}
$this->stream->flush();
$this->executeCommand("\r\n.\r\n", [250]);
}
protected function doHeloCommand(): void
@ -237,8 +240,6 @@ class SmtpTransport extends AbstractTransport
list($code) = sscanf($response, '%3d');
$valid = \in_array($code, $codes);
$this->getLogger()->debug(sprintf('Email transport "%s" received response "%s" (%s).', __CLASS__, trim($response), $valid ? 'ok' : 'error'));
if (!$valid) {
throw new TransportException(sprintf('Expected response code "%s" but got code "%s", with message "%s".', implode('/', $codes), $code, trim($response)), $code);
}

View File

@ -28,8 +28,16 @@ abstract class AbstractStream
protected $in;
protected $out;
public function write(string $bytes): void
private $debug = '';
public function write(string $bytes, $debug = true): void
{
if ($debug) {
foreach (explode("\n", trim($bytes)) as $line) {
$this->debug .= sprintf("> %s\n", $line);
}
}
$bytesToWrite = \strlen($bytes);
$totalBytesWritten = 0;
while ($totalBytesWritten < $bytesToWrite) {
@ -74,9 +82,19 @@ abstract class AbstractStream
}
}
$this->debug .= sprintf('< %s', $line);
return $line;
}
public function getDebug(): string
{
$debug = $this->debug;
$this->debug = '';
return $debug;
}
public static function replace(string $from, string $to, iterable $chunks): \Generator
{
if ('' === $from) {