Merge pull request #2580 from alphagov/set-bad-email-addresses-to-perm-fail

Set bad email addresses to permanent-failure
This commit is contained in:
Katie Smith
2019-08-12 13:24:27 +01:00
committed by GitHub
5 changed files with 5 additions and 162 deletions

View File

@@ -114,7 +114,6 @@ def register_blueprint(application):
from app.events.rest import events as events_blueprint
from app.provider_details.rest import provider_details as provider_details_blueprint
from app.email_branding.rest import email_branding_blueprint
from app.delivery.rest import delivery_blueprint
from app.inbound_number.rest import inbound_number_blueprint
from app.inbound_sms.rest import inbound_sms as inbound_sms_blueprint
from app.notifications.receive_notifications import receive_notifications_blueprint
@@ -160,9 +159,6 @@ def register_blueprint(application):
invite_blueprint.before_request(requires_admin_auth)
application.register_blueprint(invite_blueprint)
delivery_blueprint.before_request(requires_admin_auth)
application.register_blueprint(delivery_blueprint)
inbound_number_blueprint.before_request(requires_admin_auth)
application.register_blueprint(inbound_number_blueprint)

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.delivery import send_to_providers
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)
@@ -47,8 +47,8 @@ def deliver_email(self, notification_id):
raise NoResultFound()
send_to_providers.send_email_to_provider(notification)
except InvalidEmailError as e:
current_app.logger.exception(e)
update_notification_status_by_id(notification_id, 'technical-failure')
current_app.logger.info(e)
update_notification_status_by_id(notification_id, NOTIFICATION_PERMANENT_FAILURE)
except Exception:
try:
current_app.logger.exception(

View File

@@ -1,48 +0,0 @@
from flask import Blueprint, jsonify, current_app
from app.config import QueueNames
from app.delivery import send_to_providers
from app.models import EMAIL_TYPE
from app.celery import provider_tasks
from app.dao import notifications_dao
from app.errors import register_errors
delivery_blueprint = Blueprint('delivery', __name__)
register_errors(delivery_blueprint)
@delivery_blueprint.route('/deliver/notification/<uuid:notification_id>', methods=['POST'])
def send_notification_to_provider(notification_id):
notification = notifications_dao.get_notification_by_id(notification_id)
if not notification:
return jsonify({"result": "error", "message": "No result found"}), 404
if notification.notification_type == EMAIL_TYPE:
send_response(
send_to_providers.send_email_to_provider,
provider_tasks.deliver_email,
notification,
QueueNames.SEND_EMAIL
)
else:
send_response(
send_to_providers.send_sms_to_provider,
provider_tasks.deliver_sms,
notification,
QueueNames.SEND_SMS
)
return jsonify({}), 204
def send_response(send_call, task_call, notification, queue):
try:
send_call(notification)
except Exception as e:
current_app.logger.exception(
"Failed to send notification, retrying in celery. ID {} type {}".format(
notification.id,
notification.notification_type),
e)
task_call.apply_async((str(notification.id)), queue=queue)

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
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.celery.provider_tasks.deliver_email.retry')
deliver_email(sample_notification.id)
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):

View File

@@ -1,105 +0,0 @@
from flask import json
import app
from tests import create_authorization_header
def test_should_reject_if_not_authenticated(notify_api):
with notify_api.test_request_context():
with notify_api.test_client() as client:
response = client.post('/deliver/notification/{}'.format(app.create_uuid()))
assert response.status_code == 401
def test_should_reject_if_invalid_uuid(notify_api):
with notify_api.test_request_context():
with notify_api.test_client() as client:
auth = create_authorization_header()
response = client.post(
'/deliver/notification/{}',
headers=[auth]
)
body = json.loads(response.get_data(as_text=True))
assert response.status_code == 404
assert body['message'] == 'The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.' # noqa
assert body['result'] == 'error'
def test_should_reject_if_notification_id_cannot_be_found(notify_api):
with notify_api.test_request_context():
with notify_api.test_client() as client:
auth = create_authorization_header()
response = client.post(
'/deliver/notification/{}'.format(app.create_uuid()),
headers=[auth]
)
body = json.loads(response.get_data(as_text=True))
assert response.status_code == 404
assert body['message'] == 'No result found'
assert body['result'] == 'error'
def test_should_call_send_sms_to_provider_as_primary(notify_api, sample_notification, mocker):
mocker.patch('app.delivery.send_to_providers.send_sms_to_provider')
with notify_api.test_request_context():
with notify_api.test_client() as client:
auth = create_authorization_header()
response = client.post(
'/deliver/notification/{}'.format(sample_notification.id),
headers=[auth]
)
app.delivery.send_to_providers.send_sms_to_provider.assert_called_with(sample_notification)
assert response.status_code == 204
def test_should_call_send_email_to_provider_as_primary(notify_api, sample_email_notification, mocker):
mocker.patch('app.delivery.send_to_providers.send_email_to_provider')
with notify_api.test_request_context():
with notify_api.test_client() as client:
auth = create_authorization_header()
response = client.post(
'/deliver/notification/{}'.format(sample_email_notification.id),
headers=[auth]
)
app.delivery.send_to_providers.send_email_to_provider.assert_called_with(sample_email_notification)
assert response.status_code == 204
def test_should_call_deliver_sms_task_if_send_sms_to_provider_fails(notify_api, sample_notification, mocker):
mocker.patch('app.delivery.send_to_providers.send_sms_to_provider', side_effect=Exception())
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
with notify_api.test_request_context():
with notify_api.test_client() as client:
auth = create_authorization_header()
response = client.post(
'/deliver/notification/{}'.format(sample_notification.id),
headers=[auth]
)
app.delivery.send_to_providers.send_sms_to_provider.assert_called_with(sample_notification)
app.celery.provider_tasks.deliver_sms.apply_async.assert_called_with(
(str(sample_notification.id)), queue='send-sms-tasks'
)
assert response.status_code == 204
def test_should_call_deliver_email_task_if_send_email_to_provider_fails(
notify_api,
sample_email_notification,
mocker
):
mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=Exception())
mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
with notify_api.test_request_context():
with notify_api.test_client() as client:
auth = create_authorization_header()
response = client.post(
'/deliver/notification/{}'.format(sample_email_notification.id),
headers=[auth]
)
app.delivery.send_to_providers.send_email_to_provider.assert_called_with(sample_email_notification)
app.celery.provider_tasks.deliver_email.apply_async.assert_called_with(
(str(sample_email_notification.id)), queue='send-email-tasks'
)
assert response.status_code == 204