Skip to content

Commit

Permalink
#2163 - Consolidate Status and Status Reason
Browse files Browse the repository at this point in the history
Co-authored-by: Chris Johnson <[email protected]>
  • Loading branch information
EvanParish and Chris Johnson authored Dec 19, 2024
1 parent b23997f commit c4bebf1
Show file tree
Hide file tree
Showing 42 changed files with 635 additions and 719 deletions.
86 changes: 2 additions & 84 deletions .talismanrc
Original file line number Diff line number Diff line change
@@ -1,92 +1,10 @@
fileignoreconfig:
- filename: README.md
checksum: b2cbbb8508af49abccae8b35b317f7ce09215f3508430ed31440add78f450e5a
- filename: app/callback/webhook_callback_strategy.py
checksum: 47846ab651c27512d3ac7864c08cb25d647f63bb84321953f907551fd9d2e85f
- filename: app/celery/contact_information_tasks.py
checksum: 80d0acf88bafb1358583016b9e143f4523ef1160d6eacdc9754ca68859b90eae
- filename: app/celery/nightly_tasks.py
checksum: b5b4999f778361603fdc7d710772968461e79a016f66badfbafd590bf70c5a11
- filename: app/celery/service_callback_tasks.py
checksum: 83b61b21668c1b1a0ea33a4c3130e82c9b14edbd6542079f1dd3d7493f3e9a79
- filename: app/commands.py
checksum: 79d914948e4e28dabc9c074956a11a9ceec27e7ae57b8eb8fb483f4a6217f07d
- filename: app/constants.py
checksum: 44390d0a1258b184cf84dc9b6e97bd0768af84a9aa346ba963aa7735fc8bcb36
- filename: app/dao/api_key_dao.py
checksum: c44cbd8ae02fb1d551a1f0941365c11977564a6444950ee2b0282ee4b5fd1314
- filename: app/letters/utils.py
checksum: 5e6071b9cab380f9f3ee172f8c731061241200f53453a9863f22bb5eaa05e6af
- filename: app/notifications/process_notifications.py
checksum: ae4e31c6eb56d91ec80ae09d13baf4558cf461c65f08893b93fee43f036a17a7
- filename: app/notifications/send_notifications.py
checksum: 12ed8e1334772c0d65cc13be03b6d5197ebbf4d35460aee2fac1fc2a9644e9e1
- filename: app/service/rest.py
checksum: 7f8a30dd84b3ceb0d08bae949b5b127fd408ee2fd8097eb7d4b291ede61f8d0f
- filename: app/service/utils.py
checksum: c1e34f85b5b0fc32eb6a3afcc16c7099b74294e578487004dc6ba3bc46832010
- filename: app/template/rest.py
checksum: 1e5bdac8bc694d50f8f656dec127dd036b7b1b5b6156e3282d3411956c71ba0b
- filename: app/user/rest.py
checksum: e867ef2c802c8a3fd187fd0df76533e62cccb1721582c4fc4d2a920eb11c1e3b
- filename: app/v3/notifications/rest.py
checksum: aea8f32b796f324e4b57e1be6703b31c0550447011550d944cdb461489203f7d
- filename: app/va/va_profile/va_profile_client.py
checksum: fe634f26f7dc3874f4afcfd1ba3f03bae380b53befe973a752c7347097a88701
- filename: cd/application-deployment/dev/dev.env
checksum: d0897f1124e6725347d18a9ba8de56baf848151ecad688db189e0ec2d6282bcb
- filename: cd/application-deployment/dev/vaec-api-task-definition.json
checksum: f328ff821339b802eb1d82559e624d5b719857c813d427da5aaa39b240331ddd
- filename: cd/application-deployment/perf/perf.env
checksum: d8f93f6bf746751e364d4cb81f26e656bd7ed0f146d98c03fd704d716acc5805
- filename: cd/application-deployment/prod/prod.env
checksum: 949dc3234b36d9fc8706a927b5c342372aa1ac5d6ba8f9458f7482e0ee31a180
- filename: cd/application-deployment/staging/staging.env
checksum: 15e41a76cd47e45be327188513e82763aa1a8d4d64ead457a8866f6f854e61d7
- filename: ci/.local.env
checksum: 2c4024d4e94ffa39ff73e6039d65f0b1d19557067ddf6d97ef14aea86cfb58ad
- filename: ci/docker-compose-test.yml
checksum: e3efec2749e8c19e60f5bfc68eafabe24eba647530a482ceccfc4e0e62cff424
- filename: documents/openapi/openapi.yaml
checksum: f67b71f8506be867a206e8f4605c1bbc7a3b13c9d18ce6aa5317b4cbed4a50aa
- filename: lambda_functions/pinpoint_callback/pinpoint_callback_lambda.py
checksum: 7bd4900e14b1fa789bbb2568b8a8d7a400e3c8350ba32fb44cc0b5b66a2df037
- filename: lambda_functions/ses_callback/ses_callback_lambda.py
checksum: b20c36921290a9609f158784e2a3278c36190887e6054ea548004a67675fd79b
- filename: lambda_functions/va_profile/va_profile_opt_in_out_lambda.py
checksum: a05165537ffbfac90000c5d04d8628251d771f6d1334c91c3aed28bf6c32368c
- filename: poetry.lock
checksum: 099b2059d9ef8b472afbd7029680f16b4833c35feca2cf9a5acc2e6f7a8ffa9e
- filename: scripts/trigger_task.py
checksum: 0e9d244dbe285de23fc84bb643407963dacf7d25a3358373f01f6272fb217778
- filename: tests/README.md
checksum: 4de2edc5ca36f266118231611036c7ccc6d9328bcceefafd8ca5ca92021f8aed
- filename: tests/app/callback/test_webhook_callback_strategy.py
checksum: 288841d3209dc3ca885cd0bb08591221f7f15e5b3406fb7140505096db212554
- filename: tests/app/celery/test_process_delivery_status_result_tasks.py
checksum: 62fa6216b62971d62c2e53f6b31aeeb659d7a1e404665362ee89cb3ec04793a6
- filename: tests/app/celery/test_process_ga4_measurement_task.py
checksum: 6ffb8742a19c5b834c608826fd459cc1b6ea35ebfffd2d929a3a0f269c74183d
- filename: tests/app/celery/test_service_callback_tasks.py
checksum: 70575434f7a4fedd43d4c9164bc899a606768526d432c364db372524eec26542
- filename: tests/app/clients/test_twilio.py
checksum: cad49e634cc5ba56157358aa3dfba2dafe7b9dbd3a0c580ec5cda3072f6a76e5
- filename: tests/app/conftest.py
checksum: a80aa727586db82ed1b50bdb81ddfe1379e649a9dfc1ece2c36047486b41b83d
- filename: tests/app/dao/test_api_key_dao.py
checksum: 40e551ca6677aab7657bbb43efdac56aa3c51065ed99052faff9bc1519e5b0df
- filename: tests/app/notifications/test_process_notifications_for_profile_v3.py
checksum: 4e15e63d349635131173ffdd7aebcd547621db08de877ef926d3a41fde72d065
- filename: tests/app/notifications/test_send_notifications.py
checksum: 69304e1edd6993acd9a842a87dba8ee16854befc643863e1589b15c303a71464
checksum: 5bab4eaddf8760c502111ae3e5f9f8bee59482d99f053f94598e8c77bd10b7b6
- filename: tests/app/v2/notifications/test_get_notifications.py
checksum: f2460d8b22ff7ff2e89778dbbf3e0740cbb41dead60c49c32648d367aa05fe0e
- filename: tests/app/v2/notifications/test_post_notifications.py
checksum: 3181930a13e3679bb2f17eaa3f383512eb9caf4ed5d5e14496ca4193c6083965
- filename: tests/app/v3/notifications/test_rest.py
checksum: 25aeb31ce5974f0a414ff43a9a75f787a6cf41c783f7eacf2415ce1fedf54c1d
- filename: tests/lambda_functions/va_profile/test_va_profile_integration.py
checksum: 4f0f4d7a4113762219e45a51f7b26a7c0cb83f1d8f10c5598533f6cdcf0e0ada
- filename: tests/lambda_functions/vetext_incoming_forwarder_lambda/test_vetext_incoming_forwarder_lambda.py
checksum: 7494eb4321fd2fbc3ff3915d8753d8fec7a936a69dc6ab78f0b532a701f032eb
checksum: 167aa80c8aac01566b3cf627cdaa839feac9dc77f0b13b6d0a008035aa8e15f1
version: "1.0"
29 changes: 15 additions & 14 deletions app/celery/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@

from app.celery.service_callback_tasks import check_and_queue_callback_task
from app.dao.notifications_dao import get_notification_by_id, update_notification_status_by_id
from app.constants import NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TECHNICAL_FAILURE


RETRIES_EXCEEDED = 'Retries exceeded'
TECHNICAL_ERROR = 'VA Notify non-retryable technical error'
from app.constants import NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_UNDELIVERABLE


def can_retry(
Expand All @@ -34,33 +30,38 @@ def handle_max_retries_exceeded(
message = (
'RETRY FAILED: Max retries reached. '
f'The task {method_name} failed for notification {notification_id}. '
'Notification has been updated to technical-failure'
'Notification has been updated to permanent-failure'
)
update_notification_status_by_id(
notification_id,
NOTIFICATION_PERMANENT_FAILURE,
status_reason=STATUS_REASON_UNDELIVERABLE,
)
update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE, status_reason=RETRIES_EXCEEDED)
return message


def log_and_update_technical_failure(
def log_and_update_critical_failure(
notification_id: UUID,
method_name: str,
e: Exception,
status_reason: str = None,
status_reason: str,
) -> None:
"""Handles sms/email deliver requests that failed in a technical manner due to an exception"""
"""Handles sms/email deliver requests that failed in a technical manner due to an exception."""
current_app.logger.critical(
'%s: Notification: %s - Experienced an exception: %s',
'%s: Notification: %s - Experienced a critical failure with exception: %s',
method_name,
notification_id,
e,
)

update_notification_status_by_id(
notification_id,
NOTIFICATION_TECHNICAL_FAILURE,
status_reason=status_reason or TECHNICAL_ERROR,
NOTIFICATION_PERMANENT_FAILURE,
status_reason=status_reason,
)

current_app.logger.critical(
'Notification %s encountered a technical exception and has been updated to a technical-failure',
'Notification %s encountered a technical exception and has been updated to a permanent-failure',
notification_id,
)

Expand Down
22 changes: 10 additions & 12 deletions app/celery/contact_information_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
from app.celery.service_callback_tasks import check_and_queue_callback_task
from app.constants import (
NOTIFICATION_PERMANENT_FAILURE,
NOTIFICATION_PREFERENCES_DECLINED,
EMAIL_TYPE,
SMS_TYPE,
STATUS_REASON_DECLINED,
)
from app.dao.notifications_dao import (
get_notification_by_id,
Expand Down Expand Up @@ -142,9 +142,7 @@ def handle_lookup_contact_info_exception(
)
current_app.logger.info('%s - %s: %s', e.__class__.__name__, str(e), message)

update_notification_status_by_id(
notification.id, NOTIFICATION_PERMANENT_FAILURE, status_reason=e.failure_reason
)
update_notification_status_by_id(notification.id, NOTIFICATION_PERMANENT_FAILURE, status_reason=e.status_reason)
check_and_queue_callback_task(notification)
# Expected chain termination
lookup_task.request.chain = None
Expand All @@ -154,9 +152,7 @@ def handle_lookup_contact_info_exception(
'Notification has been updated to permanent-failure'
)
current_app.logger.info(message)
update_notification_status_by_id(
notification.id, NOTIFICATION_PERMANENT_FAILURE, status_reason=e.failure_reason
)
update_notification_status_by_id(notification.id, NOTIFICATION_PERMANENT_FAILURE, status_reason=e.status_reason)
check_and_queue_callback_task(notification)
# Expected chain termination
lookup_task.request.chain = None
Expand All @@ -168,9 +164,9 @@ def handle_lookup_contact_info_exception(
)
if not notification.default_send:
update_notification_status_by_id(
notification_id=notification.id,
status=NOTIFICATION_PERMANENT_FAILURE,
status_reason='No recipient opt-in found for explicit preference',
notification.id,
NOTIFICATION_PERMANENT_FAILURE,
status_reason=e.status_reason,
)
check_and_queue_callback_task(notification)
# Expected chain termination
Expand Down Expand Up @@ -206,8 +202,10 @@ def handle_communication_not_allowed(
recipient_identifier.id_value,
notification.id,
)
reason = permission_message if permission_message is not None else 'Contact preferences set to false'
update_notification_status_by_id(notification.id, NOTIFICATION_PREFERENCES_DECLINED, status_reason=reason)

update_notification_status_by_id(
notification.id, NOTIFICATION_PERMANENT_FAILURE, status_reason=STATUS_REASON_DECLINED
)

message = f'The recipient for notification {notification.id} has declined permission to receive notifications.'
current_app.logger.info(message)
Expand Down
9 changes: 4 additions & 5 deletions app/celery/lookup_va_profile_id_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from flask import current_app
from notifications_utils.statsd_decorators import statsd

from app.constants import NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_PERMANENT_FAILURE
from app.constants import NOTIFICATION_PERMANENT_FAILURE, STATUS_REASON_NO_ID_FOUND
from app.exceptions import NotificationTechnicalFailureException
from app.models import RecipientIdentifier
from app import notify_celery
Expand Down Expand Up @@ -78,20 +78,19 @@ def lookup_va_profile_id(
)
current_app.logger.warning(message)
notifications_dao.update_notification_status_by_id(
notification_id, NOTIFICATION_PERMANENT_FAILURE, status_reason=e.failure_reason
notification_id, NOTIFICATION_PERMANENT_FAILURE, status_reason=e.status_reason
)
check_and_queue_callback_task(notification)
# Expected chain termination
self.request.chain = None
except Exception as e:
message = (
f'Failed to retrieve VA Profile ID from MPI for notification: {notification_id} '
'Notification has been updated to technical-failure'
'Notification has been updated to permanent-failure'
)
current_app.logger.exception(message)
status_reason = e.failure_reason if hasattr(e, 'failure_reason') else 'Unknown error from MPI'
notifications_dao.update_notification_status_by_id(
notification_id, NOTIFICATION_TECHNICAL_FAILURE, status_reason=status_reason
notification_id, NOTIFICATION_PERMANENT_FAILURE, status_reason=STATUS_REASON_NO_ID_FOUND
)
check_and_queue_callback_task(notification)
raise NotificationTechnicalFailureException(message) from e
20 changes: 12 additions & 8 deletions app/celery/nightly_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,24 +99,28 @@ def delete_letter_notifications_older_than_retention():
@notify_celery.task(name='timeout-sending-notifications')
@cronitor('timeout-sending-notifications')
@statsd(namespace='tasks')
def timeout_notifications():
"""A task that runs every night at 12:05 AM EST to update the status of notifications that have timed out."""
technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(
def timeout_notifications() -> None:
"""Task that runs every night at 12:05 AM EST to update the status of notifications that have timed out.
Raises:
NotificationTechnicalFailureException: If there are any notifications that were stuck in created status
"""
unsent_created_notifications, temporary_failure_notifications = dao_timeout_notifications(
current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')
)
notifications = technical_failure_notifications + temporary_failure_notifications
notifications = unsent_created_notifications + temporary_failure_notifications
for notification in notifications:
# queue callback task only if the service_callback_api exists
check_and_queue_callback_task(notification)

current_app.logger.info(
'Timeout period reached for {} notifications, status has been updated.'.format(len(notifications))
)
if technical_failure_notifications:
if unsent_created_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]
'{} notifications have been updated to permanent-failure because they '
'have timed out and are still in created. Notification ids: {}'.format(
len(unsent_created_notifications), [str(x.id) for x in unsent_created_notifications]
)
)
raise NotificationTechnicalFailureException(message)
Expand Down
10 changes: 7 additions & 3 deletions app/celery/process_ses_receipts_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
NOTIFICATION_PENDING,
NOTIFICATION_PERMANENT_FAILURE,
NOTIFICATION_TEMPORARY_FAILURE,
STATUS_REASON_RETRYABLE,
STATUS_REASON_UNDELIVERABLE,
)
from app.clients.email.aws_ses import get_aws_responses
from app.dao import notifications_dao, services_dao, templates_dao
Expand Down Expand Up @@ -212,17 +214,19 @@ def process_ses_results( # noqa: C901 (too complex 14 > 10)
if notification_status in (NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE):
# Add the failure status reason to the notification.
if notification_status == NOTIFICATION_PERMANENT_FAILURE:
status_reason = 'Failed to deliver email due to hard bounce'
failure_reason = 'Failed to deliver email due to hard bounce'
status_reason = STATUS_REASON_UNDELIVERABLE
else:
status_reason = 'Temporarily failed to deliver email due to soft bounce'
failure_reason = 'Temporarily failed to deliver email due to soft bounce'
status_reason = STATUS_REASON_RETRYABLE

notification.status_reason = status_reason
notification.status = notification_status

current_app.logger.warning(
'%s - %s - in process_ses_results for notification %s',
notification_status,
status_reason,
failure_reason,
notification.id,
)

Expand Down
Loading

0 comments on commit c4bebf1

Please sign in to comment.