diff --git a/lib/Config/Migrator.php b/lib/Config/Migrator.php index e9ead7e49d..09db0bc205 100644 --- a/lib/Config/Migrator.php +++ b/lib/Config/Migrator.php @@ -131,6 +131,8 @@ class Migrator { 'task_id int(11) unsigned NOT NULL,', 'subscriber_id int(11) unsigned NOT NULL,', 'processed int(1) NOT NULL,', + 'failed int(1) NOT NULL,', + 'error text NULL,', 'created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,', 'PRIMARY KEY (task_id, subscriber_id),', 'KEY subscriber_id (subscriber_id)' diff --git a/lib/Cron/Workers/SendingQueue/SendingErrorHandler.php b/lib/Cron/Workers/SendingQueue/SendingErrorHandler.php index cdb72dd339..0470369cc3 100644 --- a/lib/Cron/Workers/SendingQueue/SendingErrorHandler.php +++ b/lib/Cron/Workers/SendingQueue/SendingErrorHandler.php @@ -3,13 +3,34 @@ namespace MailPoet\Cron\Workers\SendingQueue; use MailPoet\Mailer\MailerError; use MailPoet\Mailer\MailerLog; +use MailPoet\Tasks\Sending as SendingTask; class SendingErrorHandler { - function processError(MailerError $error) { + function processError( + MailerError $error, + SendingTask $sending_task, + array $prepared_subscribers_ids, + array $prepared_subscribers + ) { + if($error->getLevel() === MailerError::LEVEL_HARD) { + return $this->processHardError($error); + } + $this->processSoftError($error, $sending_task, $prepared_subscribers_ids, $prepared_subscribers); + } + + private function processHardError(MailerError $error) { if($error->getRetryInterval() !== null) { MailerLog::processNonBlockingError($error->getOperation(), $error->getMessageWithFailedSubscribers(), $error->getRetryInterval()); } else { MailerLog::processError($error->getOperation(), $error->getMessageWithFailedSubscribers()); } } + + private function processSoftError(MailerError $error, SendingTask $sending_task, $prepared_subscribers_ids, $prepared_subscribers) { + foreach($error->getSubscriberErrors() as $subscriber_error) { + $subscriber_id_index = array_search($subscriber_error->getEmail(), $prepared_subscribers); + $message = $subscriber_error->getMessage() ?: $error->getMessage(); + $sending_task->saveSubscriberError($prepared_subscribers_ids[$subscriber_id_index], $message); + } + } } diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index a9f16274f3..2cbc8ef55b 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -167,7 +167,13 @@ class SendingQueue { $prepared_subscriber, $extra_params ); - return $this->processSendResult($sending_task, $send_result, [$prepared_subscriber_id], [$statistics]); + return $this->processSendResult( + $sending_task, + $send_result, + [$prepared_subscriber], + [$prepared_subscriber_id], + [$statistics] + ); } function sendNewsletters( @@ -180,19 +186,27 @@ class SendingQueue { $prepared_subscribers, $extra_params ); - return $this->processSendResult($sending_task, $send_result, $prepared_subscribers_ids, $statistics); + return $this->processSendResult( + $sending_task, + $send_result, + $prepared_subscribers, + $prepared_subscribers_ids, + $statistics + ); } private function processSendResult( SendingTask $sending_task, $send_result, - array $prepared_subscribers_ids, array $statistics + array $prepared_subscribers, + array $prepared_subscribers_ids, + array $statistics ) { // log error message and schedule retry/pause sending if($send_result['response'] === false) { $error = $send_result['error']; assert($error instanceof MailerError); - $this->error_handler->processError($error); + $this->error_handler->processError($error, $sending_task, $prepared_subscribers_ids, $prepared_subscribers); } // update processed/to process list if(!$sending_task->updateProcessedSubscribers($prepared_subscribers_ids)) { diff --git a/lib/Mailer/MailerError.php b/lib/Mailer/MailerError.php index 37eabd1170..46ad67504b 100644 --- a/lib/Mailer/MailerError.php +++ b/lib/Mailer/MailerError.php @@ -67,7 +67,7 @@ class MailerError { } /** - * @return array + * @return SubscriberError[] */ function getSubscriberErrors() { return $this->subscribers_errors; diff --git a/lib/Models/ScheduledTaskSubscriber.php b/lib/Models/ScheduledTaskSubscriber.php index 606852616c..a3db1ba7f2 100644 --- a/lib/Models/ScheduledTaskSubscriber.php +++ b/lib/Models/ScheduledTaskSubscriber.php @@ -7,6 +7,9 @@ class ScheduledTaskSubscriber extends Model { const STATUS_UNPROCESSED = 0; const STATUS_PROCESSED = 1; + const FAIL_STATUS_OK = 0; + const FAIL_STATUS_FAILED = 1; + public static $_table = MP_SCHEDULED_TASK_SUBSCRIBERS_TABLE; public static $_id_column = array('task_id', 'subscriber_id'); @@ -19,6 +22,7 @@ class ScheduledTaskSubscriber extends Model { return; } $data['processed'] = !empty($data['processed']) ? self::STATUS_PROCESSED : self::STATUS_UNPROCESSED; + $data['failed'] = !empty($data['failed']) ? self::FAIL_STATUS_FAILED : self::FAIL_STATUS_OK; return parent::_createOrUpdate($data, array( 'subscriber_id' => $data['subscriber_id'], 'task_id' => $data['task_id'] diff --git a/lib/Tasks/Sending.php b/lib/Tasks/Sending.php index 7f533b1577..995339ad6d 100644 --- a/lib/Tasks/Sending.php +++ b/lib/Tasks/Sending.php @@ -166,6 +166,11 @@ class Sending { return $this->updateCount()->getErrors() === false; } + public function saveSubscriberError($subcriber_id, $error_message) { + $this->task_subscribers->saveSubscriberError($subcriber_id, $error_message); + return $this->updateCount()->getErrors() === false; + } + function updateCount() { $this->queue->count_processed = ScheduledTaskSubscriber::getProcessedCount($this->task->id); $this->queue->count_to_process = ScheduledTaskSubscriber::getUnprocessedCount($this->task->id); diff --git a/lib/Tasks/Subscribers.php b/lib/Tasks/Subscribers.php index 33b9df77e9..27cc45f134 100644 --- a/lib/Tasks/Subscribers.php +++ b/lib/Tasks/Subscribers.php @@ -53,6 +53,15 @@ class Subscribers { $this->checkCompleted(); } + function saveSubscriberError($subcriber_id, $error_message) { + $this->getSubscribers() + ->where('subscriber_id', $subcriber_id) + ->findResultSet() + ->set('failed', ScheduledTaskSubscriber::FAIL_STATUS_FAILED) + ->set('error', $error_message) + ->save(); + } + private function checkCompleted($count = null) { if(!$count && !ScheduledTaskSubscriber::getUnprocessedCount($this->task->id)) { $this->task->complete(); diff --git a/tests/unit/Cron/Workers/SendingQueue/SendingErrorHandlerTest.php b/tests/unit/Cron/Workers/SendingQueue/SendingErrorHandlerTest.php new file mode 100644 index 0000000000..08817e3801 --- /dev/null +++ b/tests/unit/Cron/Workers/SendingQueue/SendingErrorHandlerTest.php @@ -0,0 +1,56 @@ +error_handler = new SendingErrorHandler(); + } + + function testItShouldProcessSoftErrorCorrectly() { + $subscribers = [ + 'john@doe.com', + 'john@rambo.com', + ]; + $subscriber_ids = [1, 2]; + $subscriber_errors = [ + new SubscriberError('john@doe.com', 'Subscriber Message'), + new SubscriberError('john@rambo.com', null), + ]; + $error = new MailerError( + MailerError::OPERATION_SEND, + MailerError::LEVEL_SOFT, + 'Error Message', + null, $subscriber_errors + ); + + $sending_task = Stub::make( + SendingTask::class, + [ + 'saveSubscriberError' => Expected::exactly( + 2, + function($id, $message) { + if($id === 2) { + expect($message)->equals('Error Message'); + } else { + expect($message)->equals('Subscriber Message'); + } + } + ), + ], + $this + ); + + $this->error_handler->processError($error, $sending_task, $subscriber_ids, $subscribers); + } +} diff --git a/tests/unit/Cron/Workers/SendingQueue/SendingQueueTest.php b/tests/unit/Cron/Workers/SendingQueue/SendingQueueTest.php index 2b7cc400bb..80cc27d515 100644 --- a/tests/unit/Cron/Workers/SendingQueue/SendingQueueTest.php +++ b/tests/unit/Cron/Workers/SendingQueue/SendingQueueTest.php @@ -147,8 +147,8 @@ class SendingQueueTest extends \MailPoetTest { $sending_queue_worker->sendNewsletters( $this->queue, $prepared_subscribers = array(), - $prepared_newsletters = false, - $prepared_subscribers = false, + $prepared_newsletters = [], + $prepared_subscribers = [], $statistics[] = array( 'newsletter_id' => 1, 'subscriber_id' => 1, @@ -608,7 +608,7 @@ class SendingQueueTest extends \MailPoetTest { $prepared_subscribers = [], $prepared_newsletters = [], $prepared_subscribers = [], - $statistics = false + $statistics = [] ); $this->fail('Paused sending exception was not thrown.'); } catch(\Exception $e) {