Throw an exception whenever we updated a notification to technical failure.

If this is happening we want to know about it.

app/celery/provider_tasks.py

@@ -9,6 +9,8 @@ from app.config import QueueNames
 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
 
 
 @worker_process_shutdown.connect
@@ -31,10 +33,10 @@ def deliver_sms(self, notification_id):
             )
             self.retry(queue=QueueNames.RETRY)
         except self.MaxRetriesExceededError:
-            current_app.logger.exception(
-                "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification_id),
-            )
-            update_notification_status_by_id(notification_id, 'technical-failure')
+            message = "RETRY FAILED: task send_sms_to_provider failed for notification {}. " \
+                      "Notification has been updated to technical-failure".format(notification_id)
+            update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
+            raise NotificationTechnicalFailureException(message)
 
 
 @notify_celery.task(bind=True, name="deliver_email", max_retries=48, default_retry_delay=300)
@@ -55,7 +57,7 @@ def deliver_email(self, notification_id):
             )
             self.retry(queue=QueueNames.RETRY)
         except self.MaxRetriesExceededError:
-            current_app.logger.error(
-                "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id)
-            )
-            update_notification_status_by_id(notification_id, 'technical-failure')
+            message = "RETRY FAILED: task send_email_to_provider failed for notification {}. " \
+                      "Notification has been updated to technical-failure".format(notification_id)
+            update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
+            raise NotificationTechnicalFailureException(message)

app/celery/scheduled_tasks.py

@@ -16,6 +16,7 @@ from app.dao.services_dao import (
     dao_fetch_monthly_historical_stats_by_template
 )
 from app.dao.stats_template_usage_by_month_dao import insert_or_update_stats_for_template
+from app.exceptions import NotificationTechnicalFailureException
 from app.performance_platform import total_sent_notifications, processing_time
 from app import performance_platform_client, deskpro_client
 from app.dao.date_util import get_month_start_and_end_date_in_utc
@@ -201,8 +202,10 @@ def delete_invitations():
 @notify_celery.task(name='timeout-sending-notifications')
 @statsd(namespace="tasks")
 def timeout_notifications():
-    notifications = dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD'))
+    technical_failure_notifications, temporary_failure_notifications = \
+        dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD'))
 
+    notifications = technical_failure_notifications + temporary_failure_notifications
     for notification in notifications:
         # queue callback task only if the service_callback_api exists
         service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id)
@@ -213,6 +216,11 @@ def timeout_notifications():
 
     current_app.logger.info(
         "Timeout period reached for {} notifications, status has been updated.".format(len(notifications)))
+    if technical_failure_notifications:
+        message = "{} notifications have been updated to technical-failure because they " \
+                  "have timed out and are still in created.Notification ids: {}".format(
+                      len(technical_failure_notifications), [str(x.id) for x in technical_failure_notifications])
+        raise NotificationTechnicalFailureException(message)
 
 
 @notify_celery.task(name='send-daily-performance-platform-stats')

app/celery/tasks.py

@@ -50,7 +50,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.exceptions import DVLAException, NotificationTechnicalFailureException
 from app.models import (
     DVLA_RESPONSE_STATUS_SENT,
     EMAIL_TYPE,
@@ -371,8 +371,10 @@ def update_letter_notifications_to_error(self, notification_references):
             'updated_at': datetime.utcnow()
         }
     )
-
-    current_app.logger.debug("Updated {} letter notifications to technical-failure".format(updated_count))
+    message = "Updated {} letter notifications to technical-failure with references {}".format(
+        updated_count, notification_references
+    )
+    raise NotificationTechnicalFailureException(message=message)
 
 
 def handle_exception(task, notification, notification_id, exc):

app/dao/notifications_dao.py

@@ -375,14 +375,15 @@ def dao_timeout_notifications(timeout_period_in_seconds):
     timeout = functools.partial(_timeout_notifications, timeout_start=timeout_start, updated_at=updated_at)
 
     # Notifications still in created status are marked with a technical-failure:
-    updated_ids = timeout([NOTIFICATION_CREATED], NOTIFICATION_TECHNICAL_FAILURE)
+    technical_failure_notifications = timeout([NOTIFICATION_CREATED], NOTIFICATION_TECHNICAL_FAILURE)
 
     # Notifications still in sending or pending status are marked with a temporary-failure:
-    updated_ids += timeout([NOTIFICATION_SENDING, NOTIFICATION_PENDING], NOTIFICATION_TEMPORARY_FAILURE)
+    temporary_failure_notifications = timeout([NOTIFICATION_SENDING, NOTIFICATION_PENDING],
+                                              NOTIFICATION_TEMPORARY_FAILURE)
 
     db.session.commit()
 
-    return updated_ids
+    return technical_failure_notifications, temporary_failure_notifications
 
 
 def get_total_sent_notifications_in_date_range(start_date, end_date, notification_type):

app/delivery/send_to_providers.py

@@ -19,6 +19,7 @@ from app.dao.provider_details_dao import (
 )
 from app.celery.research_mode_tasks import send_sms_response, send_email_response
 from app.dao.templates_dao import dao_get_template_by_id
+from app.exceptions import NotificationTechnicalFailureException
 from app.models import (
     SMS_TYPE,
     KEY_TYPE_TEST,
@@ -211,7 +212,7 @@ def get_html_email_options(service):
 def technical_failure(notification):
     notification.status = NOTIFICATION_TECHNICAL_FAILURE
     dao_update_notification(notification)
-    current_app.logger.warn(
+    raise NotificationTechnicalFailureException(
         "Send {} for notification id {} to provider is not allowed: service {} is inactive".format(
             notification.notification_type,
             notification.id,

app/exceptions.py

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

tests/app/celery/test_ftp_update_tasks.py

@@ -5,7 +5,7 @@ import pytest
 from freezegun import freeze_time
 from flask import current_app
 
-from app.exceptions import DVLAException
+from app.exceptions import DVLAException, NotificationTechnicalFailureException
 from app.models import (
     Job,
     Notification,
@@ -276,7 +276,9 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe
     create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com")
     dt = datetime.utcnow()
     with freeze_time(dt):
-        update_letter_notifications_to_error([first.reference])
+        with pytest.raises(NotificationTechnicalFailureException) as e:
+            update_letter_notifications_to_error([first.reference])
+    assert first.reference in e.value.message
 
     assert first.status == NOTIFICATION_TECHNICAL_FAILURE
     assert first.sent_by is None

tests/app/celery/test_provider_tasks.py

@@ -1,9 +1,11 @@
+import pytest
 from celery.exceptions import MaxRetriesExceededError
 from notifications_utils.recipients import InvalidEmailError
 
 import app
 from app.celery import provider_tasks
 from app.celery.provider_tasks import deliver_sms, deliver_email
+from app.exceptions import NotificationTechnicalFailureException
 
 
 def test_should_have_decorated_tasks_functions():
@@ -59,21 +61,25 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task(s
     mocker.patch('app.delivery.send_to_providers.send_sms_to_provider', side_effect=Exception("EXPECTED"))
     mocker.patch('app.celery.provider_tasks.deliver_sms.retry', side_effect=MaxRetriesExceededError())
 
-    deliver_sms(sample_notification.id)
+    with pytest.raises(NotificationTechnicalFailureException) as e:
+        deliver_sms(sample_notification.id)
 
     provider_tasks.deliver_sms.retry.assert_called_with(queue="retry-tasks")
 
     assert sample_notification.status == 'technical-failure'
+    assert str(sample_notification.id) in e.value.message
 
 
 def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task(sample_notification, mocker):
     mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=Exception("EXPECTED"))
     mocker.patch('app.celery.provider_tasks.deliver_email.retry', side_effect=MaxRetriesExceededError())
 
-    deliver_email(sample_notification.id)
+    with pytest.raises(NotificationTechnicalFailureException) as e:
+        deliver_email(sample_notification.id)
 
     provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks")
     assert sample_notification.status == 'technical-failure'
+    assert str(sample_notification.id) in e.value.message
 
 
 def test_should_technical_error_and_not_retry_if_invalid_email(sample_notification, mocker):

tests/app/celery/test_scheduled_tasks.py

@@ -44,6 +44,7 @@ from app.dao.provider_details_dao import (
     dao_update_provider_details,
     get_current_provider
 )
+from app.exceptions import NotificationTechnicalFailureException
 from app.models import (
     MonthlyBilling,
     NotificationHistory,
@@ -181,8 +182,10 @@ def test_update_status_of_notifications_after_timeout(notify_api, sample_templat
             status='pending',
             created_at=datetime.utcnow() - timedelta(
                 seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
-        timeout_notifications()
-
+        with pytest.raises(NotificationTechnicalFailureException) as e:
+            timeout_notifications()
+        print(e.value.message)
+        assert str(not2.id) in e.value.message
         assert not1.status == 'temporary-failure'
         assert not2.status == 'technical-failure'
         assert not3.status == 'temporary-failure'

tests/app/dao/notification_dao/test_notification_dao.py

@@ -1222,7 +1222,7 @@ def test_dao_timeout_notifications(sample_template):
     assert Notification.query.get(sending.id).status == 'sending'
     assert Notification.query.get(pending.id).status == 'pending'
     assert Notification.query.get(delivered.id).status == 'delivered'
-    updated_ids = dao_timeout_notifications(1)
+    technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1)
     assert Notification.query.get(created.id).status == 'technical-failure'
     assert Notification.query.get(sending.id).status == 'temporary-failure'
     assert Notification.query.get(pending.id).status == 'temporary-failure'
@@ -1231,7 +1231,7 @@ def test_dao_timeout_notifications(sample_template):
     assert NotificationHistory.query.get(sending.id).status == 'temporary-failure'
     assert NotificationHistory.query.get(pending.id).status == 'temporary-failure'
     assert NotificationHistory.query.get(delivered.id).status == 'delivered'
-    assert len(updated_ids) == 3
+    assert len(technical_failure_notifications + temporary_failure_notifications) == 3
 
 
 def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_template):
@@ -1245,12 +1245,12 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_t
     assert Notification.query.get(sending.id).status == 'sending'
     assert Notification.query.get(pending.id).status == 'pending'
     assert Notification.query.get(delivered.id).status == 'delivered'
-    updated_ids = dao_timeout_notifications(1)
+    technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1)
     assert NotificationHistory.query.get(created.id).status == 'created'
     assert NotificationHistory.query.get(sending.id).status == 'sending'
     assert NotificationHistory.query.get(pending.id).status == 'pending'
     assert NotificationHistory.query.get(delivered.id).status == 'delivered'
-    assert len(updated_ids) == 0
+    assert len(technical_failure_notifications + temporary_failure_notifications) == 0
 
 
 def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template):
@@ -1265,13 +1265,13 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template)
     assert Notification.query.get(pending.id).status == 'pending'
     assert Notification.query.get(delivered.id).status == 'delivered'
 
-    updated_ids = dao_timeout_notifications(1)
+    technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1)
 
     assert NotificationHistory.query.get(created.id).status == 'created'
     assert NotificationHistory.query.get(sending.id).status == 'sending'
     assert NotificationHistory.query.get(pending.id).status == 'pending'
     assert NotificationHistory.query.get(delivered.id).status == 'delivered'
-    assert len(updated_ids) == 0
+    assert len(technical_failure_notifications + temporary_failure_notifications) == 0
 
 
 def test_should_return_notifications_excluding_jobs_by_default(sample_template, sample_job, sample_api_key):

tests/app/delivery/test_send_to_providers.py

@@ -13,6 +13,7 @@ from app import mmg_client, firetext_client
 from app.dao import (provider_details_dao, notifications_dao)
 from app.dao.provider_details_dao import dao_switch_sms_provider_to_provider_with_identifier
 from app.delivery import send_to_providers
+from app.exceptions import NotificationTechnicalFailureException
 from app.models import (
     Notification,
     EmailBranding,
@@ -137,9 +138,11 @@ def test_should_not_send_email_message_when_service_is_inactive_notifcation_is_i
     sample_service.active = False
     send_mock = mocker.patch("app.aws_ses_client.send_email", return_value='reference')
 
-    send_to_providers.send_email_to_provider(sample_notification)
+    with pytest.raises(NotificationTechnicalFailureException) as e:
+        send_to_providers.send_email_to_provider(sample_notification)
     send_mock.assert_not_called()
     assert Notification.query.get(sample_notification.id).status == 'technical-failure'
+    assert str(sample_notification.id) in e.value.message
 
 
 @pytest.mark.parametrize("client_send", ["app.mmg_client.send_sms", "app.firetext_client.send_sms"])
@@ -148,9 +151,11 @@ def test_should_not_send_sms_message_when_service_is_inactive_notifcation_is_in_
     sample_service.active = False
     send_mock = mocker.patch(client_send, return_value='reference')
 
-    send_to_providers.send_sms_to_provider(sample_notification)
+    with pytest.raises(NotificationTechnicalFailureException) as e:
+        send_to_providers.send_sms_to_provider(sample_notification)
     send_mock.assert_not_called()
     assert Notification.query.get(sample_notification.id).status == 'technical-failure'
+    assert str(sample_notification.id) in e.value.message
 
 
 def test_send_sms_should_use_template_version_from_notification_not_latest(