Skip to content

Commit

Permalink
Also retry requests where Jira specifies a Retry-after of 0 seconds
Browse files Browse the repository at this point in the history
When rejecting request with a 429 response, Jira sometimes sends a Retry-after header asking for a backoff of 0 seconds. With the existing retry logic this would mark the request as non-retryable and thus fail the request. With this change, such requests are treated as if Jira had send a retry-after value of 1 second.

This solves one of the issues reported in #1805.
  • Loading branch information
matthias-bach-by committed Feb 26, 2024
1 parent 5a14fda commit c7e5b53
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
4 changes: 3 additions & 1 deletion jira/resilientsession.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ def __recoverable(
if response.status_code in recoverable_error_codes:
retry_after = response.headers.get("Retry-After")
if retry_after:
suggested_delay = 2 * int(retry_after) # Do as told
suggested_delay = 2 * max(
int(retry_after), 1
) # Do as told but always wait at least a little
elif response.status_code == HTTPStatus.TOO_MANY_REQUESTS:
suggested_delay = 10 * 2**counter # Exponential backoff

Expand Down
14 changes: 9 additions & 5 deletions tests/test_resilientsession.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,17 @@ def tearDown(self):


# Retry test data tuples: (status_code, with_rate_limit_header, with_retry_after_header, retry_expected)
with_rate_limit = with_retry_after = True
without_rate_limit = without_retry_after = False
with_rate_limit = True
with_retry_after = 1
without_rate_limit = False
without_retry_after = None
status_codes_retries_test_data = [
# Always retry 429 responses
(HTTPStatus.TOO_MANY_REQUESTS, with_rate_limit, with_retry_after, True),
(HTTPStatus.TOO_MANY_REQUESTS, with_rate_limit, 0, True),
(HTTPStatus.TOO_MANY_REQUESTS, with_rate_limit, without_retry_after, True),
(HTTPStatus.TOO_MANY_REQUESTS, without_rate_limit, with_retry_after, True),
(HTTPStatus.TOO_MANY_REQUESTS, without_rate_limit, 0, True),
(HTTPStatus.TOO_MANY_REQUESTS, without_rate_limit, without_retry_after, True),
# Retry 503 responses only when 'Retry-After' in headers
(HTTPStatus.SERVICE_UNAVAILABLE, with_rate_limit, with_retry_after, True),
Expand Down Expand Up @@ -103,10 +107,10 @@ def test_status_codes_retries(
mocked_request_method: Mock,
status_code: int,
with_rate_limit_header: bool,
with_retry_after_header: bool,
with_retry_after_header: int | None,
retry_expected: bool,
):
RETRY_AFTER_SECONDS = 1 if with_retry_after_header else 0
RETRY_AFTER_SECONDS = with_retry_after_header or 0
RETRY_AFTER_HEADER = {"Retry-After": f"{RETRY_AFTER_SECONDS}"}
RATE_LIMIT_HEADERS = {
"X-RateLimit-FillRate": "1",
Expand All @@ -125,7 +129,7 @@ def test_status_codes_retries(

mocked_response: Response = Response()
mocked_response.status_code = status_code
if with_retry_after_header:
if with_retry_after_header is not None:
mocked_response.headers.update(RETRY_AFTER_HEADER)
if with_rate_limit_header:
mocked_response.headers.update(RATE_LIMIT_HEADERS)
Expand Down

0 comments on commit c7e5b53

Please sign in to comment.