Merge pull request #1395 from alphagov/catch-inbound-sms-push-exception

Retry unknown exceptions for send_inbound_sms_to_service task

app/celery/tasks.py

@@ -14,7 +14,8 @@ from notifications_utils.template import (
 )
 from requests import (
     HTTPError,
-    request
+    request,
+    RequestException
 )
 from sqlalchemy.exc import SQLAlchemyError
 from app import (
@@ -492,25 +493,24 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id):
         "date_received": inbound_sms.provider_date.strftime(DATETIME_FORMAT)
     }
 
-    response = request(
-        method="POST",
-        url=inbound_api.url,
-        data=json.dumps(data),
-        headers={
-            'Content-Type': 'application/json',
-            'Authorization': 'Bearer {}'.format(inbound_api.bearer_token)
-        },
-        timeout=60
-    )
-    current_app.logger.info('send_inbound_sms_to_service sending {} to {}, response {}'.format(
-        inbound_sms_id,
-        inbound_api.url,
-        response.status_code
-    ))
-
     try:
+        response = request(
+            method="POST",
+            url=inbound_api.url,
+            data=json.dumps(data),
+            headers={
+                'Content-Type': 'application/json',
+                'Authorization': 'Bearer {}'.format(inbound_api.bearer_token)
+            },
+            timeout=60
+        )
+        current_app.logger.info('send_inbound_sms_to_service sending {} to {}, response {}'.format(
+            inbound_sms_id,
+            inbound_api.url,
+            response.status_code
+        ))
         response.raise_for_status()
-    except HTTPError as e:
+    except RequestException as e:
         current_app.logger.warning(
             "send_inbound_sms_to_service request failed for service_id: {} and url: {}. exc: {}".format(
                 service_id,
@@ -518,7 +518,7 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id):
                 e
             )
         )
-        if e.response.status_code >= 500:
+        if not isinstance(e, HTTPError) or e.response.status_code >= 500:
             try:
                 self.retry(queue=QueueNames.RETRY,
                            exc='Unable to send_inbound_sms_to_service for service_id: {} and url: {}. \n{}'.format(

tests/app/celery/test_tasks.py

@@ -7,6 +7,7 @@ import pytest
 import requests_mock
 from flask import current_app
 from freezegun import freeze_time
+from requests import RequestException
 from sqlalchemy.exc import SQLAlchemyError
 from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate, LetterDVLATemplate
 from celery.exceptions import Retry
@@ -1160,6 +1161,26 @@ def test_send_inbound_sms_to_service_retries_if_request_returns_500(notify_api,
     assert exc_msg in mocked.call_args[1]['exc']
 
 
+def test_send_inbound_sms_to_service_retries_if_request_throws_unknown(notify_api, sample_service, mocker):
+    inbound_api = create_service_inbound_api(service=sample_service, url="https://some.service.gov.uk/",
+                                             bearer_token="something_unique")
+    inbound_sms = create_inbound_sms(service=sample_service, notify_number="0751421", user_number="447700900111",
+                                     provider_date=datetime(2017, 6, 20), content="Here is some content")
+
+    mocked = mocker.patch('app.celery.tasks.send_inbound_sms_to_service.retry')
+    mocker.patch("app.celery.tasks.request", side_effect=RequestException())
+
+    send_inbound_sms_to_service(inbound_sms.id, inbound_sms.service_id)
+
+    exc_msg = 'Unable to send_inbound_sms_to_service for service_id: {} and url: {url}'.format(
+        sample_service.id,
+        url=inbound_api.url
+    )
+    assert mocked.call_count == 1
+    assert mocked.call_args[1]['queue'] == 'retry-tasks'
+    assert exc_msg in mocked.call_args[1]['exc']
+
+
 def test_send_inbound_sms_to_service_does_not_retries_if_request_returns_404(notify_api, sample_service, mocker):
     inbound_api = create_service_inbound_api(service=sample_service, url="https://some.service.gov.uk/",
                                              bearer_token="something_unique")