Skip to content

Commit

Permalink
Updated status reason
Browse files Browse the repository at this point in the history
  • Loading branch information
mchlwellman committed Dec 5, 2024
1 parent b6bc1e9 commit 310e79c
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 8 deletions.
6 changes: 3 additions & 3 deletions app/celery/provider_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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.clients.sms import OPT_OUT_MESSAGE
from app.clients.sms import MESSAGE_TOO_LONG, OPT_OUT_MESSAGE
from app.config import QueueNames
from app.constants import NOTIFICATION_TECHNICAL_FAILURE
from app.dao import notifications_dao
Expand Down Expand Up @@ -77,10 +77,10 @@ def deliver_sms(
)
raise NotificationTechnicalFailureException from e
except NonRetryableException as e:
# Likely an opted out from pinpoint

if 'opted out' in str(e).lower():
status_reason = OPT_OUT_MESSAGE
elif str(e) == MESSAGE_TOO_LONG:
status_reason = MESSAGE_TOO_LONG
else:
status_reason = 'ERROR: NonRetryableException - permanent failure, not retrying'

Expand Down
1 change: 1 addition & 0 deletions app/clients/sms/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
RETRYABLE_AWS_RESPONSE = 'Encountered a temporary failure. Send the request to VA Notify again'
UNABLE_TO_TRANSLATE = 'unable to translate delivery status'
UNEXPECTED_PROVIDER_RESULT = 'Unexpected result'
MESSAGE_TOO_LONG = 'Message too long'


class SmsClientResponseException(ClientException):
Expand Down
9 changes: 5 additions & 4 deletions app/clients/sms/twilio.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from twilio.base.exceptions import TwilioRestException

from app.celery.exceptions import NonRetryableException
from app.clients.sms import SmsClient, SmsStatusRecord, OPT_OUT_MESSAGE, UNABLE_TO_TRANSLATE
from app.clients.sms import SmsClient, SmsStatusRecord, OPT_OUT_MESSAGE, UNABLE_TO_TRANSLATE, MESSAGE_TOO_LONG
from app.constants import (
NOTIFICATION_CREATED,
NOTIFICATION_DELIVERED,
Expand Down Expand Up @@ -58,7 +58,7 @@ class TwilioSMSClient(SmsClient):
'21610': TwilioStatus(21610, NOTIFICATION_PERMANENT_FAILURE, OPT_OUT_MESSAGE),
'21612': TwilioStatus(21612, NOTIFICATION_PERMANENT_FAILURE, 'Invalid to/from combo'),
'21614': TwilioStatus(21614, NOTIFICATION_PERMANENT_FAILURE, 'Non-mobile number'),
'21617': TwilioStatus(21617, NOTIFICATION_PERMANENT_FAILURE, 'Message too long'),
'21617': TwilioStatus(21617, NOTIFICATION_PERMANENT_FAILURE, MESSAGE_TOO_LONG),
'21635': TwilioStatus(21635, NOTIFICATION_PERMANENT_FAILURE, 'Non-mobile number'),
'30001': TwilioStatus(30001, NOTIFICATION_TEMPORARY_FAILURE, 'Queue overflow'),
'30002': TwilioStatus(30002, NOTIFICATION_PERMANENT_FAILURE, 'Account suspended'),
Expand Down Expand Up @@ -239,11 +239,12 @@ def send_sms(
self.logger.exception('Twilio send SMS request for %s failed', reference)
raise InvalidProviderException from e
elif e.status == 400 and e.code == 21617: # Twilio error code for max length exceeded
status = self.twilio_error_code_map.get('21617')
self.logger.exception(
'Twilio send SMS request for %s failed, message content max length exceeded.', reference
'Twilio send SMS request for %s failed, message content length exceeded.', reference
)
self.logger.debug('Twilio error details for %s - %s: %s', reference, e.code, e.msg)
raise NonRetryableException('Twilio request failed') from e
raise NonRetryableException(status.status_reason) from e
else:
raise
except:
Expand Down
49 changes: 48 additions & 1 deletion tests/app/celery/test_provider_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from app.celery.exceptions import NonRetryableException, AutoRetryException
from app.celery.provider_tasks import deliver_sms, deliver_email, deliver_sms_with_rate_limiting
from app.clients.email.aws_ses import AwsSesClientThrottlingSendRateException
from app.clients.sms import OPT_OUT_MESSAGE
from app.clients.sms import MESSAGE_TOO_LONG, OPT_OUT_MESSAGE
from app.config import QueueNames
from app.constants import (
EMAIL_TYPE,
Expand Down Expand Up @@ -485,3 +485,50 @@ def test_deliver_sms_opt_out(
notify_db_session.session.refresh(notification)
assert notification.status == NOTIFICATION_PERMANENT_FAILURE
assert notification.status_reason == OPT_OUT_MESSAGE


@pytest.mark.parametrize(
'exception_message, status_reason',
(
('Message too long', MESSAGE_TOO_LONG),
('Destination phone number opted out', OPT_OUT_MESSAGE),
),
)
def test_deliver_sms_content_exceeded(
notify_db_session,
mocker,
sample_service,
sample_sms_sender,
sample_template,
sample_notification,
exception_message,
status_reason,
):
"""
An SMS notification sent with content that exceeds the maximum length allowed
should result in permanent failure with a relevant status reason.
"""

service = sample_service()
sms_sender = sample_sms_sender(service_id=service.id, sms_sender='17045555555')
template = sample_template(service=service)
notification = sample_notification(
template=template,
status=NOTIFICATION_CREATED,
sms_sender_id=sms_sender.id,
)
assert notification.notification_type == SMS_TYPE
assert notification.status == NOTIFICATION_CREATED
assert notification.sms_sender_id == sms_sender.id
assert notification.status_reason is None

mock_send_sms_to_provider = mocker.patch(
'app.delivery.send_to_providers.send_sms_to_provider',
side_effect=NonRetryableException(exception_message),
)
deliver_sms(notification.id, sms_sender_id=notification.sms_sender_id)
mock_send_sms_to_provider.assert_called_once()

notify_db_session.session.refresh(notification)
assert notification.status == NOTIFICATION_PERMANENT_FAILURE
assert notification.status_reason == status_reason

0 comments on commit 310e79c

Please sign in to comment.