diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 5b92db0a5..9935ad609 100644 --- a/app/celery/tasks.py +++ b/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( diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 69f32c421..5267dd436 100644 --- a/tests/app/celery/test_tasks.py +++ b/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")