Now that we have discovered that the catch all Exception handler doesn't work, I've created a custom exception (SendNotificationQueueError) that handles this case and an error handler for it.

Talked these through with @becca as an approach.
This commit is contained in:
Martyn Inglis
2016-11-21 15:11:19 +00:00
parent 7cfc58c994
commit 58bbc5a5aa
6 changed files with 27 additions and 8 deletions

View File

@@ -6,6 +6,7 @@ from sqlalchemy.exc import SQLAlchemyError, DataError
from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.exc import NoResultFound
from marshmallow import ValidationError from marshmallow import ValidationError
from app.authentication.auth import AuthError from app.authentication.auth import AuthError
from app.notifications import SendNotificationToQueueError
class InvalidRequest(Exception): class InvalidRequest(Exception):
@@ -83,6 +84,11 @@ def register_errors(blueprint):
current_app.logger.exception(e) current_app.logger.exception(e)
return jsonify(result='error', message="No result found"), 404 return jsonify(result='error', message="No result found"), 404
@blueprint.errorhandler(SendNotificationToQueueError)
def failed_to_create_notification(e):
current_app.logger.exception(e)
return jsonify(result='error', message=e.message), 500
@blueprint.errorhandler(SQLAlchemyError) @blueprint.errorhandler(SQLAlchemyError)
def db_error(e): def db_error(e):
current_app.logger.exception(e) current_app.logger.exception(e)

View File

@@ -0,0 +1,7 @@
class SendNotificationToQueueError(Exception):
status_code = 500
def __init__(self):
self.message = "Failed to create the notification"

View File

@@ -9,7 +9,7 @@ from app.celery import provider_tasks
from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id
from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE
from app.notifications.validators import check_sms_content_char_count from app.notifications.validators import check_sms_content_char_count
from app.v2.errors import BadRequestError from app.v2.errors import BadRequestError, SendNotificationToQueueError
def create_content_for_notification(template, personalisation): def create_content_for_notification(template, personalisation):
@@ -78,9 +78,9 @@ def send_notification_to_queue(notification, research_mode):
queue='send-email' if not research_mode else 'research-mode' queue='send-email' if not research_mode else 'research-mode'
) )
except Exception as e: except Exception as e:
current_app.logger.exception("Failed to send to SQS exception") current_app.logger.exception(e)
dao_delete_notifications_and_history_by_id(notification.id) dao_delete_notifications_and_history_by_id(notification.id)
raise e raise SendNotificationToQueueError()
current_app.logger.info( current_app.logger.info(
"{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at) "{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at)

View File

@@ -242,10 +242,7 @@ def send_notification(notification_type):
notification_type=notification_type, notification_type=notification_type,
api_key_id=api_user.id, api_key_id=api_user.id,
key_type=api_user.key_type) key_type=api_user.key_type)
try: send_notification_to_queue(saved_notification, service.research_mode)
send_notification_to_queue(saved_notification, service.research_mode)
except Exception as e:
return jsonify(result='error', message="Internal server error"), 500
notification_id = create_uuid() if saved_notification is None else saved_notification.id notification_id = create_uuid() if saved_notification is None else saved_notification.id
notification.update({"template_version": template.version}) notification.update({"template_version": template.version})

View File

@@ -5,6 +5,7 @@ from sqlalchemy.exc import DataError
from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.exc import NoResultFound
from app.authentication.auth import AuthError from app.authentication.auth import AuthError
from app.errors import InvalidRequest from app.errors import InvalidRequest
from app.notifications import SendNotificationToQueueError
class TooManyRequestsError(InvalidRequest): class TooManyRequestsError(InvalidRequest):
@@ -47,6 +48,13 @@ def register_errors(blueprint):
def auth_error(error): def auth_error(error):
return jsonify(error.to_dict_v2()), error.code return jsonify(error.to_dict_v2()), error.code
@blueprint.errorhandler(SendNotificationToQueueError)
def failed_to_create_notification(error):
current_app.logger.exception(error)
return jsonify(
status_code=500,
errors=[{"error": error.__class__.__name__, "message": error.message}]), 500
@blueprint.errorhandler(Exception) @blueprint.errorhandler(Exception)
def internal_server_error(error): def internal_server_error(error):
current_app.logger.exception(error) current_app.logger.exception(error)

View File

@@ -5,6 +5,7 @@ from boto3.exceptions import Boto3Error
from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.exc import SQLAlchemyError
from app.models import Template, Notification, NotificationHistory from app.models import Template, Notification, NotificationHistory
from app.notifications import SendNotificationToQueueError
from app.notifications.process_notifications import (create_content_for_notification, from app.notifications.process_notifications import (create_content_for_notification,
persist_notification, send_notification_to_queue) persist_notification, send_notification_to_queue)
from app.v2.errors import BadRequestError from app.v2.errors import BadRequestError
@@ -108,7 +109,7 @@ def test_send_notification_to_queue(notify_db, notify_db_session,
def test_send_notification_to_queue_throws_exception_deletes_notification(sample_notification, mocker): def test_send_notification_to_queue_throws_exception_deletes_notification(sample_notification, mocker):
mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Boto3Error("EXPECTED")) mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Boto3Error("EXPECTED"))
with pytest.raises(Boto3Error): with pytest.raises(SendNotificationToQueueError):
send_notification_to_queue(sample_notification, False) send_notification_to_queue(sample_notification, False)
mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms') mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms')