Add notification ID to SES bounce reason

At the moment we log everytime we get a bounce from SES, however we
don't link it to a particular notification so it's hard to know for what
sub reason a notifcation did not deliver by looking at the logs.

This commit changes this by now looking the bounce reason after we have
found the notification ID and including them together. So if you know
search for a notification ID in Kibana, you will see full logs for why
it failed to deliver.
This commit is contained in:
David McDonald
2020-11-20 14:10:13 +00:00
parent 3aa602bd6b
commit 43f1f48093
3 changed files with 10 additions and 3 deletions

View File

@@ -26,9 +26,10 @@ def process_ses_results(self, response):
try:
ses_message = json.loads(response['Message'])
notification_type = ses_message['notificationType']
bounce_message = None
if notification_type == 'Bounce':
notification_type = determine_notification_bounce_type(notification_type, ses_message)
notification_type, bounce_message = determine_notification_bounce_type(notification_type, ses_message)
elif notification_type == 'Complaint':
_check_and_queue_complaint_callback_task(*handle_complaint(ses_message))
return True
@@ -54,6 +55,9 @@ def process_ses_results(self, response):
)
return
if bounce_message:
current_app.logger.info(f"SES bounce for notification ID {notification.id}: {bounce_message}")
if notification.status not in [NOTIFICATION_SENDING, NOTIFICATION_PENDING]:
notifications_dao._duplicate_update_warning(
notification=notification,

View File

@@ -17,12 +17,11 @@ from app.config import QueueNames
def determine_notification_bounce_type(notification_type, ses_message):
remove_emails_from_bounce(ses_message)
current_app.logger.info('SES bounce dict: {}'.format(ses_message))
if ses_message['bounce']['bounceType'] == 'Permanent':
notification_type = ses_message['bounce']['bounceType'] # permanent or not
else:
notification_type = 'Temporary'
return notification_type
return notification_type, ses_message
def handle_complaint(ses_message):

View File

@@ -200,6 +200,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
send_mock = mocker.patch(
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
)
mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.info')
notification = create_notification(
template=sample_email_template,
status='sending',
@@ -210,6 +211,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client,
assert process_ses_results(ses_soft_bounce_callback(reference='ref'))
assert get_notification_by_id(notification.id).status == 'temporary-failure'
assert send_mock.called
assert f'SES bounce for notification ID {notification.id}: ' in mock_logger.call_args[0][0]
def test_ses_callback_should_set_status_to_permanent_failure(client,
@@ -219,6 +221,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
send_mock = mocker.patch(
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
)
mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.info')
notification = create_notification(
template=sample_email_template,
status='sending',
@@ -230,6 +233,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client,
assert process_ses_results(ses_hard_bounce_callback(reference='ref'))
assert get_notification_by_id(notification.id).status == 'permanent-failure'
assert send_mock.called
assert f'SES bounce for notification ID {notification.id}: ' in mock_logger.call_args[0][0]
def test_ses_callback_should_send_on_complaint_to_user_callback_api(sample_email_template, mocker):