1800 Delivery Status Callbacks (revision 1)

app/callback/queue_callback_strategy.py

@@ -28,6 +28,6 @@ def send_callback(
         except ClientError as e:
             statsd_client.incr(f'callback.queue.{callback.callback_type}.non_retryable_error')
             raise NonRetryableException(e)
-        else:
-            current_app.logger.info(f'Callback sent to {callback.url}, {tags}')
-            statsd_client.incr(f'callback.queue.{callback.callback_type}.success')
+
+        current_app.logger.info('Callback sent to %s, %s', callback.url, tags)
+        statsd_client.incr(f'callback.queue.{callback.callback_type}.success')

app/celery/contact_information_tasks.py

@@ -30,19 +30,18 @@ def lookup_contact_info(
     current_app.logger.info('Looking up contact information for notification_id: %s.', notification_id)
 
     notification = get_notification_by_id(notification_id)
-    va_profile_id = notification.recipient_identifiers[IdentifierType.VA_PROFILE_ID.value]
+    recipient_identifier = notification.recipient_identifiers[IdentifierType.VA_PROFILE_ID.value]
 
     try:
         if EMAIL_TYPE == notification.notification_type:
-            recipient = va_profile_client.get_email(va_profile_id)
+            recipient = va_profile_client.get_email(recipient_identifier)
         elif SMS_TYPE == notification.notification_type:
-            recipient = va_profile_client.get_telephone(va_profile_id)
+            recipient = va_profile_client.get_telephone(recipient_identifier)
         else:
             raise NotImplementedError(
                 f'The task lookup_contact_info failed for notification {notification_id}. '
                 f'{notification.notification_type} is not supported'
             )
-
     except (Timeout, VAProfileRetryableException) as e:
         if can_retry(self.request.retries, self.max_retries, notification_id):
             current_app.logger.warning('Unable to get contact info for notification id: %s, retrying', notification_id)
@@ -63,7 +62,6 @@ def lookup_contact_info(
         )
         check_and_queue_callback_task(notification)
         raise NotificationPermanentFailureException(message) from e
-
     except (VAProfileIDNotFoundException, VAProfileNonRetryableException) as e:
         current_app.logger.exception(e)
         message = (
@@ -76,6 +74,5 @@ def lookup_contact_info(
         check_and_queue_callback_task(notification)
         raise NotificationPermanentFailureException(message) from e
 
-    else:
-        notification.to = recipient
-        dao_update_notification(notification)
+    notification.to = recipient
+    dao_update_notification(notification)

app/celery/lookup_recipient_communication_permissions_task.py

@@ -7,9 +7,10 @@
 from app import notify_celery, va_profile_client
 from app.celery.common import can_retry, handle_max_retries_exceeded
 from app.celery.exceptions import AutoRetryException
+from app.celery.service_callback_tasks import check_and_queue_callback_task
 from app.dao.communication_item_dao import get_communication_item
 from app.dao.notifications_dao import get_notification_by_id, update_notification_status_by_id
-from app.exceptions import NotificationTechnicalFailureException
+from app.exceptions import NotificationTechnicalFailureException, NotificationPermanentFailureException
 from app.models import RecipientIdentifier, NOTIFICATION_PREFERENCES_DECLINED
 from app.va.va_profile import VAProfileRetryableException
 from app.va.va_profile.exceptions import CommunicationItemNotFoundException
@@ -30,44 +31,52 @@ def lookup_recipient_communication_permissions(
     self,
     notification_id: str,
 ) -> None:
-    current_app.logger.info(f'Looking up communication preferences for notification_id:{notification_id}')
+    current_app.logger.info('Looking up communication preferences for notification_id: %s', notification_id)
 
     notification = get_notification_by_id(notification_id)
 
     try:
-        notification.recipient_identifiers[IdentifierType.VA_PROFILE_ID.value]
+        va_profile_recipient_identifier = notification.recipient_identifiers[IdentifierType.VA_PROFILE_ID.value]
     except KeyError as e:
-        current_app.logger.info(f'{VAProfileIdNotFoundException.failure_reason} on notification ' f'{notification_id}')
-        raise VAProfileIdNotFoundException from e
-
-    va_profile_recipient_identifier = notification.recipient_identifiers[IdentifierType.VA_PROFILE_ID.value]
+        current_app.logger.info('No VA Profile ID for notification %s.', notification_id)
+        raise VAProfileIdNotFoundException(f'No VA Profile ID for notification {notification_id}.') from e
 
     va_profile_id = va_profile_recipient_identifier.id_value
     communication_item_id = notification.template.communication_item_id
     notification_type = notification.notification_type
 
-    status_reason = recipient_has_given_permission(
-        self,
-        IdentifierType.VA_PROFILE_ID.value,
-        va_profile_id,
-        notification_id,
-        notification_type,
-        communication_item_id,
-    )
+    try:
+        status_reason = recipient_has_given_permission(
+            self,
+            IdentifierType.VA_PROFILE_ID.value,
+            va_profile_id,
+            notification_id,
+            notification_type,
+            communication_item_id,
+        )
+    except NotificationTechnicalFailureException:
+        check_and_queue_callback_task(notification)
+        raise
 
     if status_reason is not None:
+        # The recipient doesn't grant permission.  a.k.a. preferences-declined
+
         update_notification_status_by_id(
             notification_id, NOTIFICATION_PREFERENCES_DECLINED, status_reason=status_reason
         )
-        current_app.logger.info(
-            f'Recipient for notification {notification_id}' f'has declined permission to receive notifications'
-        )
-        self.request.chain = None
+        message = f'The recipient for notification {notification_id} has declined permission to receive notifications.'
+        current_app.logger.info(message)
+        check_and_queue_callback_task(notification)
+        raise NotificationPermanentFailureException(message)
 
 
 def recipient_has_given_permission(
     task, id_type: str, id_value: str, notification_id: str, notification_type: str, communication_item_id: str
 ) -> Optional[str]:
+    """
+    Return None if the recipient has the permissions.  Otherwise, return a string to explain the lack of permission.
+    """
+
     default_send_flag = True
     communication_item = None
     identifier = RecipientIdentifier(id_type=id_type, id_value=id_value)

app/celery/onsite_notification_tasks.py

@@ -9,7 +9,10 @@ def send_va_onsite_notification_task(
     template_id: str,
     onsite_enabled: bool = False,
 ):
-    """This function is used by celery to POST a notification to VA_Onsite."""
+    """
+    POST a notification to VA_Onsite.
+    """
+
     current_app.logger.info(
         'Calling va_onsite_notification_task with va_profile_id: %s\ntemplate_id: %s\nonsite_notification set to: %s',
         va_profile_id,
@@ -19,4 +22,6 @@ def send_va_onsite_notification_task(
 
     if onsite_enabled and va_profile_id:
         data = {'onsite_notification': {'template_id': template_id, 'va_profile_id': va_profile_id}}
+
+        # This method catches exceptions.  It should never disrupt execution of the Celery task chain.
         va_onsite_client.post_onsite_notification(data)

app/celery/provider_tasks.py

@@ -6,13 +6,19 @@
     log_and_update_technical_failure,
 )
 from app.celery.exceptions import NonRetryableException, AutoRetryException
+from app.celery.service_callback_tasks import check_and_queue_callback_task
 from app.clients.email.aws_ses import AwsSesClientThrottlingSendRateException
 from app.config import QueueNames
 from app.dao import notifications_dao
 from app.dao.notifications_dao import update_notification_status_by_id
 from app.dao.service_sms_sender_dao import dao_get_service_sms_sender_by_service_id_and_number
 from app.delivery import send_to_providers
-from app.exceptions import NotificationTechnicalFailureException, MalwarePendingException, InvalidProviderException
+from app.exceptions import (
+    MalwarePendingException,
+    NotificationPermanentFailureException,
+    NotificationTechnicalFailureException,
+    InvalidProviderException,
+)
 from app.models import NOTIFICATION_TECHNICAL_FAILURE
 from app.v2.errors import RateLimitError
 from flask import current_app
@@ -37,8 +43,9 @@ def deliver_sms(
     notification_id,
     sms_sender_id=None,
 ):
+    current_app.logger.info('Start sending SMS for notification id: %s', notification_id)
+
     try:
-        current_app.logger.info('Start sending SMS for notification id: %s', notification_id)
         notification = notifications_dao.get_notification_by_id(notification_id)
         if not notification:
             # Distributed computing race condition
@@ -57,14 +64,15 @@ def deliver_sms(
             e,
             'SMS provider configuration invalid',
         )
-        raise NotificationTechnicalFailureException(str(e))
+        raise NotificationTechnicalFailureException from e
     except InvalidPhoneError as e:
         log_and_update_permanent_failure(
             notification.id,
             'deliver_sms',
             e,
             'Phone number is invalid',
         )
+        raise NotificationTechnicalFailureException from e
     except NonRetryableException as e:
         # Max retries exceeded, celery raised exception
         log_and_update_permanent_failure(
@@ -73,6 +81,7 @@ def deliver_sms(
             e,
             'ERROR: NonRetryableException - permanent failure, not retrying',
         )
+        raise NotificationPermanentFailureException from e
     except (NullValueForNonConditionalPlaceholderException, AttributeError, RuntimeError) as e:
         log_and_update_technical_failure(notification_id, 'deliver_sms', e)
         raise NotificationTechnicalFailureException(f'Found {type(e).__name__}, NOT retrying...', e, e.args)
@@ -83,6 +92,7 @@ def deliver_sms(
             raise AutoRetryException(f'Found {type(e).__name__}, autoretrying...', e, e.args)
         else:
             msg = handle_max_retries_exceeded(notification_id, 'deliver_sms')
+            check_and_queue_callback_task(notification)
             raise NotificationTechnicalFailureException(msg)
 
 
@@ -104,8 +114,9 @@ def deliver_sms_with_rate_limiting(
 ):
     from app.notifications.validators import check_sms_sender_over_rate_limit
 
+    current_app.logger.info('Start sending SMS with rate limiting for notification id: %s', notification_id)
+
     try:
-        current_app.logger.info('Start sending SMS with rate limiting for notification id: %s', notification_id)
         notification = notifications_dao.get_notification_by_id(notification_id)
         if not notification:
             current_app.logger.warning('Notification not found for: %s, retrying', notification_id)
@@ -135,6 +146,7 @@ def deliver_sms_with_rate_limiting(
             e,
             'Phone number is invalid',
         )
+        raise NotificationPermanentFailureException from e
     except NonRetryableException as e:
         # Max retries exceeded, celery raised exception
         log_and_update_permanent_failure(
@@ -143,6 +155,7 @@ def deliver_sms_with_rate_limiting(
             e,
             'ERROR: NonRetryableException - permanent failure, not retrying',
         )
+        raise NotificationTechnicalFailureException from e
     except RateLimitError:
         retry_time = sms_sender.rate_limit_interval / sms_sender.rate_limit
         current_app.logger.info(
@@ -165,6 +178,7 @@ def deliver_sms_with_rate_limiting(
             raise AutoRetryException(f'Found {type(e).__name__}, autoretrying...', e, e.args)
         else:
             msg = handle_max_retries_exceeded(notification_id, 'deliver_sms_with_rate_limiting')
+            check_and_queue_callback_task(notification)
             raise NotificationTechnicalFailureException(msg)
 
 
@@ -184,8 +198,9 @@ def deliver_email(
     notification_id: str,
     sms_sender_id=None,
 ):
+    current_app.logger.info('Start sending email for notification id: %s', notification_id)
+
     try:
-        current_app.logger.info('Start sending email for notification id: %s', notification_id)
         notification = notifications_dao.get_notification_by_id(notification_id)
         if not notification:
             current_app.logger.warning('Notification not found for: %s, retrying', notification_id)
@@ -201,7 +216,8 @@ def deliver_email(
         update_notification_status_by_id(
             notification_id, NOTIFICATION_TECHNICAL_FAILURE, status_reason='Email address is in invalid format'
         )
-        raise NotificationTechnicalFailureException(str(e))
+        check_and_queue_callback_task(notification)
+        raise NotificationTechnicalFailureException from e
     except MalwarePendingException:
         current_app.logger.info(
             'RETRY number %s: Email notification %s is pending malware scans', self.request.retries, notification_id
@@ -212,10 +228,11 @@ def deliver_email(
         update_notification_status_by_id(
             notification_id, NOTIFICATION_TECHNICAL_FAILURE, status_reason='Email provider configuration invalid'
         )
-        raise NotificationTechnicalFailureException(str(e))
+        check_and_queue_callback_task(notification)
+        raise NotificationTechnicalFailureException from e
     except (NullValueForNonConditionalPlaceholderException, AttributeError, RuntimeError) as e:
         log_and_update_technical_failure(notification_id, 'deliver_email', e)
-        raise NotificationTechnicalFailureException(f'Found {type(e).__name__}, NOT retrying...', e, e.args)
+        raise NotificationTechnicalFailureException('NOT retrying...') from e
     except Exception as e:
         current_app.logger.exception('Email delivery for notification id: %s failed', notification_id)
         if can_retry(self.request.retries, self.max_retries, notification_id):
@@ -230,4 +247,5 @@ def deliver_email(
             raise AutoRetryException(f'Found {type(e).__name__}, autoretrying...', e, e.args)
         else:
             msg = handle_max_retries_exceeded(notification_id, 'deliver_email')
+            check_and_queue_callback_task(notification)
             raise NotificationTechnicalFailureException(msg)