Merge pull request #1904 from Sineaggi/retry-additional-http-transient-errors

nejch · web-flow · commit 0353bd4cceb3 · 2022-04-04T23:45:34.000+02:00
Retry additional http transient errors
diff --git a/gitlab/client.py b/gitlab/client.py
@@ -35,6 +35,8 @@
     "{source!r} to {target!r}"
 )
 
+RETRYABLE_TRANSIENT_ERROR_CODES = [500, 502, 503, 504] + list(range(520, 531))
+
 
 class Gitlab:
     """Represents a GitLab server connection.
@@ -675,30 +677,42 @@ def http_request(
         json, data, content_type = self._prepare_send_data(files, post_data, raw)
         opts["headers"]["Content-type"] = content_type
 
+        retry_transient_errors = kwargs.get(
+            "retry_transient_errors", self.retry_transient_errors
+        )
         cur_retries = 0
         while True:
-            result = self.session.request(
-                method=verb,
-                url=url,
-                json=json,
-                data=data,
-                params=params,
-                timeout=timeout,
-                verify=verify,
-                stream=streamed,
-                **opts,
-            )
+            try:
+                result = self.session.request(
+                    method=verb,
+                    url=url,
+                    json=json,
+                    data=data,
+                    params=params,
+                    timeout=timeout,
+                    verify=verify,
+                    stream=streamed,
+                    **opts,
+                )
+            except requests.ConnectionError:
+                if retry_transient_errors and (
+                    max_retries == -1 or cur_retries < max_retries
+                ):
+                    wait_time = 2**cur_retries * 0.1
+                    cur_retries += 1
+                    time.sleep(wait_time)
+                    continue
+
+                raise
 
             self._check_redirects(result)
 
             if 200 <= result.status_code < 300:
                 return result
 
-            retry_transient_errors = kwargs.get(
-                "retry_transient_errors", self.retry_transient_errors
-            )
             if (429 == result.status_code and obey_rate_limit) or (
-                result.status_code in [500, 502, 503, 504] and retry_transient_errors
+                result.status_code in RETRYABLE_TRANSIENT_ERROR_CODES
+                and retry_transient_errors
             ):
                 # Response headers documentation:
                 # https://docs.gitlab.com/ee/user/admin_area/settings/user_and_ip_rate_limits.html#response-headers
diff --git a/tests/unit/test_gitlab_http_methods.py b/tests/unit/test_gitlab_http_methods.py
@@ -3,6 +3,7 @@
 import responses
 
 from gitlab import GitlabHttpError, GitlabList, GitlabParsingError, RedirectError
+from gitlab.client import RETRYABLE_TRANSIENT_ERROR_CODES
 from tests.unit import helpers
 
 MATCH_EMPTY_QUERY_PARAMS = [responses.matchers.query_param_matcher({})]
@@ -51,7 +52,7 @@ def test_http_request_404(gl):
 
 
 @responses.activate
-@pytest.mark.parametrize("status_code", [500, 502, 503, 504])
+@pytest.mark.parametrize("status_code", RETRYABLE_TRANSIENT_ERROR_CODES)
 def test_http_request_with_only_failures(gl, status_code):
     url = "http://localhost/api/v4/projects"
     responses.add(
@@ -97,6 +98,37 @@ def request_callback(request):
     assert len(responses.calls) == calls_before_success
 
 
+@responses.activate
+def test_http_request_with_retry_on_method_for_transient_network_failures(gl):
+    call_count = 0
+    calls_before_success = 3
+
+    url = "http://localhost/api/v4/projects"
+
+    def request_callback(request):
+        nonlocal call_count
+        call_count += 1
+        status_code = 200
+        headers = {}
+        body = "[]"
+
+        if call_count >= calls_before_success:
+            return (status_code, headers, body)
+        raise requests.ConnectionError("Connection aborted.")
+
+    responses.add_callback(
+        method=responses.GET,
+        url=url,
+        callback=request_callback,
+        content_type="application/json",
+    )
+
+    http_r = gl.http_request("get", "/projects", retry_transient_errors=True)
+
+    assert http_r.status_code == 200
+    assert len(responses.calls) == calls_before_success
+
+
 @responses.activate
 def test_http_request_with_retry_on_class_for_transient_failures(gl_retry):
     call_count = 0
@@ -126,6 +158,37 @@ def request_callback(request: requests.models.PreparedRequest):
     assert len(responses.calls) == calls_before_success
 
 
+@responses.activate
+def test_http_request_with_retry_on_class_for_transient_network_failures(gl_retry):
+    call_count = 0
+    calls_before_success = 3
+
+    url = "http://localhost/api/v4/projects"
+
+    def request_callback(request: requests.models.PreparedRequest):
+        nonlocal call_count
+        call_count += 1
+        status_code = 200
+        headers = {}
+        body = "[]"
+
+        if call_count >= calls_before_success:
+            return (status_code, headers, body)
+        raise requests.ConnectionError("Connection aborted.")
+
+    responses.add_callback(
+        method=responses.GET,
+        url=url,
+        callback=request_callback,
+        content_type="application/json",
+    )
+
+    http_r = gl_retry.http_request("get", "/projects", retry_transient_errors=True)
+
+    assert http_r.status_code == 200
+    assert len(responses.calls) == calls_before_success
+
+
 @responses.activate
 def test_http_request_with_retry_on_class_and_method_for_transient_failures(gl_retry):
     call_count = 0
@@ -155,6 +218,39 @@ def request_callback(request):
     assert len(responses.calls) == 1
 
 
+@responses.activate
+def test_http_request_with_retry_on_class_and_method_for_transient_network_failures(
+    gl_retry,
+):
+    call_count = 0
+    calls_before_success = 3
+
+    url = "http://localhost/api/v4/projects"
+
+    def request_callback(request):
+        nonlocal call_count
+        call_count += 1
+        status_code = 200
+        headers = {}
+        body = "[]"
+
+        if call_count >= calls_before_success:
+            return (status_code, headers, body)
+        raise requests.ConnectionError("Connection aborted.")
+
+    responses.add_callback(
+        method=responses.GET,
+        url=url,
+        callback=request_callback,
+        content_type="application/json",
+    )
+
+    with pytest.raises(requests.ConnectionError):
+        gl_retry.http_request("get", "/projects", retry_transient_errors=False)
+
+    assert len(responses.calls) == 1
+
+
 def create_redirect_response(
     *, response: requests.models.Response, http_method: str, api_path: str
 ) -> requests.models.Response:
