Retry requests on ConnectionError and Timeout (#210)

Adds a retry logic to `send_post_request` and `send_put_request` to retry on ConnectionError and Timeout.
Also makes the legacy upload sender use those functions.

There might be a better way to do that without the custom logic, but the `Retry` option I saw you specify retry error codes,
and I'm not sure ECONNRESET gives an error code. But apparently it is a `requests.exceptions.COnnectionError`, so...

Fixes uploader #537

Author: giovanni-guidini

codecov_cli/helpers/request.py

@@ -1,24 +1,46 @@
 import logging
 import uuid
+from time import sleep
 
 import requests
 
 from codecov_cli.types import RequestError, RequestResult
 
 logger = logging.getLogger("codecovcli")
 
+MAX_RETRIES = 3
 
+
+def backoff_time(curr_retry):
+    return 2 ** (curr_retry - 1)
+
+
+def retry_request(func):
+    def wrapper(*args, **kwargs):
+        retry = 0
+        while retry < MAX_RETRIES:
+            try:
+                return func(*args, **kwargs)
+            except (
+                requests.exceptions.ConnectionError,
+                requests.exceptions.Timeout,
+            ) as exp:
+                logger.warning(
+                    "Request failed. Retrying",
+                    extra=dict(extra_log_attributes=dict(retry=retry)),
+                )
+                sleep(backoff_time(retry))
+                retry += 1
+        raise Exception("Request failed after too many retries")
+
+    return wrapper
+
+
+@retry_request
 def send_post_request(
-    url: str,
-    data: dict = None,
-    headers: dict = None,
+    url: str, data: dict = None, headers: dict = None, params: dict = None
 ):
-    resp = requests.post(
-        url=url,
-        json=data,
-        headers=headers,
-    )
-
+    resp = requests.post(url=url, json=data, headers=headers, params=params)
     return request_result(resp)
 
 
@@ -30,6 +52,7 @@ def get_token_header_or_fail(token: uuid.UUID) -> dict:
     return {"Authorization": f"token {token.hex}"}
 
 
+@retry_request
 def send_put_request(
     url: str,
     data: dict = None,

codecov_cli/services/upload/legacy_upload_sender.py

@@ -7,6 +7,7 @@
 
 from codecov_cli import __version__ as codecov_cli_version
 from codecov_cli.helpers.config import LEGACY_CODECOV_API_URL
+from codecov_cli.helpers.request import send_post_request, send_put_request
 from codecov_cli.types import UploadCollectionResult, UploadCollectionResultFile
 
 logger = logging.getLogger("codecovcli")
@@ -75,34 +76,16 @@ def send_upload_data(
             headers = {"X-Upload-Token": ""}
 
         upload_url = enterprise_url or LEGACY_CODECOV_API_URL
-        resp = requests.post(f"{upload_url}/upload/v4", headers=headers, params=params)
-
+        resp = send_post_request(
+            f"{upload_url}/upload/v4", headers=headers, params=params
+        )
         if resp.status_code >= 400:
-            return UploadSendingResult(
-                error=UploadSendingError(
-                    code=f"HTTP Error {resp.status_code}",
-                    description=resp.text,
-                    params={},
-                ),
-                warnings=[],
-            )
-
+            return resp
         result_url, put_url = resp.text.split("\n")
 
         reports_payload = self._generate_payload(upload_data, env_vars)
-        resp = requests.put(put_url, data=reports_payload)
-
-        if resp.status_code >= 400:
-            return UploadSendingResult(
-                error=UploadSendingError(
-                    code=f"HTTP Error {resp.status_code}",
-                    description=resp.text,
-                    params={},
-                ),
-                warnings=[],
-            )
-
-        return UploadSendingResult(error=None, warnings=[])
+        resp = send_put_request(put_url, data=reports_payload)
+        return resp
 
     def _generate_payload(
         self, upload_data: UploadCollectionResult, env_vars: typing.Dict[str, str]

tests/helpers/test_legacy_upload_sender.py

@@ -45,6 +45,18 @@ def mocked_legacy_upload_endpoint(mocked_responses):
     yield resp
 
 
+@pytest.fixture
+def mocked_legacy_upload_endpoint_too_many_fails(mocked_responses):
+    resp = responses.Response(
+        responses.POST,
+        "https://codecov.io/upload/v4",
+        body="https://resulturl.com\nhttps://puturl.com",
+        status=400,
+    )
+    for _ in range(4):
+        mocked_responses.add(resp)
+
+
 @pytest.fixture
 def mocked_storage_server(mocked_responses):
     resp = responses.Response(responses.PUT, "https://puturl.com", status=200)
@@ -116,9 +128,10 @@ def test_upload_sender_result_success(
         assert not sender.warnings
 
     def test_upload_sender_result_fail_post_400(
-        self, mocked_responses, mocked_legacy_upload_endpoint
+        self, mocked_responses, mocked_legacy_upload_endpoint, mocker
     ):
         mocked_legacy_upload_endpoint.status = 400
+        mocker.patch("codecov_cli.helpers.request.sleep")
 
         sender = LegacyUploadSender().send_upload_data(
             upload_collection, random_sha, random_token, {}, **named_upload_data

tests/helpers/test_request.py

@@ -1,15 +1,26 @@
 import uuid
 
 import pytest
+import requests
+from requests import Response
 
 from codecov_cli.helpers.request import (
     get_token_header_or_fail,
     log_warnings_and_errors_if_any,
 )
 from codecov_cli.helpers.request import logger as req_log
+from codecov_cli.helpers.request import request_result, send_post_request
 from codecov_cli.types import RequestError, RequestResult
 
 
+@pytest.fixture
+def valid_response():
+    valid_response = Response()
+    valid_response.status_code = 200
+    valid_response._content = b"response text"
+    return valid_response
+
+
 def test_log_error_no_raise(mocker):
     mock_log_error = mocker.patch.object(req_log, "error")
     error = RequestError(
@@ -54,3 +65,38 @@ def test_get_token_header_or_fail():
         get_token_header_or_fail(token)
 
     assert str(e.value) == f"Token must be UUID. Received {type(token)}"
+
+
+def test_request_retry(mocker, valid_response):
+    expected_response = request_result(valid_response)
+    mock_sleep = mocker.patch("codecov_cli.helpers.request.sleep")
+    mocker.patch.object(
+        requests,
+        "post",
+        side_effect=[
+            requests.exceptions.ConnectionError(),
+            requests.exceptions.Timeout(),
+            valid_response,
+        ],
+    )
+    resp = send_post_request("my_url")
+    assert resp == expected_response
+    mock_sleep.assert_called()
+
+
+def test_request_retry_too_many_errors(mocker):
+    mock_sleep = mocker.patch("codecov_cli.helpers.request.sleep")
+    mocker.patch.object(
+        requests,
+        "post",
+        side_effect=[
+            requests.exceptions.ConnectionError(),
+            requests.exceptions.Timeout(),
+            requests.exceptions.Timeout(),
+            requests.exceptions.Timeout(),
+            requests.exceptions.Timeout(),
+        ],
+    )
+    with pytest.raises(Exception) as exp:
+        resp = send_post_request("my_url")
+    assert str(exp.value) == "Request failed after too many retries"