From 1da13a054db772e1648cbea36d2f1344b1dc24af Mon Sep 17 00:00:00 2001 From: Gguidini Date: Wed, 4 Oct 2023 15:34:03 -0300 Subject: [PATCH] chore: handle internal `send_single_upload_put` exceptions Instead of failing (and raising) in the case of a request exception (in `send_single_upload_put`) we will now catch the exception, emit warning, and return safely. This is important because the aggregate of all tasks WILL fail if one of the requests fail. Also notice that `httpx.HTTPError` is the base error class for a number of request-related errors, so this covers many of the failure possibilities here --- .../services/staticanalysis/__init__.py | 45 ++++---- .../test_static_analysis_service.py | 101 +++++++++++++++++- 2 files changed, 126 insertions(+), 20 deletions(-) diff --git a/codecov_cli/services/staticanalysis/__init__.py b/codecov_cli/services/staticanalysis/__init__.py index aa2ab058..95175ae8 100644 --- a/codecov_cli/services/staticanalysis/__init__.py +++ b/codecov_cli/services/staticanalysis/__init__.py @@ -188,35 +188,44 @@ async def process_files( ) -async def send_single_upload_put(client, all_data, el): +async def send_single_upload_put(client, all_data, el) -> typing.Dict: retryable_statuses = (429,) presigned_put = el["raw_upload_location"] number_retries = 5 - for current_retry in range(number_retries): - response = await client.put( - presigned_put, data=json.dumps(all_data[el["filepath"]]) - ) - if response.status_code < 300: - return { - "status_code": response.status_code, - "filepath": el["filepath"], - "succeeded": True, - } - if response.status_code in retryable_statuses: - await asyncio.sleep(2**current_retry) + try: + for current_retry in range(number_retries): + response = await client.put( + presigned_put, data=json.dumps(all_data[el["filepath"]]) + ) + if response.status_code < 300: + return { + "status_code": response.status_code, + "filepath": el["filepath"], + "succeeded": True, + } + if response.status_code in retryable_statuses: + await asyncio.sleep(2**current_retry) + status_code = response.status_code + message_to_warn = response.text + exception = None + except httpx.HTTPError as exp: + status_code = None + exception = type(exp) + message_to_warn = str(exp) logger.warning( - "Unable to send data", + "Unable to send single_upload_put", extra=dict( extra_log_attributes=dict( - response=response.text, + message=message_to_warn, + exception=exception, filepath=el["filepath"], - url=presigned_put, - latest_status_code=response.status_code, + latest_status_code=status_code, ) ), ) return { - "status_code": response.status_code, + "status_code": status_code, + "exception": exception, "filepath": el["filepath"], "succeeded": False, } diff --git a/tests/services/static_analysis/test_static_analysis_service.py b/tests/services/static_analysis/test_static_analysis_service.py index ea443700..fae25226 100644 --- a/tests/services/static_analysis/test_static_analysis_service.py +++ b/tests/services/static_analysis/test_static_analysis_service.py @@ -2,12 +2,17 @@ from unittest.mock import MagicMock import click +import httpx import pytest import requests import responses from responses import matchers -from codecov_cli.services.staticanalysis import process_files, run_analysis_entrypoint +from codecov_cli.services.staticanalysis import ( + process_files, + run_analysis_entrypoint, + send_single_upload_put, +) from codecov_cli.services.staticanalysis.types import ( FileAnalysisRequest, FileAnalysisResult, @@ -67,7 +72,7 @@ def imap_side_effect(mapped_func, files): ) @pytest.mark.asyncio - async def test_static_analysis_service(self, mocker): + async def test_static_analysis_service_success(self, mocker): mock_file_finder = mocker.patch( "codecov_cli.services.staticanalysis.select_file_finder" ) @@ -143,6 +148,98 @@ async def side_effect(*args, **kwargs): "raw_upload_location": "http://storage-url", } + @pytest.mark.asyncio + async def test_send_single_upload_put_success(self, mocker): + mock_client = MagicMock() + + async def side_effect(presigned_put, data): + if presigned_put == "http://storage-url-001": + return httpx.Response(status_code=204) + + mock_client.put.side_effect = side_effect + + all_data = { + "file-001": {"some": "data", "id": "1"}, + "file-002": {"some": "data", "id": "2"}, + "file-003": {"some": "data", "id": "3"}, + } + + success_response = await send_single_upload_put( + mock_client, + all_data=all_data, + el={ + "filepath": "file-001", + "raw_upload_location": "http://storage-url-001", + }, + ) + assert success_response == { + "status_code": 204, + "filepath": "file-001", + "succeeded": True, + } + + @pytest.mark.asyncio + async def test_send_single_upload_put_fail_401(self, mocker): + mock_client = MagicMock() + + async def side_effect(presigned_put, data): + if presigned_put == "http://storage-url-002": + return httpx.Response(status_code=401) + + mock_client.put.side_effect = side_effect + + all_data = { + "file-001": {"some": "data", "id": "1"}, + "file-002": {"some": "data", "id": "2"}, + "file-003": {"some": "data", "id": "3"}, + } + + fail_401_response = await send_single_upload_put( + mock_client, + all_data=all_data, + el={ + "filepath": "file-002", + "raw_upload_location": "http://storage-url-002", + }, + ) + assert fail_401_response == { + "status_code": 401, + "exception": None, + "filepath": "file-002", + "succeeded": False, + } + + @pytest.mark.asyncio + async def test_send_single_upload_put_fail_exception(self, mocker): + mock_client = MagicMock() + + async def side_effect(presigned_put, data): + if presigned_put == "http://storage-url-003": + raise httpx.HTTPError("Some error occured in the request") + + mock_client.put.side_effect = side_effect + + all_data = { + "file-001": {"some": "data", "id": "1"}, + "file-002": {"some": "data", "id": "2"}, + "file-003": {"some": "data", "id": "3"}, + } + + fail_401_response = await send_single_upload_put( + mock_client, + all_data=all_data, + el={ + "filepath": "file-003", + "raw_upload_location": "http://storage-url-003", + }, + ) + assert fail_401_response == { + "status_code": None, + "exception": httpx.HTTPError, + "filepath": "file-003", + "succeeded": False, + } + @pytest.mark.asyncio @pytest.mark.parametrize( "finish_endpoint_response,expected",