#2070 Notification Status Reason for content length exceeded (#2161)

app/celery/provider_tasks.py

@@ -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
@@ -41,7 +41,7 @@
     retry_backoff_max=60,
 )
 @statsd(namespace='tasks')
-def deliver_sms(
+def deliver_sms(  # noqa: C901
     self: Task,
     notification_id,
     sms_sender_id=None,
@@ -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'
 

app/clients/sms/__init__.py

@@ -4,6 +4,7 @@
 from app.clients import Client, ClientException
 
 BLOCKED_MESSAGE = 'The message has been blocked'
+MESSAGE_TOO_LONG = 'Message too long'
 OPT_OUT_MESSAGE = 'The individual has opted out'
 PRICE_THRESHOLD_EXCEEDED = 'Price threshold exceeded'
 REPORTED_AS_SPAM = 'The message has been reported as spam'

app/clients/sms/twilio.py

@@ -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,
@@ -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'),
@@ -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:

tests/app/celery/test_provider_tasks.py

@@ -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,
@@ -449,17 +449,26 @@ def test_deliver_sms_with_rate_limiting_max_retries_exceeded(
     mocked_check_and_queue_callback_task.assert_called_once()
 
 
-def test_deliver_sms_opt_out(
+@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_non_retryables(
     notify_db_session,
     mocker,
     sample_service,
     sample_sms_sender,
     sample_template,
     sample_notification,
+    exception_message,
+    status_reason,
 ):
     """
-    An SMS notification sent to a recipient who has opted out of receiving notifications
-    from the given SMS sender should result in permanent failure with a relevant status reason.
+    An SMS notification sent to a non-retryable exception should be marked as permanent failure and have an
+    appropriate status reason.
     """
 
     service = sample_service()
@@ -477,11 +486,11 @@ def test_deliver_sms_opt_out(
 
     mock_send_sms_to_provider = mocker.patch(
         'app.delivery.send_to_providers.send_sms_to_provider',
-        side_effect=NonRetryableException('Destination phone number opted out'),
+        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 == OPT_OUT_MESSAGE
+    assert notification.status_reason == status_reason