Added a new exception type for DVLAException.

The Notify team needs to investigate when a notification is marked as failed.
We will process the whole file and mark the notifications with the appropriate status, if any are failed an exception is raised.
The exception will trigger a cloud watch error for the team to investigate.

app/celery/celery.py

@@ -18,7 +18,7 @@ def make_task(app):
 
         def on_failure(self, exc, task_id, args, kwargs, einfo):
             # ensure task will log exceptions to correct handlers
-            app.logger.exception('Celery task failed')
+            app.logger.exception('Celery task: {} failed'.format(self.name))
             super().on_failure(exc, task_id, args, kwargs, einfo)
 
         def __call__(self, *args, **kwargs):

app/celery/tasks.py

@@ -52,6 +52,7 @@ from app.dao.provider_details_dao import get_current_provider
 from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service
 from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count
 from app.dao.templates_dao import dao_get_template_by_id
+from app.exceptions import DVLAException
 from app.models import (
     DVLA_RESPONSE_STATUS_SENT,
     EMAIL_TYPE,
@@ -469,31 +470,18 @@ def update_letter_notifications_statuses(self, filename):
     try:
         notification_updates = process_updates_from_file(response_file_content)
     except TypeError:
-        current_app.logger.exception('DVLA response file: {} has an invalid format'.format(filename))
-        raise
+        raise DVLAException('DVLA response file: {} has an invalid format'.format(filename))
     else:
+        temporary_failures = []
         for update in notification_updates:
             check_billable_units(update)
+            update_letter_notification(filename, temporary_failures, update)
 
-            status = NOTIFICATION_DELIVERED if update.status == DVLA_RESPONSE_STATUS_SENT \
-                else NOTIFICATION_TEMPORARY_FAILURE
-            updated_count = dao_update_notifications_by_reference(
-                references=[update.reference],
-                update_dict={"status": status,
-                             "billable_units": update.page_count,
-                             "updated_at": datetime.utcnow()
-                             }
-            )
-
-            if not updated_count:
-                msg = "Update letter notification file {filename} failed: notification either not found " \
-                    "or already updated from delivered. Status {status} for notification reference {reference}".format(
-                        filename=filename, status=status, reference=update.reference)
-                current_app.logger.error(msg)
-            else:
-                current_app.logger.info(
-                    'DVLA file: {filename}, notification updated to {status}: {reference}'.format(
-                        filename=filename, status=status, reference=str(update.reference)))
+        if temporary_failures:
+            # This will alert Notify that DVLA was unable to deliver the letters, we need to investigate
+            message = "DVLA response file: {filename} has failed letters with notification.reference {failures}".format(
+                filename=filename, failures=temporary_failures)
+            raise DVLAException(message)
 
 
 def process_updates_from_file(response_file):
@@ -502,6 +490,28 @@ def process_updates_from_file(response_file):
     return notification_updates
 
 
+def update_letter_notification(filename, temporary_failures, update):
+    if update.status == DVLA_RESPONSE_STATUS_SENT:
+        status = NOTIFICATION_DELIVERED
+    else:
+        status = NOTIFICATION_TEMPORARY_FAILURE
+        temporary_failures.append(update.reference)
+
+    updated_count = dao_update_notifications_by_reference(
+        references=[update.reference],
+        update_dict={"status": status,
+                     "billable_units": update.page_count,
+                     "updated_at": datetime.utcnow()
+                     }
+    )
+
+    if not updated_count:
+        msg = "Update letter notification file {filename} failed: notification either not found " \
+              "or already updated from delivered. Status {status} for notification reference {reference}".format(
+                  filename=filename, status=status, reference=update.reference)
+        current_app.logger.error(msg)
+
+
 def check_billable_units(notification_update):
     notification = dao_get_notification_by_reference(notification_update.reference)
 

app/exceptions.py

@@ -0,0 +1,3 @@
+class DVLAException(Exception):
+    def __init__(self, message):
+        self.message = message

tests/app/celery/test_ftp_update_tasks.py

@@ -5,6 +5,7 @@ import pytest
 from freezegun import freeze_time
 from flask import current_app
 
+from app.exceptions import DVLAException
 from app.models import (
     Job,
     Notification,
@@ -65,8 +66,23 @@ def test_update_letter_notifications_statuses_raises_for_invalid_format(notify_a
     invalid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2'
     mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=invalid_file)
 
-    with pytest.raises(TypeError):
+    with pytest.raises(DVLAException) as e:
         update_letter_notifications_statuses(filename='foo.txt')
+    assert 'DVLA response file: {} has an invalid format'.format('foo.txt') in str(e)
+
+
+def test_update_letter_notifications_statuses_raises_dvla_exception(notify_api, mocker, sample_letter_template):
+    valid_file = 'ref-foo|Failed|1|Unsorted'
+    mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file)
+    create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING,
+                        billable_units=0)
+
+    with pytest.raises(DVLAException) as e:
+        update_letter_notifications_statuses(filename="failed.txt")
+    failed = ["ref-foo"]
+    assert "DVLA response file: {filename} has failed letters with notification.reference {failures}".format(
+        filename="failed.txt", failures=failed
+    ) in str(e)
 
 
 def test_update_letter_notifications_statuses_calls_with_correct_bucket_location(notify_api, mocker):
@@ -114,7 +130,8 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp
         sent_letter.reference, failed_letter.reference)
     mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file)
 
-    update_letter_notifications_statuses(filename='foo.txt')
+    with pytest.raises(expected_exception=DVLAException) as e:
+        update_letter_notifications_statuses(filename='foo.txt')
 
     assert sent_letter.status == NOTIFICATION_DELIVERED
     assert sent_letter.billable_units == 1
@@ -122,6 +139,8 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp
     assert failed_letter.status == NOTIFICATION_TEMPORARY_FAILURE
     assert failed_letter.billable_units == 2
     assert failed_letter.updated_at
+    assert "DVLA response file: {filename} has failed letters with notification.reference {failures}".format(
+        filename="foo.txt", failures=[format(failed_letter.reference)]) in str(e)
 
 
 def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry(notify_api, mocker,