From c9519f0b3da09f27f180bd688c909f6ac31af7a4 Mon Sep 17 00:00:00 2001 From: Vlad Date: Thu, 16 Feb 2017 22:44:03 -0500 Subject: [PATCH] Adds logger to record SMTP communication Returns a more complete error message and last unprocessed subscriber --- lib/Mailer/Methods/SMTP.php | 48 ++++++++++++++++++++------ tests/unit/Mailer/Methods/SMTPTest.php | 31 +++++++++++++++++ 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/lib/Mailer/Methods/SMTP.php b/lib/Mailer/Methods/SMTP.php index 8667035fc5..795175e696 100644 --- a/lib/Mailer/Methods/SMTP.php +++ b/lib/Mailer/Methods/SMTP.php @@ -16,6 +16,7 @@ class SMTP { public $reply_to; public $return_path; public $mailer; + const SMTP_CONNECTION_TIMEOUT = 10; // seconds function __construct( $host, $port, $authentication, $login = null, $password = null, $encryption, @@ -32,6 +33,8 @@ class SMTP { $return_path : $this->sender['from_email']; $this->mailer = $this->buildMailer(); + $this->mailer_logger = new \Swift_Plugins_Loggers_ArrayLogger(); + $this->mailer->registerPlugin(new \Swift_Plugins_LoggerPlugin($this->mailer_logger)); } function send($newsletter, $subscriber, $extra_params = array()) { @@ -39,19 +42,19 @@ class SMTP { $message = $this->createMessage($newsletter, $subscriber, $extra_params); $result = $this->mailer->send($message); } catch(\Exception $e) { - return Mailer::formatMailerSendErrorResult($e->getMessage()); + return Mailer::formatMailerSendErrorResult( + $this->processExceptionMessage($e->getMessage()) + ); } return ($result === 1) ? Mailer::formatMailerSendSuccessResult() : - Mailer::formatMailerSendErrorResult( - sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_SMTP) - ); + Mailer::formatMailerSendErrorResult($this->processLogMessage($subscriber)); } function buildMailer() { $transport = \Swift_SmtpTransport::newInstance( $this->host, $this->port, $this->encryption); - $transport->setTimeout(10); + $transport->setTimeout(self::SMTP_CONNECTION_TIMEOUT); if($this->authentication) { $transport ->setUsername($this->login) @@ -63,13 +66,17 @@ class SMTP { function createMessage($newsletter, $subscriber, $extra_params = array()) { $message = \Swift_Message::newInstance() ->setTo($this->processSubscriber($subscriber)) - ->setFrom(array( + ->setFrom( + array( $this->sender['from_email'] => $this->sender['from_name'] - )) + ) + ) ->setSender($this->sender['from_email']) - ->setReplyTo(array( - $this->reply_to['reply_to_email'] => $this->reply_to['reply_to_name'] - )) + ->setReplyTo( + array( + $this->reply_to['reply_to_email'] => $this->reply_to['reply_to_name'] + ) + ) ->setReturnPath($this->return_path) ->setSubject($newsletter['subject']); if(!empty($extra_params['unsubscribe_url'])) { @@ -97,4 +104,25 @@ class SMTP { (isset($subscriber_data['name'])) ? $subscriber_data['name'] : '' ); } + + function processLogMessage($subscriber, $log = false) { + $log = ($log) ? $log : $this->mailer_logger->dump(); + // extract error message from log + preg_match('/!! (.*?)>>/ism', $log, $message); + if(!empty($message[1])) { + $message = $message[1]; + // remove line breaks from the message due to how logger's dump() method works + $message = preg_replace('/\r|\n/', '', $message); + } else { + $message = sprintf(__('%s has returned an unknown error.', 'mailpoet'), Mailer::METHOD_SMTP); + } + $message .= sprintf(' %s: %s', __('Unprocessed subscriber', 'mailpoet'), $subscriber); + return $message; + } + + function processExceptionMessage($message) { + // remove redundant information appended by Swift logger to exception messages + $message = explode(PHP_EOL, $message); + return $message[0]; + } } \ No newline at end of file diff --git a/tests/unit/Mailer/Methods/SMTPTest.php b/tests/unit/Mailer/Methods/SMTPTest.php index 77243e378b..b1121698b8 100644 --- a/tests/unit/Mailer/Methods/SMTPTest.php +++ b/tests/unit/Mailer/Methods/SMTPTest.php @@ -1,5 +1,6 @@ false(); } + function testItCanProcessExceptionMessage() { + $message = 'Connection could not be established with host localhost [Connection refused #111]' . PHP_EOL + . 'Log data:' . PHP_EOL + . '++ Starting Swift_SmtpTransport' . PHP_EOL + . '!! Connection could not be established with host localhost [Connection refused #111] (code: 0)'; + expect($this->mailer->processExceptionMessage($message)) + ->equals('Connection could not be established with host localhost [Connection refused #111]'); + } + + function testItCanProcessLogMessageWhenOneExists() { + $message = '++ Swift_SmtpTransport started' . PHP_EOL + . '>> MAIL FROM:' . PHP_EOL + . '<< 250 OK' . PHP_EOL + . '>> RCPT TO:' . PHP_EOL + . '<< 550 No such recipient here' . PHP_EOL + . '!! Expected response code 250/251/252 but got code "550", with message "550 No such recipient here' . PHP_EOL + . '" (code: 550)' . PHP_EOL + . '>> RSET' . PHP_EOL + . '<< 250 Reset OK' . PHP_EOL; + expect($this->mailer->processLogMessage('test@example.com', $message)) + ->equals('Expected response code 250/251/252 but got code "550", with message "550 No such recipient here" (code: 550) Unprocessed subscriber: test@example.com'); + expect($this->mailer->processLogMessage('test@example.com', $message)) + ->equals('Expected response code 250/251/252 but got code "550", with message "550 No such recipient here" (code: 550) Unprocessed subscriber: test@example.com'); + } + + function testItReturnsGenericMessageWhenLogMessageDoesNotExist() { + expect($this->mailer->processLogMessage('test@example.com')) + ->equals(Mailer::METHOD_SMTP . ' has returned an unknown error. Unprocessed subscriber: test@example.com'); + } + function testItCanSend() { if(getenv('WP_TEST_MAILER_ENABLE_SENDING') !== 'true') return; $result = $this->mailer->send(