Handle rate limit error

* add status code 429 to the list of retriable errors

FIXES pycontribs#925

Author: Stefano Boriero

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,14 @@ 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]:
+                rate_limit_remaining = response.headers["X-RateLimit-Remaining"]
                 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: number of tokens remaining {rate_limit_remaining}. Consider adding 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
@@ -128,7 +131,7 @@ def __recoverable(
             else:
                 msg = "Atlassian's bug https://jira.atlassian.com/browse/JRA-41559"
 
-        # Exponential backoff with full jitter.
+        # Exponential backoff with full jijitter.
         delay = min(self.max_retry_delay, 10 * 2**counter) * random.random()
         logging.warning(
             "Got recoverable error from %s %s, will retry [%s/%s] in %ss. Err: %s"
@@ -137,6 +140,7 @@ def __recoverable(
         if isinstance(response, Response):
             logging.debug("response.headers: %s", response.headers)
             logging.debug("response.body: %s", response.content)
+            logging.debug(msg)
         time.sleep(delay)
         return True
 

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,36 @@ def test_logging_with_connection_error(self):
     def tearDown(self):
         jira.resilientsession.logging.getLogger().removeHandler(self.loggingHandler)
         del self.loggingHandler
+
+
+@patch("requests.Session.get")
+@patch("time.sleep")
+def test_throttling_error_is_retried(
+    mocked_sleep_method: Mock, mocked_get_method: Mock
+):
+    mocked_throttled_response: Response = Response()
+    mocked_throttled_response.status_code = 429
+    mocked_throttled_response.headers["X-RateLimit-Remaining"] = "1"
+    mocked_get_method.return_value = mocked_throttled_response
+    session: jira.resilientsession.ResilientSession = (
+        jira.resilientsession.ResilientSession()
+    )
+    with pytest.raises(JIRAError):
+        session.get("mocked_url")
+    assert mocked_get_method.call_count == 4
+    assert mocked_sleep_method.call_count == 3
+
+
+@patch("requests.Session.get")
+@patch("time.sleep")
+def test_5xx_error_is_not_retried(mocked_sleep_method: Mock, mocked_get_method: Mock):
+    mocked_5xx_response: Response = Response()
+    mocked_5xx_response.status_code = 502
+    mocked_get_method.return_value = mocked_5xx_response
+    session: jira.resilientsession.ResilientSession = (
+        jira.resilientsession.ResilientSession()
+    )
+    with pytest.raises(JIRAError):
+        session.get("mocked_url")
+    assert mocked_get_method.call_count == 1
+    assert mocked_sleep_method.call_count == 0