Skip to content

Commit

Permalink
Exception Retry Hotfix (#1409)
Browse files Browse the repository at this point in the history
* Added method to handle non-retryable

* Handled NullValueForNonConditionalPlaceholderException

* Update app/celery/provider_tasks.py

Copy/paste error fixed

Co-authored-by: Evan Parish <[email protected]>

---------

Co-authored-by: Evan Parish <[email protected]>
  • Loading branch information
k-macmillan and EvanParish authored Aug 7, 2023
1 parent 0c7fde3 commit edad7c8
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
14 changes: 14 additions & 0 deletions app/celery/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@


RETRIES_EXCEEDED = "Retries exceeded"
TECHNICAL_ERROR = "VA Notify non-retryable technical error"


def can_retry(retries: int, max_retries: int, notification_id: str) -> bool:
Expand All @@ -24,3 +25,16 @@ def handle_max_retries_exceeded(notification_id: str, method_name: str) -> str:
status_reason=RETRIES_EXCEEDED
)
return message


def handle_non_retryable(notification_id: str, method_name: str) -> str:
""" Handles sms/email deliver requests that failed in a non-retryable manner """
current_app.logger.critical("%s: Notification %s encountered a non-retryable exception",
method_name, notification_id)
message = "Notification has been updated to technical-failure due to a non-retryable exception"
update_notification_status_by_id(
notification_id,
NOTIFICATION_TECHNICAL_FAILURE,
status_reason=TECHNICAL_ERROR
)
return message
12 changes: 11 additions & 1 deletion app/celery/provider_tasks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from app import notify_celery
from app.celery.common import can_retry, handle_max_retries_exceeded
from app.celery.common import can_retry, handle_max_retries_exceeded, handle_non_retryable
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
Expand All @@ -12,6 +12,7 @@
from app.models import NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_PERMANENT_FAILURE
from app.v2.errors import RateLimitError
from flask import current_app
from notifications_utils.field import NullValueForNonConditionalPlaceholderException
from notifications_utils.recipients import InvalidEmailError
from notifications_utils.statsd_decorators import statsd

Expand Down Expand Up @@ -51,6 +52,9 @@ def deliver_sms(self, notification_id, sms_sender_id=None):
)
notification = notifications_dao.get_notification_by_id(notification_id)
check_and_queue_callback_task(notification)
except NullValueForNonConditionalPlaceholderException:
msg = handle_non_retryable(notification_id, 'deliver_sms')
raise NotificationTechnicalFailureException(msg)
except Exception as e:
current_app.logger.exception(
"SMS delivery for notification id: %s failed", notification_id
Expand Down Expand Up @@ -106,6 +110,9 @@ def deliver_sms_with_rate_limiting(self, notification_id, sms_sender_id=None):
)

self.retry(queue=QueueNames.RATE_LIMIT_RETRY, max_retries=None, countdown=retry_time)
except NullValueForNonConditionalPlaceholderException:
msg = handle_non_retryable(notification_id, 'deliver_sms_with_rate_limiting')
raise NotificationTechnicalFailureException(msg)
except Exception as e:
current_app.logger.exception(
"Rate Limit SMS notification delivery for id: %s failed", notification_id
Expand Down Expand Up @@ -155,6 +162,9 @@ def deliver_email(self, notification_id: str, sms_sender_id=None):
status_reason="Email provider configuration invalid"
)
raise NotificationTechnicalFailureException(str(e))
except NullValueForNonConditionalPlaceholderException:
msg = handle_non_retryable(notification_id, 'deliver_email')
raise NotificationTechnicalFailureException(msg)
except Exception as e:
current_app.logger.exception(
"Email delivery for notification id: %s failed", notification_id
Expand Down

0 comments on commit edad7c8

Please sign in to comment.