diff --git a/lib/Cron/Daemon.php b/lib/Cron/Daemon.php index c51ede42bd..2e230bdc36 100644 --- a/lib/Cron/Daemon.php +++ b/lib/Cron/Daemon.php @@ -128,4 +128,4 @@ class Daemon { function terminateRequest($message = false) { die($message); } -} \ No newline at end of file +} diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index 03101f5362..2bab712340 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/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( - $send_result['operation'], - $send_result['error_message'] - ); + if(isset($send_result['retry_interval'])) { + MailerLog::processNonBlockingError($send_result['operation'], $send_result['error_message'], $send_result['retry_interval']); + } else { + MailerLog::processError($send_result['operation'], $send_result['error_message']); + } } // update processed/to process list if(!$queue->updateProcessedSubscribers($prepared_subscribers_ids)) { diff --git a/lib/Mailer/MailerLog.php b/lib/Mailer/MailerLog.php index 64b76a560f..ef400be7d2 100644 --- a/lib/Mailer/MailerLog.php +++ b/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; } -} \ No newline at end of file +} diff --git a/lib/Mailer/Methods/MailPoet.php b/lib/Mailer/Methods/MailPoet.php index e044d1236f..cd1f73cf93 100644 --- a/lib/Mailer/Methods/MailPoet.php +++ b/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) { - Bridge::invalidateKey(); - } - return Mailer::formatMailerSendErrorResult($result['message']); + return $this->processSendError($result, $subscriber); 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.*?)\s<(?P.*?)>!', $subscriber, $subscriber_data); if(!isset($subscriber_data['email'])) { @@ -104,4 +128,37 @@ class MailPoet { } return $body; } -} \ No newline at end of file + + 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; + } +} diff --git a/lib/Services/Bridge/API.php b/lib/Services/Bridge/API.php index cf097ae89a..e97352e295 100644 --- a/lib/Services/Bridge/API.php +++ b/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); } -} \ No newline at end of file +} diff --git a/tests/unit/Mailer/MailerLogTest.php b/tests/unit/Mailer/MailerLogTest.php index 256ddec0d3..3d67d0ffbb 100644 --- a/tests/unit/Mailer/MailerLogTest.php +++ b/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); } -} \ No newline at end of file +} diff --git a/tests/unit/Mailer/Methods/MailPoetAPITest.php b/tests/unit/Mailer/Methods/MailPoetAPITest.php index 0ad6976348..3ce64a90a9 100644 --- a/tests/unit/Mailer/Methods/MailPoetAPITest.php +++ b/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 ']); + 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 ']); + expect($result)->equals([ + 'response' => false, + 'operation' => 'send', + 'error_message' => 'Error while sending: (a@example.com: subject is missing), (c d : subject is missing)', + ]); + } + + }