darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-02-04 02:11:11 -05:00
Code Issues Packages Projects Releases Wiki Activity

Change email status to permanent-failure if SES raises InvalidParameterValue

Browse Source 
If SES raised an `InvalidParameterValue` error (because an email address
was wrong) we were logging an exception and setting the email status to
`technical-failure`. We now set it to `permanent-failure` instead and
change the log level to `info` - setting it to `permanent-failure` means
that people will know not to retry the message.
This commit is contained in:
Katie Smith
2019-08-12 10:24:59 +01:00
parent 6ab6f09b7b
commit 51716fbaf8
2 changed files with 5 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/celery/provider_tasks.py
View File

@@ -9,7 +9,7 @@ from app.dao import notifications_dao
from app.dao.notifications_dao import update_notification_status_by_id from app.dao.notifications_dao import update_notification_status_by_id
from app.delivery import send_to_providers from app.delivery import send_to_providers
from app.exceptions import NotificationTechnicalFailureException from app.exceptions import NotificationTechnicalFailureException
from app.models import NOTIFICATION_TECHNICAL_FAILURE from app.models import NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TECHNICAL_FAILURE
@notify_celery.task(bind=True, name="deliver_sms", max_retries=48, default_retry_delay=300) @notify_celery.task(bind=True, name="deliver_sms", max_retries=48, default_retry_delay=300)
@@ -47,8 +47,8 @@ def deliver_email(self, notification_id):
raise NoResultFound() raise NoResultFound()
send_to_providers.send_email_to_provider(notification) send_to_providers.send_email_to_provider(notification)
except InvalidEmailError as e: except InvalidEmailError as e:
current_app.logger.exception(e) current_app.logger.info(e)
update_notification_status_by_id(notification_id, 'technical-failure') update_notification_status_by_id(notification_id, NOTIFICATION_PERMANENT_FAILURE)
except Exception: except Exception:
try: try:
current_app.logger.exception( current_app.logger.exception(

4
tests/app/celery/test_provider_tasks.py
View File

@@ -84,14 +84,14 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task
assert str(sample_notification.id) in e.value.message assert str(sample_notification.id) in e.value.message
def test_should_technical_error_and_not_retry_if_invalid_email(sample_notification, mocker): def test_should_go_into_permanent_failure_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')) mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=InvalidEmailError('bad email'))
mocker.patch('app.celery.provider_tasks.deliver_email.retry') mocker.patch('app.celery.provider_tasks.deliver_email.retry')
deliver_email(sample_notification.id) deliver_email(sample_notification.id)
assert provider_tasks.deliver_email.retry.called is False assert provider_tasks.deliver_email.retry.called is False
assert sample_notification.status == 'technical-failure' assert sample_notification.status == 'permanent-failure'
def test_should_retry_and_log_exception(sample_notification, mocker): def test_should_retry_and_log_exception(sample_notification, mocker):