mirror of
https://github.com/GSA/notifications-api.git
synced 2026-01-31 23:26:23 -05:00
Add retries for SES callbacks for recent notifications
We've seen errors caused by what we suspect is a race condition when SES callback processing tries to look up the notification before the sender worker has saved notification reference from the SES POST response to the database. This adds a retry for SES callback task if the notification was not found and the message is less than 10 minutes old and removes the error log message for notifications older than 3 days (since they might no longer exist in the notifications table and would've been marked as failure by then either way). In order to be able to call retry and silence the error log based on notification time this change inlines `process_ses_response` and `update_notification_by_reference` functions into the celery task. It also removes a lot of defensive error-handling that doesn't appear to have been triggered in the last few months (for things like missing keys in SES callback data).
This commit is contained in:
Alexey Bezhan
@@ -2,155 +2,20 @@ from datetime import datetime
|
||||
|
||||
import pytest
|
||||
from flask import json
|
||||
from freezegun import freeze_time
|
||||
from sqlalchemy.exc import SQLAlchemyError
|
||||
|
||||
from app import statsd_client, encryption
|
||||
from app.dao.notifications_dao import get_notification_by_id
|
||||
from app.models import Notification, Complaint
|
||||
from app.notifications.notifications_ses_callback import (
|
||||
process_ses_response,
|
||||
handle_complaint
|
||||
)
|
||||
from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback
|
||||
from app.celery.service_callback_tasks import create_delivery_status_callback_data
|
||||
from app.models import Complaint
|
||||
from app.notifications.notifications_ses_callback import handle_complaint
|
||||
|
||||
from tests.app.conftest import sample_notification as create_sample_notification
|
||||
from tests.app.db import (
|
||||
create_service_callback_api, create_notification, ses_complaint_callback_malformed_message_id,
|
||||
create_notification, ses_complaint_callback_malformed_message_id,
|
||||
ses_complaint_callback_with_missing_complaint_type,
|
||||
ses_complaint_callback
|
||||
)
|
||||
|
||||
|
||||
def test_ses_callback_should_update_notification_status(
|
||||
client,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
sample_email_template,
|
||||
mocker):
|
||||
with freeze_time('2001-01-01T12:00:00'):
|
||||
mocker.patch('app.statsd_client.incr')
|
||||
mocker.patch('app.statsd_client.timing_with_dates')
|
||||
send_mock = mocker.patch(
|
||||
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
|
||||
)
|
||||
notification = create_sample_notification(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template=sample_email_template,
|
||||
reference='ref',
|
||||
status='sending',
|
||||
sent_at=datetime.utcnow()
|
||||
)
|
||||
callback_api = create_service_callback_api(service=sample_email_template.service,
|
||||
url="https://original_url.com")
|
||||
assert get_notification_by_id(notification.id).status == 'sending'
|
||||
|
||||
errors = process_ses_response(ses_notification_callback(reference='ref'))
|
||||
assert errors is None
|
||||
assert get_notification_by_id(notification.id).status == 'delivered'
|
||||
statsd_client.timing_with_dates.assert_any_call(
|
||||
"callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at
|
||||
)
|
||||
statsd_client.incr.assert_any_call("callback.ses.delivered")
|
||||
updated_notification = Notification.query.get(notification.id)
|
||||
encrypted_data = create_delivery_status_callback_data(updated_notification, callback_api)
|
||||
send_mock.assert_called_once_with([str(notification.id), encrypted_data], queue="service-callbacks")
|
||||
|
||||
|
||||
def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry(
|
||||
client,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
sample_email_template,
|
||||
mocker):
|
||||
with freeze_time('2001-01-01T12:00:00'):
|
||||
|
||||
send_mock = mocker.patch(
|
||||
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
|
||||
)
|
||||
notification = create_sample_notification(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template=sample_email_template,
|
||||
reference='ref',
|
||||
status='sending',
|
||||
sent_at=datetime.utcnow()
|
||||
)
|
||||
|
||||
assert get_notification_by_id(notification.id).status == 'sending'
|
||||
|
||||
errors = process_ses_response(ses_notification_callback(reference='ref'))
|
||||
assert errors is None
|
||||
assert get_notification_by_id(notification.id).status == 'delivered'
|
||||
|
||||
send_mock.assert_not_called()
|
||||
|
||||
|
||||
def test_ses_callback_should_update_multiple_notification_status_sent(
|
||||
client,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
sample_email_template,
|
||||
mocker):
|
||||
|
||||
send_mock = mocker.patch(
|
||||
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
|
||||
)
|
||||
create_sample_notification(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template=sample_email_template,
|
||||
reference='ref1',
|
||||
sent_at=datetime.utcnow(),
|
||||
status='sending')
|
||||
|
||||
create_sample_notification(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template=sample_email_template,
|
||||
reference='ref2',
|
||||
sent_at=datetime.utcnow(),
|
||||
status='sending')
|
||||
|
||||
create_sample_notification(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template=sample_email_template,
|
||||
reference='ref3',
|
||||
sent_at=datetime.utcnow(),
|
||||
status='sending')
|
||||
create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")
|
||||
assert process_ses_response(ses_notification_callback(reference='ref1')) is None
|
||||
assert process_ses_response(ses_notification_callback(reference='ref2')) is None
|
||||
assert process_ses_response(ses_notification_callback(reference='ref3')) is None
|
||||
assert send_mock.called
|
||||
|
||||
|
||||
def test_ses_callback_should_set_status_to_temporary_failure(client,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
sample_email_template,
|
||||
mocker):
|
||||
send_mock = mocker.patch(
|
||||
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
|
||||
)
|
||||
notification = create_sample_notification(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template=sample_email_template,
|
||||
reference='ref',
|
||||
status='sending',
|
||||
sent_at=datetime.utcnow()
|
||||
)
|
||||
create_service_callback_api(service=notification.service, url="https://original_url.com")
|
||||
assert get_notification_by_id(notification.id).status == 'sending'
|
||||
assert process_ses_response(ses_soft_bounce_callback(reference='ref')) is None
|
||||
assert get_notification_by_id(notification.id).status == 'temporary-failure'
|
||||
assert send_mock.called
|
||||
|
||||
|
||||
def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
@@ -168,30 +33,6 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered(client,
|
||||
assert get_notification_by_id(notification.id).status == 'delivered'
|
||||
|
||||
|
||||
def test_ses_callback_should_set_status_to_permanent_failure(client,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
sample_email_template,
|
||||
mocker):
|
||||
send_mock = mocker.patch(
|
||||
'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async'
|
||||
)
|
||||
notification = create_sample_notification(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template=sample_email_template,
|
||||
reference='ref',
|
||||
status='sending',
|
||||
sent_at=datetime.utcnow()
|
||||
)
|
||||
create_service_callback_api(service=sample_email_template.service, url="https://original_url.com")
|
||||
|
||||
assert get_notification_by_id(notification.id).status == 'sending'
|
||||
assert process_ses_response(ses_hard_bounce_callback(reference='ref')) is None
|
||||
assert get_notification_by_id(notification.id).status == 'permanent-failure'
|
||||
assert send_mock.called
|
||||
|
||||
|
||||
def test_process_ses_results_in_complaint(sample_email_template):
|
||||
notification = create_notification(template=sample_email_template, reference='ref1')
|
||||
handle_complaint(json.loads(ses_complaint_callback()['Message']))
|
||||
@@ -220,28 +61,3 @@ def test_process_ses_results_in_complaint_save_complaint_with_null_complaint_typ
|
||||
assert len(complaints) == 1
|
||||
assert complaints[0].notification_id == notification.id
|
||||
assert not complaints[0].complaint_type
|
||||
|
||||
|
||||
def test_ses_callback_should_send_on_complaint_to_user_callback_api(sample_email_template, mocker):
|
||||
send_mock = mocker.patch(
|
||||
'app.celery.service_callback_tasks.send_complaint_to_service.apply_async'
|
||||
)
|
||||
create_service_callback_api(
|
||||
service=sample_email_template.service, url="https://original_url.com", callback_type="complaint"
|
||||
)
|
||||
|
||||
notification = create_notification(template=sample_email_template, reference='ref1')
|
||||
response = ses_complaint_callback()
|
||||
errors = process_ses_response(response)
|
||||
assert errors is None
|
||||
|
||||
assert send_mock.call_count == 1
|
||||
assert encryption.decrypt(send_mock.call_args[0][0][0]) == {
|
||||
'complaint_date': '2018-06-05T13:59:58.000000Z',
|
||||
'complaint_id': str(Complaint.query.one().id),
|
||||
'notification_id': str(notification.id),
|
||||
'reference': None,
|
||||
'service_callback_api_bearer_token': 'some_super_secret',
|
||||
'service_callback_api_url': 'https://original_url.com',
|
||||
'to': 'recipient1@example.com'
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user