Lowering retry period for high priority emails to 25 seconds

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

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,30 +84,24 @@
         _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 as me:
+        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))
-            else:
-                current_app.logger.exception("RETRY: Email notification {} failed".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)
+        else:
+            current_app.logger.exception(
+                "RETRY: Email notification {} failed on pending malware scanning".format(notification_id)
+            )
+        _handle_email_retry(self, notification, me, countdown)
+    except Exception as e:
+        _handle_email_retry(self, notification, e)
 
 
 def _deliver_sms(self, notification_id):
@@ -120,15 +117,8 @@
         _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 {}. "
@@ -137,3 +127,22 @@
             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)

app/notifications/__init__.py

@@ -1,36 +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.
-    if notification_type == SMS_TYPE:
-        params["countdown"] = RETRY_PERIODS[notification_process_type]
-    else:
-        params["countdown"] = RETRY_DEFAULT
-    return params

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)