feat: add option to retry on client errors (#20)

Co-authored-by: Peter Topor <[email protected]>

docs/source/retry.rst

@@ -12,9 +12,10 @@ E.g. if retry is set to 2 and there is no success on any attempt,
 the total number of requests will be 3: nominal request and 2 retries on
 failure.
 
-Important note: the request  is not retried in case of client error,
-that is if the server responded with HTTP 4xx,
-not including HTTP 408 Request Timeout
+Important note: by default, the request is not retried in case of client error,
+that is if the server responded with HTTP 4xx, not including HTTP 408 Request Timeout.
+This behavior is configurable using ``retriable_client_errors`` constructor parameter,
+where custom list of HTTP codes can be provided to enable automatic retry.
 
 If ``sleep_before_repeat`` parameter is passed,
 the method waits for that amount of seconds before retrying.

request_session/request_session.py

@@ -92,6 +92,7 @@ def __init__(
             "log",
         ),  # type: Tuple
         ddtrace_service_name="booking_api_requests",  # type: str
+        retriable_client_errors=None,  # type: Optional[List[int]]
     ):
         # type: (...) -> None
         self.host = host
@@ -112,6 +113,7 @@ def __init__(
         self.logger = logger
         self.log_prefix = log_prefix
         self.allowed_log_levels = allowed_log_levels
+        self.retriable_client_errors = retriable_client_errors if retriable_client_errors else [408]
 
         self.prepare_new_session()
 
@@ -364,7 +366,7 @@ def _process(
                         attempt=run,
                     )
 
-                if self.is_server_error(error, status_code):
+                if self.is_server_error(error, status_code) or self.retry_on_client_errors(status_code):
                     if is_econnreset_error:
                         self.log("info", "{}.session_replace".format(request_category))
                         self.remove_session()
@@ -609,11 +611,23 @@ def is_server_error(error, http_code):
         if not isinstance(error, requests.exceptions.HTTPError):
             return True
 
-        if http_code is not None and (400 <= http_code < 500 and not http_code == 408):
+        if http_code is not None and (400 <= http_code < 500):
             return False
 
         return True
 
+    def retry_on_client_errors(self, http_code):
+        # type: (Optional[int]) -> bool
+        """Decide if retry should be done even on client http error.
+
+        :param int http_code: (optional) response HTTP status code
+        :return bool: whether retry should occur
+        """
+        if http_code is not None and http_code in self.retriable_client_errors:
+            return True
+
+        return False
+
     @staticmethod
     def get_response_text(response):
         # type: (Union[requests.Response, Any]) -> str

setup.py

@@ -13,7 +13,7 @@
 
 setup(
     name="request_session",
-    version="0.14.0",
+    version="0.15.0",
     url="https://github.com/kiwicom/request-session",
     description="Python HTTP requests on steroids",
     long_description=readme,

test/test_request_session.py

@@ -646,7 +646,7 @@ def test_reporting(request_session, mocker):
         (requests.exceptions.Timeout(), None, True),
         (requests.exceptions.HTTPError(), 400, False),
         (requests.exceptions.HTTPError(), 399, True),
-        (requests.exceptions.HTTPError(), 408, True),  # Timeout is server error
+        (requests.exceptions.HTTPError(), 408, False),
         (requests.exceptions.HTTPError(), 499, False),
         (requests.exceptions.HTTPError(), 500, True),
         (requests.exceptions.HTTPError(), None, True),
@@ -655,3 +655,22 @@ def test_reporting(request_session, mocker):
 def test_is_server_error(exception, status_code, expected):
     # type: (RequestException, Union[int, None], bool) -> None
     assert RequestSession.is_server_error(exception, status_code) == expected
+
+
+@pytest.mark.parametrize(
+    ("status_code, extended_retry_errors, expected"),
+    [
+        (None, [], False),
+        (408, [], True),  # Timeout is retried by default
+        (408, [429], False),
+        (408, [408, 429], True),
+        (200, [], False),
+        (429, [], False),
+        (429, [429], True),
+        (500, [], False),
+    ],
+)
+def test_retry_on_client_errors(status_code, extended_retry_errors, expected):
+    # type: (RequestException, Union[int, None], bool) -> None
+    session = RequestSession(retriable_client_errors=extended_retry_errors)
+    assert session.retry_on_client_errors(status_code) == expected