Skip to content

Commit

Permalink
chore: handle internal send_single_upload_put exceptions
Browse files Browse the repository at this point in the history
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
  • Loading branch information
giovanni-guidini committed Oct 4, 2023
1 parent 40483bb commit 1da13a0
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 20 deletions.
45 changes: 27 additions & 18 deletions codecov_cli/services/staticanalysis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
101 changes: 99 additions & 2 deletions tests/services/static_analysis/test_static_analysis_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
)
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 1da13a0

Please sign in to comment.