From c576e34e6db46f513661baa2e33163221d2107bf Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Sun, 25 Jan 2026 20:28:08 +0100 Subject: [PATCH 1/2] feat: retry upon ChunkedEncodingError --- craft_application/services/request.py | 36 ++++++-- tests/unit/services/test_request.py | 117 +++++++++++++++++++++++++- 2 files changed, 146 insertions(+), 7 deletions(-) diff --git a/craft_application/services/request.py b/craft_application/services/request.py index ced299e2d..4a8ab4f99 100644 --- a/craft_application/services/request.py +++ b/craft_application/services/request.py @@ -42,6 +42,7 @@ def __init__( super().__init__(app, services) self._session = requests.Session() self._session.headers["User-Agent"] = f"{self._app.name}/{self._app.version}" + self._max_retries = 3 # Passthroughs for requests methods so other services can use the session. self.request = self._session.request @@ -65,12 +66,35 @@ def download_chunks(self, url: str, dest: pathlib.Path) -> Iterator[int]: filename = util.get_filename_from_url_path(url) dest = dest / filename - with self.get(url, stream=True) as download: - with dest.open("wb") as file: - yield int(download.headers.get("Content-Length", -1)) - for chunk in download.iter_content(None): - file.write(chunk) - yield len(chunk) + content_length_yielded = False + + for attempt in range(self._max_retries): + downloaded_chunk_sizes = 0 + try: + with self.get(url, stream=True) as download: + with dest.open("wb") as file: + if not content_length_yielded: + content_length = int( + download.headers.get("Content-Length", -1) + ) + yield content_length + content_length_yielded = True + + # Download and track chunks + for chunk in download.iter_content(None): + file.write(chunk) + downloaded_chunk_sizes += len(chunk) + yield len(chunk) + break + except requests.exceptions.ChunkedEncodingError: + if attempt < self._max_retries - 1: + craft_cli.emit.progress( + f"Download interrupted, retrying... (attempt {attempt + 1}/{self._max_retries})" + ) + # Yield negative sum of chunk sizes to indicate rollback + yield -downloaded_chunk_sizes + else: + raise def download_with_progress(self, url: str, dest: pathlib.Path) -> pathlib.Path: """Download a single file with a progress bar.""" diff --git a/tests/unit/services/test_request.py b/tests/unit/services/test_request.py index 5b241f30a..6ce349e60 100644 --- a/tests/unit/services/test_request.py +++ b/tests/unit/services/test_request.py @@ -15,11 +15,12 @@ # along with this program. If not, see . """Unit tests for the Request service.""" -from unittest.mock import call +from unittest.mock import call, patch import craft_cli.pytest_plugin import pytest import pytest_check +import requests import responses from hypothesis import HealthCheck, given, settings, strategies @@ -114,3 +115,117 @@ def test_download_files_with_progress(tmp_path, emitter, request_service, downlo for url, path in results.items(): assert path.read_bytes() == downloads[url] + + +@responses.activate +def test_download_chunks_with_chunked_encoding_error_retry( + tmp_path, emitter, request_service +): + """Test that download_chunks retries on ChunkedEncodingError and eventually succeeds. + + This test simulates a ChunkedEncodingError occurring during download.iter_content(), + verifies that the download is retried, and ensures the final download completes + successfully with the correct data and chunk count (not counting failed attempts). + """ + data = b"This is test data for download retry" + output_file = tmp_path / "file" + + # Patch the get method to simulate ChunkedEncodingError on first attempts + original_get = request_service.get + call_count = {"count": 0} + + # Set up the mock response + responses.add( + responses.GET, + "http://example/file", + body=data, + headers={"Content-Length": str(len(data))}, + ) + + def patched_get(*args, **kwargs): + call_count["count"] += 1 + response = original_get(*args, **kwargs) + + # Make iter_content raise ChunkedEncodingError on first two attempts + if call_count["count"] <= 2: + + def failing_iter_content(chunk_size): + # Yield some data first to simulate partial download + yield b"partial" + # Then raise the error + raise requests.exceptions.ChunkedEncodingError( + "Connection broken: Invalid chunk encoding" + ) + + response.iter_content = failing_iter_content + + return response + + with patch.object(request_service, "get", side_effect=patched_get): + downloader = request_service.download_chunks("http://example/file", output_file) + size = next(downloader) + dl_size = sum(downloader) + + # Verify that the download eventually succeeded + pytest_check.equal(int(size), len(data), "Downloaded size is incorrect") + pytest_check.equal(dl_size, len(data), "Downloaded size is incorrect") + pytest_check.equal(output_file.read_bytes(), data, "Download data is incorrect") + + # Verify that retry messages were emitted + progress_calls = [ + interaction + for interaction in emitter.interactions + if len(interaction.args) > 0 + and interaction.args[0] == "progress" + and len(interaction.args) > 1 + and "retrying" in interaction.args[1] + ] + pytest_check.equal(len(progress_calls), 2, "Expected 2 retry progress messages") + + # Verify that we made 3 attempts (2 failures + 1 success) + pytest_check.equal(call_count["count"], 3, "Expected 3 download attempts") + + +@responses.activate +def test_download_chunks_chunked_encoding_error_exhausted(tmp_path, request_service): + """Test that download_chunks raises ChunkedEncodingError after max retries. + + This test simulates a persistent ChunkedEncodingError that occurs on every + download attempt, and verifies that after exhausting all retry attempts, + the error is properly raised to the caller. + """ + data = b"test data" + output_file = tmp_path / "file" + + # Set up the mock response + responses.add( + responses.GET, + "http://example/file", + body=data, + headers={"Content-Length": str(len(data))}, + ) + + # Patch to always fail + original_get = request_service.get + + def patched_get(*args, **kwargs): + response = original_get(*args, **kwargs) + + def failing_iter_content(chunk_size): + # Yield some data first + yield b"partial" + # Then raise the error + raise requests.exceptions.ChunkedEncodingError( + "Connection broken: Invalid chunk encoding" + ) + + response.iter_content = failing_iter_content + return response + + with patch.object(request_service, "get", side_effect=patched_get): + downloader = request_service.download_chunks("http://example/file", output_file) + next(downloader) # Get the size + + # The error should be raised after max_retries attempts + with pytest.raises(requests.exceptions.ChunkedEncodingError): + list(downloader) # Consume the iterator From 0d5f9cfb21009dfd79eab9cac7f1108ffbd6f96e Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Sun, 25 Jan 2026 20:56:37 +0100 Subject: [PATCH 2/2] chore: rename var and restructure test --- craft_application/services/request.py | 6 +++--- tests/unit/services/test_request.py | 27 ++++++++++----------------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/craft_application/services/request.py b/craft_application/services/request.py index 4a8ab4f99..23431039c 100644 --- a/craft_application/services/request.py +++ b/craft_application/services/request.py @@ -69,7 +69,7 @@ def download_chunks(self, url: str, dest: pathlib.Path) -> Iterator[int]: content_length_yielded = False for attempt in range(self._max_retries): - downloaded_chunk_sizes = 0 + downloaded_bytes = 0 try: with self.get(url, stream=True) as download: with dest.open("wb") as file: @@ -83,7 +83,7 @@ def download_chunks(self, url: str, dest: pathlib.Path) -> Iterator[int]: # Download and track chunks for chunk in download.iter_content(None): file.write(chunk) - downloaded_chunk_sizes += len(chunk) + downloaded_bytes += len(chunk) yield len(chunk) break except requests.exceptions.ChunkedEncodingError: @@ -92,7 +92,7 @@ def download_chunks(self, url: str, dest: pathlib.Path) -> Iterator[int]: f"Download interrupted, retrying... (attempt {attempt + 1}/{self._max_retries})" ) # Yield negative sum of chunk sizes to indicate rollback - yield -downloaded_chunk_sizes + yield -downloaded_bytes else: raise diff --git a/tests/unit/services/test_request.py b/tests/unit/services/test_request.py index 6ce349e60..efa4ffb6d 100644 --- a/tests/unit/services/test_request.py +++ b/tests/unit/services/test_request.py @@ -117,6 +117,16 @@ def test_download_files_with_progress(tmp_path, emitter, request_service, downlo assert path.read_bytes() == downloads[url] +def failing_iter_content(chunk_size=None): # pylint: disable=unused-argument + """Simulate a ChunkedEncodingError during iter_content().""" + # Yield some data first to simulate partial download + yield b"partial" + # Then raise the error + raise requests.exceptions.ChunkedEncodingError( + "Connection broken: Invalid chunk encoding" + ) + + @responses.activate def test_download_chunks_with_chunked_encoding_error_retry( tmp_path, emitter, request_service @@ -148,15 +158,6 @@ def patched_get(*args, **kwargs): # Make iter_content raise ChunkedEncodingError on first two attempts if call_count["count"] <= 2: - - def failing_iter_content(chunk_size): - # Yield some data first to simulate partial download - yield b"partial" - # Then raise the error - raise requests.exceptions.ChunkedEncodingError( - "Connection broken: Invalid chunk encoding" - ) - response.iter_content = failing_iter_content return response @@ -211,14 +212,6 @@ def test_download_chunks_chunked_encoding_error_exhausted(tmp_path, request_serv def patched_get(*args, **kwargs): response = original_get(*args, **kwargs) - def failing_iter_content(chunk_size): - # Yield some data first - yield b"partial" - # Then raise the error - raise requests.exceptions.ChunkedEncodingError( - "Connection broken: Invalid chunk encoding" - ) - response.iter_content = failing_iter_content return response