Adds logger to record SMTP communication

Returns a more complete error message and last unprocessed subscriber

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];
+  }
 }

tests/unit/Mailer/Methods/SMTPTest.php

@@ -1,5 +1,6 @@
 <?php
 
+use MailPoet\Mailer\Mailer;
 use MailPoet\Mailer\Methods\SMTP;
 
 class SMTPTest extends MailPoetTest {
@@ -124,6 +125,36 @@ class SMTPTest extends MailPoetTest {
     expect($result['response'])->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:<moi@mrcasual.com>' . PHP_EOL
+      . '<< 250 OK' . PHP_EOL
+      . '>> RCPT TO:<test2@ietsdoenofferte.nl>' . 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(