From e2dd9b473510c0465148115287bc8109c93798af Mon Sep 17 00:00:00 2001 From: Stefano Boriero Date: Fri, 29 Apr 2022 12:43:10 +0200 Subject: [PATCH] Handle rate limit error, status code 429 (#1364) --- jira/resilientsession.py | 22 +++++++++++++---- tests/test_resilientsession.py | 45 ++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/jira/resilientsession.py b/jira/resilientsession.py index dbd6b9c80..ec01b6f5d 100644 --- a/jira/resilientsession.py +++ b/jira/resilientsession.py @@ -88,7 +88,7 @@ def raise_on_error(r: Optional[Response], verb="???", **kwargs): class ResilientSession(Session): """This class is supposed to retry requests that do return temporary errors. - At this moment it supports: 502, 503, 504 + At this moment it supports: 429 """ def __init__(self, timeout=None): @@ -113,11 +113,23 @@ def __recoverable( f"Got ConnectionError [{response}] errno:{response.errno} on {request} {url}\n{vars(response)}\n{response.__dict__}" ) if isinstance(response, Response): - if response.status_code in [502, 503, 504, 401]: - # 401 UNAUTHORIZED still randomly returned by Atlassian Cloud as of 2017-01-16 + if response.status_code in [429]: + number_of_tokens_issued_per_interval = response.headers[ + "X-RateLimit-FillRate" + ] + token_issuing_rate_interval_seconds = response.headers[ + "X-RateLimit-Interval-Seconds" + ] + maximum_number_of_tokens = response.headers["X-RateLimit-Limit"] + retry_after = response.headers["retry-after"] msg = f"{response.status_code} {response.reason}" - # 2019-07-25: Disabled recovery for codes above^ - return False + logging.warning( + f"Request rate limited by Jira: request should be retried after {retry_after} seconds.\n" + + f"{number_of_tokens_issued_per_interval} tokens are issued every {token_issuing_rate_interval_seconds} seconds. " + + f"You can accumulate up to {maximum_number_of_tokens} tokens.\n" + + "Consider adding an exemption for the user as explained in: " + + "https://confluence.atlassian.com/adminjiraserver/improving-instance-stability-with-rate-limiting-983794911.html" + ) elif not ( response.status_code == 200 and len(response.content) == 0 diff --git a/tests/test_resilientsession.py b/tests/test_resilientsession.py index 831321cb0..023fc5c8e 100644 --- a/tests/test_resilientsession.py +++ b/tests/test_resilientsession.py @@ -1,6 +1,11 @@ import logging +from unittest.mock import Mock, patch + +import pytest +from requests import Response import jira.resilientsession +from jira.exceptions import JIRAError from tests.conftest import JiraTestCase @@ -53,3 +58,43 @@ def test_logging_with_connection_error(self): def tearDown(self): jira.resilientsession.logging.getLogger().removeHandler(self.loggingHandler) del self.loggingHandler + + +status_codes_retries_test_data = [ + (429, 4, 3), + (401, 1, 0), + (403, 1, 0), + (404, 1, 0), + (502, 1, 0), + (503, 1, 0), + (504, 1, 0), +] + + +@patch("requests.Session.get") +@patch("time.sleep") +@pytest.mark.parametrize( + "status_code,expected_number_of_retries,expected_number_of_sleep_invocations", + status_codes_retries_test_data, +) +def test_status_codes_retries( + mocked_sleep_method: Mock, + mocked_get_method: Mock, + status_code: int, + expected_number_of_retries: int, + expected_number_of_sleep_invocations: int, +): + mocked_response: Response = Response() + mocked_response.status_code = status_code + mocked_response.headers["X-RateLimit-FillRate"] = "1" + mocked_response.headers["X-RateLimit-Interval-Seconds"] = "1" + mocked_response.headers["retry-after"] = "1" + mocked_response.headers["X-RateLimit-Limit"] = "1" + mocked_get_method.return_value = mocked_response + session: jira.resilientsession.ResilientSession = ( + jira.resilientsession.ResilientSession() + ) + with pytest.raises(JIRAError): + session.get("mocked_url") + assert mocked_get_method.call_count == expected_number_of_retries + assert mocked_sleep_method.call_count == expected_number_of_sleep_invocations