Merge pull request #2956 from alphagov/ses-throttle

Log warning for SES send rate throttling rather than exception

app/celery/provider_tasks.py

@@ -5,6 +5,7 @@ from sqlalchemy.orm.exc import NoResultFound
 
 from app import notify_celery
 from app.config import QueueNames
+from app.clients.email.aws_ses import AwsSesClientThrottlingSendRateException
 from app.dao import notifications_dao
 from app.dao.notifications_dao import update_notification_status_by_id
 from app.delivery import send_to_providers
@@ -49,11 +50,17 @@ def deliver_email(self, notification_id):
     except InvalidEmailError as e:
         current_app.logger.exception(e)
         update_notification_status_by_id(notification_id, 'technical-failure')
-    except Exception:
+    except Exception as e:
         try:
-            current_app.logger.exception(
-                "RETRY: Email notification {} failed".format(notification_id)
-            )
+            if isinstance(e, AwsSesClientThrottlingSendRateException):
+                current_app.logger.warning(
+                    f"RETRY: Email notification {notification_id} was rate limited by SES"
+                )
+            else:
+                current_app.logger.exception(
+                    f"RETRY: Email notification {notification_id} failed"
+                )
+
             self.retry(queue=QueueNames.RETRY)
         except self.MaxRetriesExceededError:
             message = "RETRY FAILED: Max retries reached. " \

app/clients/email/aws_ses.py

@@ -43,6 +43,10 @@ class AwsSesClientException(EmailClientException):
     pass
 
 
+class AwsSesClientThrottlingSendRateException(AwsSesClientException):
+    pass
+
+
 class AwsSesClient(EmailClient):
     '''
     Amazon SES email client.
@@ -122,6 +126,11 @@ class AwsSesClient(EmailClient):
                     to_addresses[0],
                     e.response['Error']['Message']
                 ))
+            elif (
+                e.response['Error']['Code'] == 'Throttling'
+                and e.response['Error']['Message'] == 'Maximum sending rate exceeded.'
+            ):
+                raise AwsSesClientThrottlingSendRateException(str(e))
             else:
                 self.statsd_client.incr("clients.ses.error")
                 raise AwsSesClientException(str(e))

tests/app/celery/test_provider_tasks.py

@@ -6,7 +6,7 @@ from notifications_utils.recipients import InvalidEmailError
 import app
 from app.celery import provider_tasks
 from app.celery.provider_tasks import deliver_sms, deliver_email
-from app.clients.email.aws_ses import AwsSesClientException
+from app.clients.email.aws_ses import AwsSesClientException, AwsSesClientThrottlingSendRateException
 from app.exceptions import NotificationTechnicalFailureException
 
 
@@ -72,8 +72,17 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task(s
     assert sample_notification.status == 'technical-failure'
 
 
-def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task(sample_notification, mocker):
-    mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=Exception("EXPECTED"))
+@pytest.mark.parametrize(
+    'exception_class', [
+        Exception(),
+        AwsSesClientException(),
+        AwsSesClientThrottlingSendRateException(),
+    ]
+)
+def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task(
+    sample_notification, mocker, exception_class
+):
+    mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=exception_class)
     mocker.patch('app.celery.provider_tasks.deliver_email.retry', side_effect=MaxRetriesExceededError())
 
     with pytest.raises(NotificationTechnicalFailureException) as e:
@@ -105,11 +114,38 @@ def test_should_retry_and_log_exception(sample_notification, mocker):
     ex = ClientError(error_response=error_response, operation_name='opname')
     mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=AwsSesClientException(str(ex)))
     mocker.patch('app.celery.provider_tasks.deliver_email.retry')
+    mock_logger_exception = mocker.patch('app.celery.tasks.current_app.logger.exception')
 
     deliver_email(sample_notification.id)
 
     assert provider_tasks.deliver_email.retry.called is True
     assert sample_notification.status == 'created'
+    assert mock_logger_exception.called
+
+
+def test_if_ses_send_rate_throttle_then_should_retry_and_log_warning(sample_notification, mocker):
+    error_response = {
+        'Error': {
+            'Code': 'Throttling',
+            'Message': 'Maximum sending rate exceeded.',
+            'Type': 'Sender'
+        }
+    }
+    ex = ClientError(error_response=error_response, operation_name='opname')
+    mocker.patch(
+        'app.delivery.send_to_providers.send_email_to_provider',
+        side_effect=AwsSesClientThrottlingSendRateException(str(ex))
+    )
+    mocker.patch('app.celery.provider_tasks.deliver_email.retry')
+    mock_logger_warning = mocker.patch('app.celery.tasks.current_app.logger.warning')
+    mock_logger_exception = mocker.patch('app.celery.tasks.current_app.logger.exception')
+
+    deliver_email(sample_notification.id)
+
+    assert provider_tasks.deliver_email.retry.called is True
+    assert sample_notification.status == 'created'
+    assert not mock_logger_exception.called
+    assert mock_logger_warning.called
 
 
 def test_send_sms_should_not_switch_providers_on_non_provider_failure(

tests/app/clients/test_aws_ses.py

@@ -4,7 +4,7 @@ from unittest.mock import Mock, ANY
 from notifications_utils.recipients import InvalidEmailError
 
 from app import aws_ses_client
-from app.clients.email.aws_ses import get_aws_responses, AwsSesClientException
+from app.clients.email.aws_ses import get_aws_responses, AwsSesClientException, AwsSesClientThrottlingSendRateException
 
 
 def test_should_return_correct_details_for_delivery():
@@ -116,6 +116,48 @@ def test_send_email_raises_bad_email_as_InvalidEmailError(mocker):
     assert 'definitely@invalid_email.com' in str(excinfo.value)
 
 
+def test_send_email_raises_send_rate_throttling_as_AwsSesClientThrottlingSendRateException(mocker):
+    boto_mock = mocker.patch.object(aws_ses_client, '_client', create=True)
+    mocker.patch.object(aws_ses_client, 'statsd_client', create=True)
+    error_response = {
+        'Error': {
+            'Code': 'Throttling',
+            'Message': 'Maximum sending rate exceeded.',
+            'Type': 'Sender'
+        }
+    }
+    boto_mock.send_email.side_effect = botocore.exceptions.ClientError(error_response, 'opname')
+
+    with pytest.raises(AwsSesClientThrottlingSendRateException):
+        aws_ses_client.send_email(
+            source=Mock(),
+            to_addresses='foo@bar.com',
+            subject=Mock(),
+            body=Mock()
+        )
+
+
+def test_send_email_does_not_raise_AwsSesClientThrottlingSendRateException_if_non_send_rate_throttling(mocker):
+    boto_mock = mocker.patch.object(aws_ses_client, '_client', create=True)
+    mocker.patch.object(aws_ses_client, 'statsd_client', create=True)
+    error_response = {
+        'Error': {
+            'Code': 'Throttling',
+            'Message': 'Daily message quota exceeded',
+            'Type': 'Sender'
+        }
+    }
+    boto_mock.send_email.side_effect = botocore.exceptions.ClientError(error_response, 'opname')
+
+    with pytest.raises(AwsSesClientException):
+        aws_ses_client.send_email(
+            source=Mock(),
+            to_addresses='foo@bar.com',
+            subject=Mock(),
+            body=Mock()
+        )
+
+
 def test_send_email_raises_other_errs_as_AwsSesClientException(mocker):
     boto_mock = mocker.patch.object(aws_ses_client, '_client', create=True)
     mocker.patch.object(aws_ses_client, 'statsd_client', create=True)