From 8d4a8f803bfd3a9c7df7e636cbd5ff1eb6e70e05 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Mon, 20 Nov 2023 18:01:11 -0500
Subject: [PATCH 01/22] Decoupled scan malware code + lowering retry period for
 high priority emails

---
 app/celery/provider_tasks.py   | 27 +++++++++++++++++++++------
 app/notifications/__init__.py  |  5 +----
 app/notifications/callbacks.py |  1 -
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index bfdc671a2e..bf20139ad1 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -81,17 +81,16 @@ def deliver_email(self, notification_id):
         _check_and_queue_callback_task(notification)
     except MalwareDetectedException:
         _check_and_queue_callback_task(notification)
-    except Exception as e:
-        if isinstance(e, MalwareScanInProgressException) and self.request.retries <= SCAN_MAX_BACKOFF_RETRIES:
-            countdown = SCAN_RETRY_BACKOFF * (self.request.retries + 1)  # do we need to add 1 here?
+    except MalwareScanInProgressException:
+        if self.request.retries <= SCAN_MAX_BACKOFF_RETRIES:
+            countdown = SCAN_RETRY_BACKOFF * (self.request.retries + 1)
         else:
             countdown = None
         try:
-            current_app.logger.warning(f"The exception is {repr(e)}")
             if self.request.retries <= 10:
-                current_app.logger.warning("RETRY {}: Email notification {} failed".format(self.request.retries, notification_id))
+                current_app.logger.warning("RETRY {}: Email notification {} is waiting on pending malware scanning".format(self.request.retries, notification_id))
             else:
-                current_app.logger.exception("RETRY: Email notification {} failed".format(notification_id))
+                current_app.logger.exception("RETRY: Email notification {} failed on pending malware scanning".format(notification_id))
             if countdown is not None:
                 self.retry(queue=QueueNames.RETRY, countdown=countdown)
             else:
@@ -105,6 +104,22 @@ def deliver_email(self, notification_id):
             update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
             _check_and_queue_callback_task(notification)
             raise NotificationTechnicalFailureException(message)
+    except Exception as e:
+        try:
+            if self.request.retries <= 10:
+                current_app.logger.warning("RETRY {}: Email notification {} failed".format(self.request.retries, notification_id))
+            else:
+                current_app.logger.exception("RETRY: Email notification {} failed".format(notification_id), exc_info=e)
+            self.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type))
+        except self.MaxRetriesExceededError:
+            message = (
+                "RETRY FAILED: Max retries reached. "
+                "The 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)
+            _check_and_queue_callback_task(notification)
+            raise NotificationTechnicalFailureException(message)
 
 
 def _deliver_sms(self, notification_id):
diff --git a/app/notifications/__init__.py b/app/notifications/__init__.py
index 1ec9036155..52a8187eee 100644
--- a/app/notifications/__init__.py
+++ b/app/notifications/__init__.py
@@ -29,8 +29,5 @@ def build_retry_task_params(notification_type: str, notification_process_type: s
 
     # Overring the retry policy is only supported for SMS for now;
     # email support coming later.
-    if notification_type == SMS_TYPE:
-        params["countdown"] = RETRY_PERIODS[notification_process_type]
-    else:
-        params["countdown"] = RETRY_DEFAULT
+    params["countdown"] = RETRY_PERIODS[notification_process_type]
     return params
diff --git a/app/notifications/callbacks.py b/app/notifications/callbacks.py
index 712d1c3edc..709c39f641 100644
--- a/app/notifications/callbacks.py
+++ b/app/notifications/callbacks.py
@@ -10,7 +10,6 @@ def _check_and_queue_callback_task(notification):
     service_callback_api = get_service_delivery_status_callback_api_for_service(service_id=notification.service_id)
     if service_callback_api:
         notification_data = create_delivery_status_callback_data(notification, service_callback_api)
-
         send_delivery_status_to_service.apply_async([str(notification.id), notification_data], queue=QueueNames.CALLBACKS)
 
 

From 8b3ad4e6acd06729eafe76512017ad6bf8ab5536 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 21 Nov 2023 16:51:29 -0500
Subject: [PATCH 02/22] Extract common email retry handling logic into its own
 function

---
 app/celery/__init__.py        | 39 +++++++++++++++++
 app/celery/provider_tasks.py  | 82 ++++++++++++++++-------------------
 app/notifications/__init__.py | 33 --------------
 3 files changed, 77 insertions(+), 77 deletions(-)

diff --git a/app/celery/__init__.py b/app/celery/__init__.py
index e69de29bb2..db8fb95469 100644
--- a/app/celery/__init__.py
+++ b/app/celery/__init__.py
@@ -0,0 +1,39 @@
+from typing import Any, Dict, Optional
+
+from flask import current_app
+
+from app.config import QueueNames
+from app.models import BULK, NORMAL, PRIORITY
+
+# Default retry periods for sending notifications.
+RETRY_DEFAULT = 300
+RETRY_HIGH = 25
+
+RETRY_PERIODS = {
+    BULK: RETRY_DEFAULT,
+    NORMAL: RETRY_DEFAULT,
+    PRIORITY: RETRY_HIGH,
+}
+
+
+def build_retry_task_params(notification_type: str, notification_process_type: str, countdown: Optional[int]) -> Dict[str, Any]:
+    """
+    Build task params for the sending parameter retry tasks.
+
+    If the notification is a high priority SMS, set the retry policy to retry every 25 seconds
+    else fall back to the default retry policy of retrying every 5 minutes.
+
+    Provide an override parameter for cases the calling task wants to override the retry policy.
+    """
+    params: dict[str, Any] = {"queue": QueueNames.RETRY}
+    if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
+        return params
+
+    if countdown is not None:
+        params["countdown"] = countdown
+    else:
+        # Overring the retry policy is only supported for SMS for now;
+        # email support coming later.
+        params["countdown"] = RETRY_PERIODS[notification_process_type]
+
+    return params
diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index bf20139ad1..ce8c772844 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -1,9 +1,12 @@
+from typing import Optional
+
 from flask import current_app
 from notifications_utils.recipients import InvalidEmailError
 from notifications_utils.statsd_decorators import statsd
 from sqlalchemy.orm.exc import NoResultFound
 
 from app import notify_celery
+from app.celery import build_retry_task_params
 from app.config import Config, QueueNames
 from app.dao import notifications_dao
 from app.dao.notifications_dao import update_notification_status_by_id
@@ -14,9 +17,9 @@
     MalwareScanInProgressException,
     NotificationTechnicalFailureException,
 )
-from app.models import NOTIFICATION_TECHNICAL_FAILURE
-from app.notifications import build_retry_task_params
+from app.models import NOTIFICATION_TECHNICAL_FAILURE, Notification
 from app.notifications.callbacks import _check_and_queue_callback_task
+from celery import Task
 
 
 # Celery rate limits are per worker instance and not a global rate limit.
@@ -81,45 +84,24 @@ def deliver_email(self, notification_id):
         _check_and_queue_callback_task(notification)
     except MalwareDetectedException:
         _check_and_queue_callback_task(notification)
-    except MalwareScanInProgressException:
+    except MalwareScanInProgressException as me:
         if self.request.retries <= SCAN_MAX_BACKOFF_RETRIES:
             countdown = SCAN_RETRY_BACKOFF * (self.request.retries + 1)
         else:
             countdown = None
-        try:
-            if self.request.retries <= 10:
-                current_app.logger.warning("RETRY {}: Email notification {} is waiting on pending malware scanning".format(self.request.retries, notification_id))
-            else:
-                current_app.logger.exception("RETRY: Email notification {} failed on pending malware scanning".format(notification_id))
-            if countdown is not None:
-                self.retry(queue=QueueNames.RETRY, countdown=countdown)
-            else:
-                self.retry(queue=QueueNames.RETRY)
-        except self.MaxRetriesExceededError:
-            message = (
-                "RETRY FAILED: Max retries reached. "
-                "The task send_email_to_provider failed for notification {}. "
-                "Notification has been updated to technical-failure".format(notification_id)
+        if self.request.retries <= 10:
+            current_app.logger.warning(
+                "RETRY {}: Email notification {} is waiting on pending malware scanning".format(
+                    self.request.retries, notification_id
+                )
             )
-            update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
-            _check_and_queue_callback_task(notification)
-            raise NotificationTechnicalFailureException(message)
-    except Exception as e:
-        try:
-            if self.request.retries <= 10:
-                current_app.logger.warning("RETRY {}: Email notification {} failed".format(self.request.retries, notification_id))
-            else:
-                current_app.logger.exception("RETRY: Email notification {} failed".format(notification_id), exc_info=e)
-            self.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type))
-        except self.MaxRetriesExceededError:
-            message = (
-                "RETRY FAILED: Max retries reached. "
-                "The task send_email_to_provider failed for notification {}. "
-                "Notification has been updated to technical-failure".format(notification_id)
+        else:
+            current_app.logger.exception(
+                "RETRY: Email notification {} failed on pending malware scanning".format(notification_id)
             )
-            update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
-            _check_and_queue_callback_task(notification)
-            raise NotificationTechnicalFailureException(message)
+        _handle_email_retry(self, notification, me, countdown)
+    except Exception as e:
+        _handle_email_retry(self, notification, e)
 
 
 def _deliver_sms(self, notification_id):
@@ -135,15 +117,8 @@ def _deliver_sms(self, notification_id):
         _check_and_queue_callback_task(notification)
     except Exception:
         try:
-            if self.request.retries == 0:
-                # Retry immediately, especially as a common failure is for the database data
-                # replication to be delayed. The immediate retry likely succeeds in these scenarios.
-                self.retry(queue=QueueNames.RETRY, countdown=0)
-            else:
-                # Once the previous retry failed, log the exception and this time,
-                # retry with the default delay.
-                current_app.logger.exception("SMS notification delivery for id: {} failed".format(notification_id))
-                self.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type))
+            current_app.logger.exception("SMS notification delivery for id: {} failed".format(notification_id))
+            self.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type))
         except self.MaxRetriesExceededError:
             message = (
                 "RETRY FAILED: Max retries reached. The task send_sms_to_provider failed for notification {}. "
@@ -152,3 +127,22 @@ def _deliver_sms(self, notification_id):
             update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
             _check_and_queue_callback_task(notification)
             raise NotificationTechnicalFailureException(message)
+
+
+def _handle_email_retry(task: Task, notification: Notification, e: Exception, countdown: Optional[None] = None):
+    try:
+        if task.request.retries <= 10:
+            current_app.logger.warning("RETRY {}: Email notification {} failed".format(task.request.retries, notification.id))
+        else:
+            current_app.logger.exception("RETRY: Email notification {} failed".format(notification.id), exc_info=e)
+
+        task.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type, countdown))
+    except task.MaxRetriesExceededError:
+        message = (
+            "RETRY FAILED: Max retries reached. "
+            "The 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)
+        _check_and_queue_callback_task(notification)
+        raise NotificationTechnicalFailureException(message)
diff --git a/app/notifications/__init__.py b/app/notifications/__init__.py
index 52a8187eee..e69de29bb2 100644
--- a/app/notifications/__init__.py
+++ b/app/notifications/__init__.py
@@ -1,33 +0,0 @@
-from typing import Any, Dict
-
-from flask import current_app
-
-from app.config import QueueNames
-from app.models import BULK, NORMAL, PRIORITY, SMS_TYPE
-
-# Default retry periods for sending notifications.
-RETRY_DEFAULT = 300
-RETRY_HIGH = 25
-
-RETRY_PERIODS = {
-    BULK: RETRY_DEFAULT,
-    NORMAL: RETRY_DEFAULT,
-    PRIORITY: RETRY_HIGH,
-}
-
-
-def build_retry_task_params(notification_type: str, notification_process_type: str) -> Dict[str, Any]:
-    """
-    Build task params for the sending parameter retry tasks.
-
-    If the notification is a high priority SMS, set the retry policy to retry every 25 seconds
-    else fall back to the default retry policy of retrying every 5 minutes.
-    """
-    params: dict[str, Any] = {"queue": QueueNames.RETRY}
-    if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
-        return params
-
-    # Overring the retry policy is only supported for SMS for now;
-    # email support coming later.
-    params["countdown"] = RETRY_PERIODS[notification_process_type]
-    return params

From 4b40f65d67b7054d3fc549137363367c75002bb6 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 21 Nov 2023 16:57:17 -0500
Subject: [PATCH 03/22] Cleaned up import

---
 app/celery/provider_tasks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index ce8c772844..f5d7ccb554 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -7,7 +7,7 @@
 
 from app import notify_celery
 from app.celery import build_retry_task_params
-from app.config import Config, QueueNames
+from app.config import Config
 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 dab3a1232a08d7f8ba3db2f03c5748a4229739e1 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 21 Nov 2023 16:59:36 -0500
Subject: [PATCH 04/22] Forgot to provide default value to optional fn arg

---
 app/celery/__init__.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/celery/__init__.py b/app/celery/__init__.py
index db8fb95469..5617c394f0 100644
--- a/app/celery/__init__.py
+++ b/app/celery/__init__.py
@@ -16,7 +16,9 @@
 }
 
 
-def build_retry_task_params(notification_type: str, notification_process_type: str, countdown: Optional[int]) -> Dict[str, Any]:
+def build_retry_task_params(
+    notification_type: str, notification_process_type: str, countdown: Optional[int] = None
+) -> Dict[str, Any]:
     """
     Build task params for the sending parameter retry tasks.
 

From 5ec97aef4d77c46d6228de1d390ed3db803ccca0 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 21 Nov 2023 18:03:49 -0500
Subject: [PATCH 05/22] Fixed test import

---
 tests/app/notifications/test_process_notification.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index c9a627439e..6c3e261d6c 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -12,6 +12,7 @@
 )
 from sqlalchemy.exc import SQLAlchemyError
 
+from app.celery import RETRY_PERIODS, build_retry_task_params
 from app.config import QueueNames
 from app.dao.service_sms_sender_dao import dao_update_service_sms_sender
 from app.models import (
@@ -26,7 +27,6 @@
     ScheduledNotification,
     Template,
 )
-from app.notifications import RETRY_PERIODS, build_retry_task_params
 from app.notifications.process_notifications import (
     choose_queue,
     create_content_for_notification,

From a6a418adc067b7e646c7897d7168b1d82b8d55ea Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Wed, 22 Nov 2023 18:11:07 -0500
Subject: [PATCH 06/22] Isolated retry task param builder in a class

---
 app/celery/__init__.py                        | 56 ++++++++++---------
 app/celery/provider_tasks.py                  |  6 +-
 .../test_process_notification.py              | 17 +++---
 3 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/app/celery/__init__.py b/app/celery/__init__.py
index 5617c394f0..35ebcba17a 100644
--- a/app/celery/__init__.py
+++ b/app/celery/__init__.py
@@ -2,40 +2,42 @@
 
 from flask import current_app
 
-from app.config import QueueNames
-from app.models import BULK, NORMAL, PRIORITY
-
 # Default retry periods for sending notifications.
 RETRY_DEFAULT = 300
 RETRY_HIGH = 25
 
-RETRY_PERIODS = {
-    BULK: RETRY_DEFAULT,
-    NORMAL: RETRY_DEFAULT,
-    PRIORITY: RETRY_HIGH,
-}
 
+class CeleryParams(object):
+    # Important to load from the object and not the module to avoid
+    # circular imports, back and forth between the app and celery modules.
+    from app.config import QueueNames
+    from app.models import BULK, NORMAL, PRIORITY
 
-def build_retry_task_params(
-    notification_type: str, notification_process_type: str, countdown: Optional[int] = None
-) -> Dict[str, Any]:
-    """
-    Build task params for the sending parameter retry tasks.
+    RETRY_PERIODS = {
+        BULK: RETRY_DEFAULT,
+        NORMAL: RETRY_DEFAULT,
+        PRIORITY: RETRY_HIGH,
+    }
 
-    If the notification is a high priority SMS, set the retry policy to retry every 25 seconds
-    else fall back to the default retry policy of retrying every 5 minutes.
+    @staticmethod
+    def retry(notification_process_type: str, countdown: Optional[int] = None) -> Dict[str, Any]:
+        """
+        Build task params for the sending parameter retry tasks.
 
-    Provide an override parameter for cases the calling task wants to override the retry policy.
-    """
-    params: dict[str, Any] = {"queue": QueueNames.RETRY}
-    if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
-        return params
+        If the notification is a high priority SMS, set the retry policy to retry every 25 seconds
+        else fall back to the default retry policy of retrying every 5 minutes.
 
-    if countdown is not None:
-        params["countdown"] = countdown
-    else:
-        # Overring the retry policy is only supported for SMS for now;
-        # email support coming later.
-        params["countdown"] = RETRY_PERIODS[notification_process_type]
+        Provide an override parameter for cases the calling task wants to override the retry policy.
+        """
+        params: dict[str, Any] = {"queue": CeleryParams.QueueNames.RETRY}
+        if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
+            return params
 
-    return params
+        if countdown is not None:
+            params["countdown"] = countdown
+        else:
+            # Overring the retry policy is only supported for SMS for now;
+            # email support coming later.
+            params["countdown"] = CeleryParams.RETRY_PERIODS[notification_process_type]
+
+        return params
diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index f5d7ccb554..b9eb12b548 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -6,7 +6,7 @@
 from sqlalchemy.orm.exc import NoResultFound
 
 from app import notify_celery
-from app.celery import build_retry_task_params
+from app.celery import CeleryParams
 from app.config import Config
 from app.dao import notifications_dao
 from app.dao.notifications_dao import update_notification_status_by_id
@@ -118,7 +118,7 @@ def _deliver_sms(self, notification_id):
     except Exception:
         try:
             current_app.logger.exception("SMS notification delivery for id: {} failed".format(notification_id))
-            self.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type))
+            self.retry(**CeleryParams.retry(notification.template.process_type))
         except self.MaxRetriesExceededError:
             message = (
                 "RETRY FAILED: Max retries reached. The task send_sms_to_provider failed for notification {}. "
@@ -136,7 +136,7 @@ def _handle_email_retry(task: Task, notification: Notification, e: Exception, co
         else:
             current_app.logger.exception("RETRY: Email notification {} failed".format(notification.id), exc_info=e)
 
-        task.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type, countdown))
+        task.retry(**CeleryParams.retry(notification.template.process_type, countdown))
     except task.MaxRetriesExceededError:
         message = (
             "RETRY FAILED: Max retries reached. "
diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index 6c3e261d6c..117c1cd0b4 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -12,7 +12,7 @@
 )
 from sqlalchemy.exc import SQLAlchemyError
 
-from app.celery import RETRY_PERIODS, build_retry_task_params
+from app.celery import RETRY_PERIODS, CeleryParams
 from app.config import QueueNames
 from app.dao.service_sms_sender_dao import dao_update_service_sms_sender
 from app.models import (
@@ -1093,19 +1093,16 @@ def test_db_save_and_send_notification_throws_exception_deletes_notification(
         assert NotificationHistory.query.count() == 0
 
     @pytest.mark.parametrize(
-        ("notification_type, process_type, expected_retry_period"),
+        ("process_type, expected_retry_period"),
         [
-            (EMAIL_TYPE, BULK, RETRY_PERIODS[BULK]),
-            (EMAIL_TYPE, NORMAL, RETRY_PERIODS[NORMAL]),
-            (EMAIL_TYPE, PRIORITY, RETRY_PERIODS[NORMAL]),
-            (SMS_TYPE, BULK, RETRY_PERIODS[BULK]),
-            (SMS_TYPE, NORMAL, RETRY_PERIODS[NORMAL]),
-            (SMS_TYPE, PRIORITY, RETRY_PERIODS[PRIORITY]),
+            (BULK, RETRY_PERIODS[BULK]),
+            (NORMAL, RETRY_PERIODS[NORMAL]),
+            (PRIORITY, RETRY_PERIODS[PRIORITY]),
         ],
     )
-    def test_retry_task_parameters(self, notify_api, notification_type, process_type, expected_retry_period):
+    def test_retry_task_parameters(self, notify_api, process_type, expected_retry_period):
         with notify_api.app_context():
-            params: Dict[str, Any] = build_retry_task_params(notification_type, process_type)
+            params = CeleryParams.retry(process_type)
 
         assert params["queue"] == QueueNames.RETRY
         assert params["countdown"] == expected_retry_period

From 1cef267b5929e4bcec3c9a0c5a9266c391c5bfd4 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Wed, 22 Nov 2023 18:19:14 -0500
Subject: [PATCH 07/22] Cleaned up import

---
 tests/app/notifications/test_process_notification.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index 117c1cd0b4..cdf6f30a07 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -1,6 +1,5 @@
 import datetime
 import uuid
-from typing import Any, Dict
 from unittest.mock import call
 
 import pytest
@@ -17,11 +16,9 @@
 from app.dao.service_sms_sender_dao import dao_update_service_sms_sender
 from app.models import (
     BULK,
-    EMAIL_TYPE,
     LETTER_TYPE,
     NORMAL,
     PRIORITY,
-    SMS_TYPE,
     Notification,
     NotificationHistory,
     ScheduledNotification,

From f4a647042e613430e1bcd31147b687644949d71e Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Wed, 22 Nov 2023 18:23:47 -0500
Subject: [PATCH 08/22] Fixed moved refs

---
 tests/app/notifications/test_process_notification.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index cdf6f30a07..a4e440938d 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -11,7 +11,7 @@
 )
 from sqlalchemy.exc import SQLAlchemyError
 
-from app.celery import RETRY_PERIODS, CeleryParams
+from app.celery import CeleryParams
 from app.config import QueueNames
 from app.dao.service_sms_sender_dao import dao_update_service_sms_sender
 from app.models import (
@@ -1092,9 +1092,9 @@ def test_db_save_and_send_notification_throws_exception_deletes_notification(
     @pytest.mark.parametrize(
         ("process_type, expected_retry_period"),
         [
-            (BULK, RETRY_PERIODS[BULK]),
-            (NORMAL, RETRY_PERIODS[NORMAL]),
-            (PRIORITY, RETRY_PERIODS[PRIORITY]),
+            (BULK, CeleryParams.RETRY_PERIODS[BULK]),
+            (NORMAL, CeleryParams.RETRY_PERIODS[NORMAL]),
+            (PRIORITY, CeleryParams.RETRY_PERIODS[PRIORITY]),
         ],
     )
     def test_retry_task_parameters(self, notify_api, process_type, expected_retry_period):

From 17482f5d6e46c5c811e6f3415e679dd3b5295647 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Wed, 22 Nov 2023 18:50:33 -0500
Subject: [PATCH 09/22] Trying a different strategy to fix circular import

---
 app/celery/__init__.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/celery/__init__.py b/app/celery/__init__.py
index 35ebcba17a..81c4a3dc9f 100644
--- a/app/celery/__init__.py
+++ b/app/celery/__init__.py
@@ -2,6 +2,8 @@
 
 from flask import current_app
 
+from app import config, models
+
 # Default retry periods for sending notifications.
 RETRY_DEFAULT = 300
 RETRY_HIGH = 25
@@ -10,13 +12,11 @@
 class CeleryParams(object):
     # Important to load from the object and not the module to avoid
     # circular imports, back and forth between the app and celery modules.
-    from app.config import QueueNames
-    from app.models import BULK, NORMAL, PRIORITY
 
     RETRY_PERIODS = {
-        BULK: RETRY_DEFAULT,
-        NORMAL: RETRY_DEFAULT,
-        PRIORITY: RETRY_HIGH,
+        models.BULK: RETRY_DEFAULT,
+        models.NORMAL: RETRY_DEFAULT,
+        models.PRIORITY: RETRY_HIGH,
     }
 
     @staticmethod
@@ -29,7 +29,7 @@ def retry(notification_process_type: str, countdown: Optional[int] = None) -> Di
 
         Provide an override parameter for cases the calling task wants to override the retry policy.
         """
-        params: dict[str, Any] = {"queue": CeleryParams.QueueNames.RETRY}
+        params: dict[str, Any] = {"queue": config.RETRY}
         if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
             return params
 

From e124f213baf263ff8f743d3fd27f30f37fa53a2a Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Wed, 22 Nov 2023 19:18:06 -0500
Subject: [PATCH 10/22] Fixing another bad import ref

---
 app/celery/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/celery/__init__.py b/app/celery/__init__.py
index 81c4a3dc9f..1d2d389c85 100644
--- a/app/celery/__init__.py
+++ b/app/celery/__init__.py
@@ -29,7 +29,7 @@ def retry(notification_process_type: str, countdown: Optional[int] = None) -> Di
 
         Provide an override parameter for cases the calling task wants to override the retry policy.
         """
-        params: dict[str, Any] = {"queue": config.RETRY}
+        params: dict[str, Any] = {"queue": config.QueueNames.RETRY}
         if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
             return params
 

From e5bfea8f4dc0f734d255cd6c3219a05ca1b482d6 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Thu, 23 Nov 2023 17:20:23 -0500
Subject: [PATCH 11/22] Introducing celery utils module instead of using celery
 root one

---
 app/celery/__init__.py                        | 43 -------------------
 app/celery/provider_tasks.py                  |  2 +-
 app/celery/utils.py                           | 43 +++++++++++++++++++
 .../test_process_notification.py              |  2 +-
 4 files changed, 45 insertions(+), 45 deletions(-)
 create mode 100644 app/celery/utils.py

diff --git a/app/celery/__init__.py b/app/celery/__init__.py
index 1d2d389c85..e69de29bb2 100644
--- a/app/celery/__init__.py
+++ b/app/celery/__init__.py
@@ -1,43 +0,0 @@
-from typing import Any, Dict, Optional
-
-from flask import current_app
-
-from app import config, models
-
-# Default retry periods for sending notifications.
-RETRY_DEFAULT = 300
-RETRY_HIGH = 25
-
-
-class CeleryParams(object):
-    # Important to load from the object and not the module to avoid
-    # circular imports, back and forth between the app and celery modules.
-
-    RETRY_PERIODS = {
-        models.BULK: RETRY_DEFAULT,
-        models.NORMAL: RETRY_DEFAULT,
-        models.PRIORITY: RETRY_HIGH,
-    }
-
-    @staticmethod
-    def retry(notification_process_type: str, countdown: Optional[int] = None) -> Dict[str, Any]:
-        """
-        Build task params for the sending parameter retry tasks.
-
-        If the notification is a high priority SMS, set the retry policy to retry every 25 seconds
-        else fall back to the default retry policy of retrying every 5 minutes.
-
-        Provide an override parameter for cases the calling task wants to override the retry policy.
-        """
-        params: dict[str, Any] = {"queue": config.QueueNames.RETRY}
-        if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
-            return params
-
-        if countdown is not None:
-            params["countdown"] = countdown
-        else:
-            # Overring the retry policy is only supported for SMS for now;
-            # email support coming later.
-            params["countdown"] = CeleryParams.RETRY_PERIODS[notification_process_type]
-
-        return params
diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index b9eb12b548..4c977cc285 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -6,7 +6,7 @@
 from sqlalchemy.orm.exc import NoResultFound
 
 from app import notify_celery
-from app.celery import CeleryParams
+from app.celery.utils import CeleryParams
 from app.config import Config
 from app.dao import notifications_dao
 from app.dao.notifications_dao import update_notification_status_by_id
diff --git a/app/celery/utils.py b/app/celery/utils.py
new file mode 100644
index 0000000000..1d2d389c85
--- /dev/null
+++ b/app/celery/utils.py
@@ -0,0 +1,43 @@
+from typing import Any, Dict, Optional
+
+from flask import current_app
+
+from app import config, models
+
+# Default retry periods for sending notifications.
+RETRY_DEFAULT = 300
+RETRY_HIGH = 25
+
+
+class CeleryParams(object):
+    # Important to load from the object and not the module to avoid
+    # circular imports, back and forth between the app and celery modules.
+
+    RETRY_PERIODS = {
+        models.BULK: RETRY_DEFAULT,
+        models.NORMAL: RETRY_DEFAULT,
+        models.PRIORITY: RETRY_HIGH,
+    }
+
+    @staticmethod
+    def retry(notification_process_type: str, countdown: Optional[int] = None) -> Dict[str, Any]:
+        """
+        Build task params for the sending parameter retry tasks.
+
+        If the notification is a high priority SMS, set the retry policy to retry every 25 seconds
+        else fall back to the default retry policy of retrying every 5 minutes.
+
+        Provide an override parameter for cases the calling task wants to override the retry policy.
+        """
+        params: dict[str, Any] = {"queue": config.QueueNames.RETRY}
+        if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
+            return params
+
+        if countdown is not None:
+            params["countdown"] = countdown
+        else:
+            # Overring the retry policy is only supported for SMS for now;
+            # email support coming later.
+            params["countdown"] = CeleryParams.RETRY_PERIODS[notification_process_type]
+
+        return params
diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index a4e440938d..933d3d27d3 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -11,7 +11,7 @@
 )
 from sqlalchemy.exc import SQLAlchemyError
 
-from app.celery import CeleryParams
+from app.celery.utils import CeleryParams
 from app.config import QueueNames
 from app.dao.service_sms_sender_dao import dao_update_service_sms_sender
 from app.models import (

From ec417937d6469813e8603433f373b6194273c4f5 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Fri, 24 Nov 2023 14:40:40 -0500
Subject: [PATCH 12/22] Cover edge cases + modified tests

---
 app/celery/provider_tasks.py            | 35 +++++++++++--------------
 app/celery/utils.py                     |  4 +--
 app/notifications/callbacks.py          |  4 +++
 tests/app/celery/test_provider_tasks.py |  4 +--
 4 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index 4c977cc285..50a888ffcc 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -89,19 +89,12 @@ def deliver_email(self, notification_id):
             countdown = SCAN_RETRY_BACKOFF * (self.request.retries + 1)
         else:
             countdown = None
-        if self.request.retries <= 10:
-            current_app.logger.warning(
-                "RETRY {}: Email notification {} is waiting on pending malware scanning".format(
-                    self.request.retries, notification_id
-                )
-            )
-        else:
-            current_app.logger.exception(
-                "RETRY: Email notification {} failed on pending malware scanning".format(notification_id)
-            )
-        _handle_email_retry(self, notification, me, countdown)
+        current_app.logger.warning(
+            "RETRY {}: Email notification {} is waiting on pending malware scanning".format(self.request.retries, notification_id)
+        )
+        _handle_error_with_email_retry(self, notification_id, notification, me, countdown)
     except Exception as e:
-        _handle_email_retry(self, notification, e)
+        _handle_error_with_email_retry(self, notification_id, notification, e)
 
 
 def _deliver_sms(self, notification_id):
@@ -129,20 +122,22 @@ def _deliver_sms(self, notification_id):
             raise NotificationTechnicalFailureException(message)
 
 
-def _handle_email_retry(task: Task, notification: Notification, e: Exception, countdown: Optional[None] = None):
+def _handle_error_with_email_retry(task: Task, e: Exception, notification_id: int, notification: Optional[Notification], countdown: Optional[None] = None):
     try:
         if task.request.retries <= 10:
-            current_app.logger.warning("RETRY {}: Email notification {} failed".format(task.request.retries, notification.id))
+            current_app.logger.warning("RETRY {}: Email notification {} failed".format(task.request.retries, notification_id))
         else:
-            current_app.logger.exception("RETRY: Email notification {} failed".format(notification.id), exc_info=e)
-
-        task.retry(**CeleryParams.retry(notification.template.process_type, countdown))
+            current_app.logger.exception("RETRY: Email notification {} failed".format(notification_id), exc_info=e)
+        # There is an edge case when a notification is not found in the database.
+        if notification is None:
+            task.retry(**CeleryParams.retry(countdown=countdown))
+        else:
+            task.retry(**CeleryParams.retry(notification.template.process_type, countdown))
     except task.MaxRetriesExceededError:
         message = (
             "RETRY FAILED: Max retries reached. "
             "The task send_email_to_provider failed for notification {}. "
-            "Notification has been updated to technical-failure".format(notification.id)
+            "Notification has been updated to technical-failure".format(notification_id)
         )
-        update_notification_status_by_id(notification.id, NOTIFICATION_TECHNICAL_FAILURE)
-        _check_and_queue_callback_task(notification)
+        update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
         raise NotificationTechnicalFailureException(message)
diff --git a/app/celery/utils.py b/app/celery/utils.py
index 1d2d389c85..b5c841423c 100644
--- a/app/celery/utils.py
+++ b/app/celery/utils.py
@@ -17,10 +17,11 @@ class CeleryParams(object):
         models.BULK: RETRY_DEFAULT,
         models.NORMAL: RETRY_DEFAULT,
         models.PRIORITY: RETRY_HIGH,
+        None : RETRY_HIGH,  # In case we cannot identify the priority, treat it as high.
     }
 
     @staticmethod
-    def retry(notification_process_type: str, countdown: Optional[int] = None) -> Dict[str, Any]:
+    def retry(notification_process_type: Optional[str], countdown: Optional[int] = None) -> Dict[str, Any]:
         """
         Build task params for the sending parameter retry tasks.
 
@@ -39,5 +40,4 @@ def retry(notification_process_type: str, countdown: Optional[int] = None) -> Di
             # Overring the retry policy is only supported for SMS for now;
             # email support coming later.
             params["countdown"] = CeleryParams.RETRY_PERIODS[notification_process_type]
-
         return params
diff --git a/app/notifications/callbacks.py b/app/notifications/callbacks.py
index 709c39f641..bad15276b1 100644
--- a/app/notifications/callbacks.py
+++ b/app/notifications/callbacks.py
@@ -3,9 +3,13 @@
 from app.dao.service_callback_api_dao import (
     get_service_delivery_status_callback_api_for_service,
 )
+from flask import current_app
 
 
 def _check_and_queue_callback_task(notification):
+    if notification is None:
+        current_app.logger.warning("No notification provided, cannot queue callback task")
+        return
     # queue callback task only if the service_callback_api exists
     service_callback_api = get_service_delivery_status_callback_api_for_service(service_id=notification.service_id)
     if service_callback_api:
diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py
index aae4707941..8385aecaa3 100644
--- a/tests/app/celery/test_provider_tasks.py
+++ b/tests/app/celery/test_provider_tasks.py
@@ -114,7 +114,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task(
         sms_method(sample_notification.id)
     assert str(sample_notification.id) in str(e.value)
 
-    getattr(provider_tasks, sms_method_name).retry.assert_called_with(queue="retry-tasks", countdown=0)
+    getattr(provider_tasks, sms_method_name).retry.assert_called_with(queue="retry-tasks", countdown=300)
 
     assert sample_notification.status == "technical-failure"
     queued_callback.assert_called_once_with(sample_notification)
@@ -135,7 +135,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task
         deliver_email(sample_notification.id)
     assert str(sample_notification.id) in str(e.value)
 
-    provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks")
+    provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks", countdown=300)
     assert sample_notification.status == "technical-failure"
     queued_callback.assert_called_once_with(sample_notification)
 

From 5433518285dec326d79905b6559659acea9cba1c Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Fri, 24 Nov 2023 14:48:02 -0500
Subject: [PATCH 13/22] Formatting

---
 app/celery/provider_tasks.py | 4 +++-
 app/celery/utils.py          | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index 50a888ffcc..abdc54db38 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -122,7 +122,9 @@ def _deliver_sms(self, notification_id):
             raise NotificationTechnicalFailureException(message)
 
 
-def _handle_error_with_email_retry(task: Task, e: Exception, notification_id: int, notification: Optional[Notification], countdown: Optional[None] = None):
+def _handle_error_with_email_retry(
+    task: Task, e: Exception, notification_id: int, notification: Optional[Notification], countdown: Optional[None] = None
+):
     try:
         if task.request.retries <= 10:
             current_app.logger.warning("RETRY {}: Email notification {} failed".format(task.request.retries, notification_id))
diff --git a/app/celery/utils.py b/app/celery/utils.py
index b5c841423c..854313af96 100644
--- a/app/celery/utils.py
+++ b/app/celery/utils.py
@@ -17,7 +17,7 @@ class CeleryParams(object):
         models.BULK: RETRY_DEFAULT,
         models.NORMAL: RETRY_DEFAULT,
         models.PRIORITY: RETRY_HIGH,
-        None : RETRY_HIGH,  # In case we cannot identify the priority, treat it as high.
+        None: RETRY_HIGH,  # In case we cannot identify the priority, treat it as high.
     }
 
     @staticmethod

From 85f5140904b07bd5767266c9a6f468311bbc01b9 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Fri, 24 Nov 2023 15:00:55 -0500
Subject: [PATCH 14/22] Sort imports

---
 app/notifications/callbacks.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/notifications/callbacks.py b/app/notifications/callbacks.py
index bad15276b1..3cc6d0bb91 100644
--- a/app/notifications/callbacks.py
+++ b/app/notifications/callbacks.py
@@ -1,9 +1,10 @@
+from flask import current_app
+
 from app.celery.service_callback_tasks import send_delivery_status_to_service
 from app.config import QueueNames
 from app.dao.service_callback_api_dao import (
     get_service_delivery_status_callback_api_for_service,
 )
-from flask import current_app
 
 
 def _check_and_queue_callback_task(notification):

From 9e4bfe6803e008c744123c0ecf3dcb3837df4db3 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Fri, 24 Nov 2023 16:42:56 -0500
Subject: [PATCH 15/22] Make notification_process_type param optional

---
 app/celery/utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/celery/utils.py b/app/celery/utils.py
index 854313af96..f849e8f02a 100644
--- a/app/celery/utils.py
+++ b/app/celery/utils.py
@@ -21,7 +21,7 @@ class CeleryParams(object):
     }
 
     @staticmethod
-    def retry(notification_process_type: Optional[str], countdown: Optional[int] = None) -> Dict[str, Any]:
+    def retry(notification_process_type: Optional[str] = None, countdown: Optional[int] = None) -> Dict[str, Any]:
         """
         Build task params for the sending parameter retry tasks.
 

From c3e5ee6e4ec4e94cf2e14a7b7835d74760a15999 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Thu, 7 Dec 2023 17:44:32 -0500
Subject: [PATCH 16/22] Fixed edge case when template not associated with
 notification obj

---
 app/celery/provider_tasks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index da18e5905a..b6ff5332d2 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -131,7 +131,7 @@ def _handle_error_with_email_retry(
         else:
             current_app.logger.exception("RETRY: Email notification {} failed".format(notification_id), exc_info=e)
         # There is an edge case when a notification is not found in the database.
-        if notification is None:
+        if notification is None or notification.template is None:
             task.retry(**CeleryParams.retry(countdown=countdown))
         else:
             task.retry(**CeleryParams.retry(notification.template.process_type, countdown))

From bcd3f160575235a13c1b259f1591836549f21c74 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Mon, 11 Dec 2023 15:04:26 -0500
Subject: [PATCH 17/22] Fixing params order

---
 app/celery/provider_tasks.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index b6ff5332d2..aef1560623 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -92,9 +92,9 @@ def deliver_email(self, notification_id):
         current_app.logger.warning(
             "RETRY {}: Email notification {} is waiting on pending malware scanning".format(self.request.retries, notification_id)
         )
-        _handle_error_with_email_retry(self, notification_id, notification, me, countdown)
+        _handle_error_with_email_retry(self, me, notification_id, notification, countdown)
     except Exception as e:
-        _handle_error_with_email_retry(self, notification_id, notification, e)
+        _handle_error_with_email_retry(self, e, notification_id, notification)
 
 
 def _deliver_sms(self, notification_id):

From 4e236d4f7c42149bacc192af4746af2f074a6b48 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Mon, 11 Dec 2023 18:04:20 -0500
Subject: [PATCH 18/22] Fixing regression tests

---
 app/celery/provider_tasks.py            | 4 +++-
 tests/app/celery/test_provider_tasks.py | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index aef1560623..5401526394 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -65,6 +65,7 @@ def deliver_sms(self, notification_id):
 @notify_celery.task(bind=True, name="deliver_email", max_retries=48, default_retry_delay=300)
 @statsd(namespace="tasks")
 def deliver_email(self, notification_id):
+    notification = None
     try:
         current_app.logger.debug("Start sending email for notification id: {}".format(notification_id))
         notification = notifications_dao.get_notification_by_id(notification_id)
@@ -111,7 +112,7 @@ def _deliver_sms(self, notification_id):
     except Exception:
         try:
             current_app.logger.exception("SMS notification delivery for id: {} failed".format(notification_id))
-            self.retry(**CeleryParams.retry(notification.template.process_type))
+            self.retry(**CeleryParams.retry(None if notification is None else notification.template.process_type))
         except self.MaxRetriesExceededError:
             message = (
                 "RETRY FAILED: Max retries reached. The task send_sms_to_provider failed for notification {}. "
@@ -142,4 +143,5 @@ def _handle_error_with_email_retry(
             "Notification has been updated to technical-failure".format(notification_id)
         )
         update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
+        _check_and_queue_callback_task(notification)
         raise NotificationTechnicalFailureException(message)
diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py
index 8385aecaa3..e79d8176d3 100644
--- a/tests/app/celery/test_provider_tasks.py
+++ b/tests/app/celery/test_provider_tasks.py
@@ -64,7 +64,7 @@ def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_sms_task
     sms_method(notification_id)
     app.delivery.send_to_providers.send_sms_to_provider.assert_not_called()
 
-    getattr(app.celery.provider_tasks, sms_method_name).retry.assert_called_with(queue="retry-tasks", countdown=0)
+    getattr(app.celery.provider_tasks, sms_method_name).retry.assert_called_with(queue="retry-tasks", countdown=25)
 
 
 def test_should_call_send_email_to_provider_from_deliver_email_task(
@@ -87,7 +87,7 @@ def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_email_ta
 
     deliver_email(notification_id)
     app.delivery.send_to_providers.send_email_to_provider.assert_not_called()
-    app.celery.provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks")
+    app.celery.provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks", countdown=25)
 
 
 # DO THESE FOR THE 4 TYPES OF TASK

From 7264ae878acc9fba5452435bdb712a26f4f83eab Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 12 Dec 2023 14:19:30 -0500
Subject: [PATCH 19/22] More tests

---
 .../test_process_notification.py              | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index 933d3d27d3..4230e7ed1d 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -39,6 +39,7 @@
 from tests.app.conftest import create_sample_api_key
 from tests.app.db import create_service, create_service_sms_sender, create_template
 
+from tests.conftest import set_config
 
 class TestContentCreation:
     def test_create_content_for_notification_passes(self, sample_email_template):
@@ -1104,6 +1105,38 @@ def test_retry_task_parameters(self, notify_api, process_type, expected_retry_pe
         assert params["queue"] == QueueNames.RETRY
         assert params["countdown"] == expected_retry_period
 
+    @pytest.mark.parametrize(
+        ("process_type"),
+        [
+            (BULK),
+            (NORMAL),
+            (PRIORITY),
+            (None)
+        ],
+    )
+    def test_retry_task_parameters_with_countdown_override(self, notify_api, process_type):
+        with notify_api.app_context():
+            params = CeleryParams.retry(process_type, countdown=-1)
+
+        assert params["queue"] == QueueNames.RETRY
+        assert params["countdown"] == -1
+
+    @pytest.mark.parametrize(
+        ("process_type, expected_retry_period"),
+        [
+            (BULK, CeleryParams.RETRY_PERIODS[BULK]),
+            (NORMAL, CeleryParams.RETRY_PERIODS[NORMAL]),
+            (PRIORITY, CeleryParams.RETRY_PERIODS[PRIORITY]),
+            (None, CeleryParams.RETRY_PERIODS[PRIORITY])
+        ],
+    )
+    def test_retry_task_parameters_with_ff_off(self, notify_api, process_type, expected_retry_period):
+        with notify_api.app_context(), set_config(notify_api, "FF_CELERY_CUSTOM_TASK_PARAMS", False):
+            params = CeleryParams.retry(process_type)
+
+        assert params["queue"] == QueueNames.RETRY
+        assert params.get("countdown") is None
+
     def test_db_save_and_send_notification_throws_exception_when_missing_template(self, sample_api_key, mocker):
         mocker.patch("app.celery.provider_tasks.deliver_sms.apply_async")
         assert Notification.query.count() == 0

From cc991f23a998c0f3d030282b979ceafcdf3f6734 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 12 Dec 2023 17:17:07 -0500
Subject: [PATCH 20/22] Added null protection against a potential NPE

---
 app/celery/provider_tasks.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py
index 5401526394..0539bd6ce1 100644
--- a/app/celery/provider_tasks.py
+++ b/app/celery/provider_tasks.py
@@ -143,5 +143,6 @@ def _handle_error_with_email_retry(
             "Notification has been updated to technical-failure".format(notification_id)
         )
         update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE)
-        _check_and_queue_callback_task(notification)
+        if notification is not None:
+            _check_and_queue_callback_task(notification)
         raise NotificationTechnicalFailureException(message)

From 5bf854b6cb52ed8cf0c474dc032d2b8f2a98a148 Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 12 Dec 2023 17:23:13 -0500
Subject: [PATCH 21/22] Formatting

---
 tests/app/notifications/test_process_notification.py | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index 4230e7ed1d..083ef32fc5 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -41,6 +41,7 @@
 
 from tests.conftest import set_config
 
+
 class TestContentCreation:
     def test_create_content_for_notification_passes(self, sample_email_template):
         template = Template.query.get(sample_email_template.id)
@@ -1107,12 +1108,7 @@ def test_retry_task_parameters(self, notify_api, process_type, expected_retry_pe
 
     @pytest.mark.parametrize(
         ("process_type"),
-        [
-            (BULK),
-            (NORMAL),
-            (PRIORITY),
-            (None)
-        ],
+        [(BULK), (NORMAL), (PRIORITY), (None)],
     )
     def test_retry_task_parameters_with_countdown_override(self, notify_api, process_type):
         with notify_api.app_context():
@@ -1127,7 +1123,7 @@ def test_retry_task_parameters_with_countdown_override(self, notify_api, process
             (BULK, CeleryParams.RETRY_PERIODS[BULK]),
             (NORMAL, CeleryParams.RETRY_PERIODS[NORMAL]),
             (PRIORITY, CeleryParams.RETRY_PERIODS[PRIORITY]),
-            (None, CeleryParams.RETRY_PERIODS[PRIORITY])
+            (None, CeleryParams.RETRY_PERIODS[PRIORITY]),
         ],
     )
     def test_retry_task_parameters_with_ff_off(self, notify_api, process_type, expected_retry_period):

From 2ea5a4bf8cede0da35f10ae7beed267e0813e68f Mon Sep 17 00:00:00 2001
From: Jimmy Royer <jimleroyer@gmail.com>
Date: Tue, 12 Dec 2023 17:43:26 -0500
Subject: [PATCH 22/22] Fix imports

---
 tests/app/notifications/test_process_notification.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index 083ef32fc5..bb194d3c28 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -38,7 +38,6 @@
 from app.v2.errors import BadRequestError
 from tests.app.conftest import create_sample_api_key
 from tests.app.db import create_service, create_service_sms_sender, create_template
-
 from tests.conftest import set_config