Skip to content

Commit

Permalink
Retry callbacks on 429 'too many requests' code (#2155)
Browse files Browse the repository at this point in the history
* Handle 429 'too many requests' differently with callbacks by retrying later

* Added comments on code intent.

* Run formatting on test file
  • Loading branch information
jimleroyer authored Apr 18, 2024
1 parent f664568 commit 522451e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 17 deletions.
20 changes: 6 additions & 14 deletions app/celery/service_callback_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,31 +65,23 @@ def _send_data_to_service_callback_api(self, data, service_callback_url, token,
data=json.dumps(data),
headers={
"Content-Type": "application/json",
"Authorization": "Bearer {}".format(token),
"Authorization": f"Bearer {token}",
},
timeout=5,
)
current_app.logger.info(
"{} sending {} to {}, response {}".format(
function_name,
notification_id,
service_callback_url,
response.status_code,
)
f"{function_name} sending {notification_id} to {service_callback_url}, response {response.status_code}"
)
response.raise_for_status()
except RequestException as e:
current_app.logger.warning(
"{} request failed for notification_id: {} and url: {}. exc: {}".format(
function_name, notification_id, service_callback_url, e
)
f"{function_name} request failed for notification_id: {notification_id} and url: {service_callback_url}. exc: {e}"
)
if not isinstance(e, HTTPError) or e.response.status_code >= 500:
# Retry if the response status code is server-side or 429 (too many requests).
if not isinstance(e, HTTPError) or e.response.status_code >= 500 or e.response.status_code == 429:
try:
self.retry(queue=QueueNames.CALLBACKS_RETRY)
except self.MaxRetriesExceededError:
current_app.logger.warning(
"Retry: {} has retried the max num of times for callback url {} and notification_id: {}".format(
function_name, service_callback_url, notification_id
)
"Retry: {function_name} has retried the max num of times for callback url {service_callback_url} and notification_id: {notification_id}"
)
7 changes: 4 additions & 3 deletions tests/app/celery/test_service_callback_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,9 @@ def test_send_complaint_to_service_posts_https_request_to_service_with_signed_da


@pytest.mark.parametrize("notification_type", ["email", "letter", "sms"])
def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_signed_data(
notify_db_session, mocker, notification_type
@pytest.mark.parametrize("status_code", [429, 500, 503])
def test__send_data_to_service_callback_api_retries_if_request_returns_error_code_with_signed_data(
notify_db_session, mocker, notification_type, status_code
):
callback_api, template = _set_up_test_data(notification_type, "delivery_status")
datestr = datetime(2017, 6, 20)
Expand All @@ -107,7 +108,7 @@ def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_
signed_data = _set_up_data_for_status_update(callback_api, notification)
mocked = mocker.patch("app.celery.service_callback_tasks.send_delivery_status_to_service.retry")
with requests_mock.Mocker() as request_mock:
request_mock.post(callback_api.url, json={}, status_code=500)
request_mock.post(callback_api.url, json={}, status_code=status_code)
send_delivery_status_to_service(notification.id, signed_status_update=signed_data)

assert mocked.call_count == 1
Expand Down

0 comments on commit 522451e

Please sign in to comment.