mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-01 15:46:07 -05:00
Raise better exception on InvalidParameterValue error
There are several reasons why we might get an `InvalidParameterValue` from the SES API. One, as correctly identified before in https://github.com/alphagov/notifications-api/pull/713/files is if we allow an email address on our side that SES rejects. However, there are other types of errors that could cause an `InvalidParameterValue`. One example is a `Header too long: 'Subject'` error that we have seen happen in production. This shouldn't raise an `InvalidEmailError` as that is not appropriate. Therefore, we introduce a new exception `EmailClientNonRetryableException`, that represents any exception back from an email client that we can use whenever we get a `InvalidParameterValue` error. Note, I chose `EmailClientNonRetryableException` rather than `SESClientNonRetryableException` as our code needs to catch this exception and it shouldn't be aware of what email client is being used, it just needs to know that it came from one of the email clients (if in time we have more than one). In time, we may wish to extend the approach of having generic `EmailClient` exceptions and `SMSClient` exceptions as this should be the most extendable pattern and a good abstraction.
This commit is contained in:
@@ -1,11 +1,11 @@
|
||||
import pytest
|
||||
from botocore.exceptions import ClientError
|
||||
from celery.exceptions import MaxRetriesExceededError
|
||||
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 import EmailClientNonRetryableException
|
||||
from app.clients.email.aws_ses import AwsSesClientException, AwsSesClientThrottlingSendRateException
|
||||
from app.exceptions import NotificationTechnicalFailureException
|
||||
|
||||
@@ -93,8 +93,11 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task
|
||||
assert sample_notification.status == 'technical-failure'
|
||||
|
||||
|
||||
def test_should_technical_error_and_not_retry_if_invalid_email(sample_notification, mocker):
|
||||
mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=InvalidEmailError('bad email'))
|
||||
def test_should_technical_error_and_not_retry_if_EmailClientNonRetryableException(sample_notification, mocker):
|
||||
mocker.patch(
|
||||
'app.delivery.send_to_providers.send_email_to_provider',
|
||||
side_effect=EmailClientNonRetryableException('bad email')
|
||||
)
|
||||
mocker.patch('app.celery.provider_tasks.deliver_email.retry')
|
||||
|
||||
deliver_email(sample_notification.id)
|
||||
|
||||
Reference in New Issue
Block a user