Improve error reporting for MSS sending

[MAILPOET-1445]

lib/Cron/Daemon.php

@@ -123,4 +123,4 @@ class Daemon {
   function terminateRequest($message = false) {
     die($message);
   }
-}
+}

lib/Cron/Workers/SendingQueue/SendingQueue.php

@@ -164,10 +164,11 @@ class SendingQueue {
     );
     // log error message and schedule retry/pause sending
     if($send_result['response'] === false) {
-      MailerLog::processError(
+      if(isset($send_result['retry_interval'])) {
-        $send_result['operation'],
+        MailerLog::processNonBlockingError($send_result['operation'], $send_result['error_message'], $send_result['retry_interval']);
-        $send_result['error_message']
+      } else {
-      );
+        MailerLog::processError($send_result['operation'], $send_result['error_message']);
+      }
     }
     // update processed/to process list
     if(!$queue->updateProcessedSubscribers($prepared_subscribers_ids)) {

lib/Mailer/MailerLog.php

@@ -75,16 +75,43 @@ class MailerLog {
     return self::resetMailerLog();
   }
 
+  /**
+   * Process error, doesn't increase retry_attempt so it will not block sending
+   *
+   * @param string $operation
+   * @param string $error_message
+   * @param int $retry_interval
+   *
+   * @throws \Exception
+   */
+  static function processNonBlockingError($operation, $error_message, $retry_interval = self::RETRY_INTERVAL) {
+    $mailer_log = self::getMailerLog();
+    $mailer_log['retry_at'] = time() + $retry_interval;
+    $mailer_log = self::setError($mailer_log, $operation, $error_message);
+    self::updateMailerLog($mailer_log);
+    self::enforceExecutionRequirements();
+  }
+
+  /**
+   * Process error, increase retry_attempt and block sending if it goes above RETRY_INTERVAL
+   *
+   * @param string $operation
+   * @param string $error_message
+   * @param string $error_code
+   * @param bool $pause_sending
+   *
+   * @throws \Exception
+   */
   static function processError($operation, $error_message, $error_code = null, $pause_sending = false) {
     $mailer_log = self::getMailerLog();
-    (int)$mailer_log['retry_attempt']++;
+    $mailer_log['retry_attempt']++;
     $mailer_log['retry_at'] = time() + self::RETRY_INTERVAL;
     $mailer_log = self::setError($mailer_log, $operation, $error_message, $error_code);
     self::updateMailerLog($mailer_log);
     if($pause_sending) {
       self::pauseSending($mailer_log);
     }
-    return self::enforceExecutionRequirements();
+    self::enforceExecutionRequirements();
   }
 
   static function setError($mailer_log, $operation, $error_message, $error_code = null) {
@@ -140,4 +167,4 @@ class MailerLog {
     $mailer_log = self::getMailerLog($mailer_log);
     return $mailer_log['status'] === self::STATUS_PAUSED;
   }
-}
+}

lib/Mailer/Methods/MailPoet.php

@@ -9,6 +9,9 @@ use MailPoet\Services\Bridge\API;
 if(!defined('ABSPATH')) exit;
 
 class MailPoet {
+
+  const TEMPORARY_UNAVAILABLE_RETRY_INTERVAL = 300; // seconds
+
   public $api;
   public $sender;
   public $reply_to;
@@ -34,16 +37,37 @@ class MailPoet {
       case API::SENDING_STATUS_CONNECTION_ERROR:
         return Mailer::formatMailerConnectionErrorResult($result['message']);
       case API::SENDING_STATUS_SEND_ERROR:
-        if(!empty($result['code']) && $result['code'] === API::RESPONSE_CODE_KEY_INVALID) {
+        return $this->processSendError($result, $subscriber);
-          Bridge::invalidateKey();
-        }
-        return Mailer::formatMailerSendErrorResult($result['message']);
       case API::SENDING_STATUS_OK:
       default:
         return Mailer::formatMailerSendSuccessResult();
     }
   }
 
+  function processSendError($result, $subscriber) {
+    if(!empty($result['code'])) {
+      switch($result['code']) {
+        case API::RESPONSE_CODE_NOT_ARRAY:
+          return Mailer::formatMailerSendErrorResult(__('JSON input is not an array', 'mailpoet'));
+        case API::RESPONSE_CODE_PAYLOAD_TOO_BIG:
+          return Mailer::formatMailerSendErrorResult($result['message']);
+        case API::RESPONSE_CODE_PAYLOAD_ERROR:
+          $error = $this->parseErrorResponse($result['message'], $subscriber);
+          return Mailer::formatMailerSendErrorResult($error);
+        case API::RESPONSE_CODE_TEMPORARY_UNAVAILABLE:
+          $error = Mailer::formatMailerSendErrorResult(__('Email service is temporarily not available, please try again in a few minutes.', 'mailpoet'));
+          $error['retry_interval'] = self::TEMPORARY_UNAVAILABLE_RETRY_INTERVAL;
+          return $error;
+        case API::RESPONSE_CODE_KEY_INVALID:
+          Bridge::invalidateKey();
+          break;
+        default:
+          return Mailer::formatMailerSendErrorResult($result['message']);
+      }
+    }
+    return Mailer::formatMailerSendErrorResult($result['message']);
+  }
+
   function processSubscriber($subscriber) {
     preg_match('!(?P<name>.*?)\s<(?P<email>.*?)>!', $subscriber, $subscriber_data);
     if(!isset($subscriber_data['email'])) {
@@ -104,4 +128,37 @@ class MailPoet {
     }
     return $body;
   }
-}
+
+  private function parseErrorResponse($result, $subscriber) {
+    $result_parsed = json_decode($result, true);
+    $errors = [];
+    if(is_array($result_parsed)) {
+      foreach($result_parsed as $result_error) {
+        $errors[] = $this->processSingleSubscriberError($result_error, $subscriber);
+      }
+    }
+    if(!empty($errors)) {
+      return __('Error while sending: ', 'mailpoet') . join(', ', $errors);
+    } else {
+      return __('Error while sending newsletters. ', 'mailpoet') . $result;
+    }
+  }
+
+  private function processSingleSubscriberError($result_error, $subscriber) {
+    $error = '';
+    if(is_array($result_error)) {
+      $subscriber_errors = [];
+      if(isset($result_error['errors']) && is_array($result_error['errors'])) {
+        array_walk_recursive($result_error['errors'], function($item) use (&$subscriber_errors) {
+          $subscriber_errors[] = $item;
+        });
+      }
+      $error .= join(', ', $subscriber_errors);
+      
+      if(isset($result_error['index']) && isset($subscriber[$result_error['index']])) {
+        $error = '(' . $subscriber[$result_error['index']] . ': ' . $error . ')';
+      }
+    }
+    return $error;
+  }
+}

lib/Services/Bridge/API.php

@@ -16,6 +16,10 @@ class API {
 
   const RESPONSE_CODE_KEY_INVALID = 401;
   const RESPONSE_CODE_STATS_SAVED = 204;
+  const RESPONSE_CODE_TEMPORARY_UNAVAILABLE = 503;
+  const RESPONSE_CODE_NOT_ARRAY = 422;
+  const RESPONSE_CODE_PAYLOAD_TOO_BIG = 413;
+  const RESPONSE_CODE_PAYLOAD_ERROR = 400;
 
   private $api_key;
 
@@ -140,4 +144,4 @@ class API {
     );
     return WPFunctions::wpRemotePost($url, $params);
   }
-}
+}

tests/unit/Mailer/MailerLogTest.php

@@ -87,7 +87,7 @@ class MailerLogTest extends \MailPoetTest {
     expect(MailerLog::isSendingLimitReached())->true();
   }
 
-  function testitChecksWhenSendingIsPaused() {
+  function testItChecksWhenSendingIsPaused() {
     $mailer_log = array('status' => MailerLog::STATUS_PAUSED);
     expect(MailerLog::isSendingPaused($mailer_log))->true();
     $mailer_log = array('status' => false);
@@ -142,13 +142,14 @@ class MailerLogTest extends \MailPoetTest {
     expect($mailer_log['retry_at'])->null();
   }
 
-  function itProcessesSendingError() {
+  function testItProcessesSendingError() {
     // retry-related mailer values should be null
     $mailer_log = MailerLog::getMailerLog();
     expect($mailer_log['retry_attempt'])->null();
     expect($mailer_log['retry_at'])->null();
     expect($mailer_log['error'])->null();
     // retry attempt should be incremented, error logged, retry attempt scheduled
+    $this->setExpectedException('\Exception');
     MailerLog::processError($operation = 'send', $error = 'email rejected');
     $mailer_log = MailerLog::getMailerLog();
     expect($mailer_log['retry_attempt'])->equals(1);
@@ -161,6 +162,24 @@ class MailerLogTest extends \MailPoetTest {
     );
   }
 
+  function testItProcessesNonBlockingSendingError() {
+    $mailer_log = MailerLog::getMailerLog();
+    expect($mailer_log['retry_attempt'])->null();
+    expect($mailer_log['retry_at'])->null();
+    expect($mailer_log['error'])->null();
+    $this->setExpectedException('\Exception');
+    MailerLog::processNonBlockingError($operation = 'send', $error = 'email rejected');
+    $mailer_log = MailerLog::getMailerLog();
+    expect($mailer_log['retry_attempt'])->equals(1);
+    expect($mailer_log['retry_at'])->greaterThan(time());
+    expect($mailer_log['error'])->equals(
+      array(
+        'operation' => 'send',
+        'error_message' => $error
+      )
+    );
+  }
+
   function testItPausesSendingAfterProcessingSendingError() {
     $mailer_log = MailerLog::getMailerLog();
     expect($mailer_log['error'])->null();
@@ -303,4 +322,4 @@ class MailerLogTest extends \MailPoetTest {
   function _after() {
     \ORM::raw_execute('TRUNCATE ' . Setting::$_table);
   }
-}
+}

tests/unit/Mailer/Methods/MailPoetAPITest.php

@@ -4,6 +4,7 @@ namespace MailPoet\Test\Mailer\Methods;
 use Codeception\Util\Stub;
 use MailPoet\Config\ServicesChecker;
 use MailPoet\Mailer\Methods\MailPoet;
+use MailPoet\Services\Bridge\API;
 
 class MailPoetAPITest extends \MailPoetTest {
   function _before() {
@@ -149,4 +150,95 @@ class MailPoetAPITest extends \MailPoetTest {
     );
     expect($result['response'])->true();
   }
+
+  function testFormatConnectionError() {
+    $this->mailer->api = Stub::makeEmpty(
+      'MailPoet\Services\Bridge\API',
+      array('sendMessages' => [
+        'status' => API::SENDING_STATUS_CONNECTION_ERROR,
+        'message' => 'connection error',
+      ]),
+      $this
+    );
+    $result = $this->mailer->send($this->newsletter, $this->subscriber);
+    expect($result)->equals([
+      'response' => false,
+      'operation' => 'connect',
+      'error_message' => 'connection error',
+    ]);
+  }
+
+  function testFormatErrorNotArray() {
+    $this->mailer->api = Stub::makeEmpty(
+      'MailPoet\Services\Bridge\API',
+      array('sendMessages' => [
+        'code' => API::RESPONSE_CODE_NOT_ARRAY,
+        'status' => API::SENDING_STATUS_SEND_ERROR,
+        'message' => 'error not array',
+      ]),
+      $this
+    );
+    $result = $this->mailer->send($this->newsletter, $this->subscriber);
+    expect($result)->equals([
+      'response' => false,
+      'operation' => 'send',
+      'error_message' => 'JSON input is not an array',
+    ]);
+  }
+
+  function testFormatErrorTooBig() {
+    $this->mailer->api = Stub::makeEmpty(
+      'MailPoet\Services\Bridge\API',
+      array('sendMessages' => [
+        'code' => API::RESPONSE_CODE_PAYLOAD_TOO_BIG,
+        'status' => API::SENDING_STATUS_SEND_ERROR,
+        'message' => 'error too big',
+      ]),
+      $this
+    );
+    $result = $this->mailer->send($this->newsletter, $this->subscriber);
+    expect($result)->equals([
+      'response' => false,
+      'operation' => 'send',
+      'error_message' => 'error too big',
+    ]);
+  }
+
+  function testFormatPayloadError() {
+    $this->mailer->api = Stub::makeEmpty(
+      'MailPoet\Services\Bridge\API',
+      array('sendMessages' => [
+        'code' => API::RESPONSE_CODE_PAYLOAD_ERROR,
+        'status' => API::SENDING_STATUS_SEND_ERROR,
+        'message' => 'Api Error',
+      ]),
+      $this
+    );
+    $result = $this->mailer->send([$this->newsletter, $this->newsletter], ['a@example.com', 'c d <b@example.com>']);
+    expect($result)->equals([
+      'response' => false,
+      'operation' => 'send',
+      'error_message' => 'Error while sending newsletters. Api Error',
+    ]);
+  }
+
+  function testFormatPayloadErrorWithErrorMessage() {
+    $this->mailer->api = Stub::makeEmpty(
+      'MailPoet\Services\Bridge\API',
+      array('sendMessages' => [
+        'code' => API::RESPONSE_CODE_PAYLOAD_ERROR,
+        'status' => API::SENDING_STATUS_SEND_ERROR,
+        'message' => '[{"index":0,"errors":{"subject":"subject is missing"}},{"index":1,"errors":{"subject":"subject is missing"}}]'
+      ]),
+      $this
+    );
+    $result = $this->mailer->send([$this->newsletter, $this->newsletter], ['a@example.com', 'c d <b@example.com>']);
+    expect($result)->equals([
+      'response' => false,
+      'operation' => 'send',
+      'error_message' => 'Error while sending: (a@example.com: subject is missing), (c d <b@example.com>: subject is missing)',
+    ]);
+  }
+
+
 }