From fd5be04dbb4f638d5697da616ac8f00a276249e9 Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Mon, 6 Apr 2026 11:20:15 -0400 Subject: [PATCH] fix(bootstrapper): log specific error types in wheel cache lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace bare `except Exception` in `_download_wheel_from_cache()` with three handlers so logs distinguish why a cache lookup failed: - ResolverException → INFO (wheel not found) - RequestException → WARNING (network error) - Exception → WARNING (unexpected error, e.g. bad wheel file, I/O) All three still return (None, None) to preserve existing behavior. Closes: #1025 Co-Authored-By: Claude Signed-off-by: Lalatendu Mohanty --- src/fromager/bootstrapper.py | 16 +++- tests/test_bootstrapper.py | 141 +++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) diff --git a/src/fromager/bootstrapper.py b/src/fromager/bootstrapper.py index e18137f0..8f101dfb 100644 --- a/src/fromager/bootstrapper.py +++ b/src/fromager/bootstrapper.py @@ -14,9 +14,11 @@ import zipfile from urllib.parse import urlparse +import requests.exceptions from packaging.requirements import Requirement from packaging.utils import NormalizedName, canonicalize_name from packaging.version import Version +from resolvelib.resolvers import ResolverException from . import ( bootstrap_requirement_resolver, @@ -1051,11 +1053,23 @@ def _download_wheel_from_cache( req, resolved_version, cached_wheel ) return cached_wheel, unpack_dir - except Exception: + except ResolverException: logger.info( f"did not find wheel for {resolved_version} in {self.cache_wheel_server_url}" ) return None, None + except requests.exceptions.RequestException as err: + logger.warning( + f"network error checking wheel cache for {resolved_version} " + f"at {self.cache_wheel_server_url}: {err}" + ) + return None, None + except Exception as err: + logger.warning( + f"unexpected error checking wheel cache for {resolved_version} " + f"at {self.cache_wheel_server_url}: {err}" + ) + return None, None def _unpack_metadata_from_wheel( self, req: Requirement, resolved_version: Version, wheel_filename: pathlib.Path diff --git a/tests/test_bootstrapper.py b/tests/test_bootstrapper.py index 46b78c79..76295314 100644 --- a/tests/test_bootstrapper.py +++ b/tests/test_bootstrapper.py @@ -2,8 +2,11 @@ import pathlib from unittest.mock import Mock, patch +import pytest +import requests.exceptions from packaging.requirements import Requirement from packaging.version import Version +from resolvelib.resolvers import ResolverException from fromager import bootstrapper, requirements_file from fromager.context import WorkContext @@ -293,3 +296,141 @@ def test_build_from_source_returns_dataclass(tmp_context: WorkContext) -> None: assert result.sdist_root_dir == mock_sdist_root assert result.build_env is not None assert result.source_type == SourceType.SDIST + + +def _make_cache_bootstrapper( + tmp_context: WorkContext, +) -> bootstrapper.Bootstrapper: + bt = bootstrapper.Bootstrapper(tmp_context) + bt.cache_wheel_server_url = "https://cache.example.com/simple" + return bt + + +def test_cache_lookup_resolver_exception_logs_info( + tmp_context: WorkContext, + caplog: pytest.LogCaptureFixture, +) -> None: + """ResolverException (wheel not found) returns (None, None) and logs info.""" + bt = _make_cache_bootstrapper(tmp_context) + + with patch( + "fromager.resolver.resolve", + side_effect=ResolverException("no matching version"), + ): + result = bt._download_wheel_from_cache( + req=Requirement("test-package"), + resolved_version=Version("1.0.0"), + ) + + assert result == (None, None) + assert "did not find wheel for" in caplog.text + + +@pytest.mark.parametrize( + "exc_class,exc_msg", + [ + (requests.exceptions.ConnectionError, "DNS failure"), + (requests.exceptions.Timeout, "timed out"), + (requests.exceptions.HTTPError, "401 Unauthorized"), + ], +) +def test_cache_lookup_request_exception_logs_warning( + tmp_context: WorkContext, + caplog: pytest.LogCaptureFixture, + exc_class: type[Exception], + exc_msg: str, +) -> None: + """RequestException subtypes return (None, None) and log warning.""" + bt = _make_cache_bootstrapper(tmp_context) + + with patch( + "fromager.resolver.resolve", + side_effect=exc_class(exc_msg), + ): + result = bt._download_wheel_from_cache( + req=Requirement("test-package"), + resolved_version=Version("1.0.0"), + ) + + assert result == (None, None) + assert "network error checking wheel cache" in caplog.text + + +def test_cache_lookup_unexpected_exception_logs_warning( + tmp_context: WorkContext, + caplog: pytest.LogCaptureFixture, +) -> None: + """Unexpected exceptions return (None, None) and log warning.""" + bt = _make_cache_bootstrapper(tmp_context) + + with patch( + "fromager.resolver.resolve", + side_effect=ValueError("unexpected parsing error"), + ): + result = bt._download_wheel_from_cache( + req=Requirement("test-package"), + resolved_version=Version("1.0.0"), + ) + + assert result == (None, None) + assert "unexpected error checking wheel cache" in caplog.text + + +@pytest.mark.parametrize( + "exc_class,exc_msg,expected_log", + [ + ( + requests.exceptions.ConnectionError, + "connection reset", + "network error checking wheel cache", + ), + (OSError, "disk full", "unexpected error checking wheel cache"), + ], +) +def test_cache_lookup_download_wheel_error_logs_warning( + tmp_context: WorkContext, + caplog: pytest.LogCaptureFixture, + exc_class: type[Exception], + exc_msg: str, + expected_log: str, +) -> None: + """Errors from download_wheel (after resolve succeeds) are caught.""" + bt = _make_cache_bootstrapper(tmp_context) + + with ( + patch( + "fromager.resolver.resolve", + return_value=( + "https://cache.example.com/simple/test-package/test_package-1.0.0-py3-none-any.whl", + "1.0.0", + ), + ), + patch( + "fromager.bootstrapper.wheels.extract_info_from_wheel_file", + return_value=("test_package", "1.0.0", None, None), + ), + patch( + "fromager.bootstrapper.wheels.download_wheel", + side_effect=exc_class(exc_msg), + ), + ): + result = bt._download_wheel_from_cache( + req=Requirement("test-package"), + resolved_version=Version("1.0.0"), + ) + + assert result == (None, None) + assert expected_log in caplog.text + + +def test_cache_lookup_no_cache_url_returns_none(tmp_context: WorkContext) -> None: + """When no cache URL is configured, returns (None, None) immediately.""" + bt = bootstrapper.Bootstrapper(tmp_context) + bt.cache_wheel_server_url = "" + + result = bt._download_wheel_from_cache( + req=Requirement("test-package"), + resolved_version=Version("1.0.0"), + ) + + assert result == (None, None)