Skip to content

Commit fd5be04

Browse files
fix(bootstrapper): log specific error types in wheel cache lookup
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 <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
1 parent 9a24ab8 commit fd5be04

File tree

2 files changed

+156
-1
lines changed

2 files changed

+156
-1
lines changed

src/fromager/bootstrapper.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
import zipfile
1515
from urllib.parse import urlparse
1616

17+
import requests.exceptions
1718
from packaging.requirements import Requirement
1819
from packaging.utils import NormalizedName, canonicalize_name
1920
from packaging.version import Version
21+
from resolvelib.resolvers import ResolverException
2022

2123
from . import (
2224
bootstrap_requirement_resolver,
@@ -1051,11 +1053,23 @@ def _download_wheel_from_cache(
10511053
req, resolved_version, cached_wheel
10521054
)
10531055
return cached_wheel, unpack_dir
1054-
except Exception:
1056+
except ResolverException:
10551057
logger.info(
10561058
f"did not find wheel for {resolved_version} in {self.cache_wheel_server_url}"
10571059
)
10581060
return None, None
1061+
except requests.exceptions.RequestException as err:
1062+
logger.warning(
1063+
f"network error checking wheel cache for {resolved_version} "
1064+
f"at {self.cache_wheel_server_url}: {err}"
1065+
)
1066+
return None, None
1067+
except Exception as err:
1068+
logger.warning(
1069+
f"unexpected error checking wheel cache for {resolved_version} "
1070+
f"at {self.cache_wheel_server_url}: {err}"
1071+
)
1072+
return None, None
10591073

10601074
def _unpack_metadata_from_wheel(
10611075
self, req: Requirement, resolved_version: Version, wheel_filename: pathlib.Path

tests/test_bootstrapper.py

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22
import pathlib
33
from unittest.mock import Mock, patch
44

5+
import pytest
6+
import requests.exceptions
57
from packaging.requirements import Requirement
68
from packaging.version import Version
9+
from resolvelib.resolvers import ResolverException
710

811
from fromager import bootstrapper, requirements_file
912
from fromager.context import WorkContext
@@ -293,3 +296,141 @@ def test_build_from_source_returns_dataclass(tmp_context: WorkContext) -> None:
293296
assert result.sdist_root_dir == mock_sdist_root
294297
assert result.build_env is not None
295298
assert result.source_type == SourceType.SDIST
299+
300+
301+
def _make_cache_bootstrapper(
302+
tmp_context: WorkContext,
303+
) -> bootstrapper.Bootstrapper:
304+
bt = bootstrapper.Bootstrapper(tmp_context)
305+
bt.cache_wheel_server_url = "https://cache.example.com/simple"
306+
return bt
307+
308+
309+
def test_cache_lookup_resolver_exception_logs_info(
310+
tmp_context: WorkContext,
311+
caplog: pytest.LogCaptureFixture,
312+
) -> None:
313+
"""ResolverException (wheel not found) returns (None, None) and logs info."""
314+
bt = _make_cache_bootstrapper(tmp_context)
315+
316+
with patch(
317+
"fromager.resolver.resolve",
318+
side_effect=ResolverException("no matching version"),
319+
):
320+
result = bt._download_wheel_from_cache(
321+
req=Requirement("test-package"),
322+
resolved_version=Version("1.0.0"),
323+
)
324+
325+
assert result == (None, None)
326+
assert "did not find wheel for" in caplog.text
327+
328+
329+
@pytest.mark.parametrize(
330+
"exc_class,exc_msg",
331+
[
332+
(requests.exceptions.ConnectionError, "DNS failure"),
333+
(requests.exceptions.Timeout, "timed out"),
334+
(requests.exceptions.HTTPError, "401 Unauthorized"),
335+
],
336+
)
337+
def test_cache_lookup_request_exception_logs_warning(
338+
tmp_context: WorkContext,
339+
caplog: pytest.LogCaptureFixture,
340+
exc_class: type[Exception],
341+
exc_msg: str,
342+
) -> None:
343+
"""RequestException subtypes return (None, None) and log warning."""
344+
bt = _make_cache_bootstrapper(tmp_context)
345+
346+
with patch(
347+
"fromager.resolver.resolve",
348+
side_effect=exc_class(exc_msg),
349+
):
350+
result = bt._download_wheel_from_cache(
351+
req=Requirement("test-package"),
352+
resolved_version=Version("1.0.0"),
353+
)
354+
355+
assert result == (None, None)
356+
assert "network error checking wheel cache" in caplog.text
357+
358+
359+
def test_cache_lookup_unexpected_exception_logs_warning(
360+
tmp_context: WorkContext,
361+
caplog: pytest.LogCaptureFixture,
362+
) -> None:
363+
"""Unexpected exceptions return (None, None) and log warning."""
364+
bt = _make_cache_bootstrapper(tmp_context)
365+
366+
with patch(
367+
"fromager.resolver.resolve",
368+
side_effect=ValueError("unexpected parsing error"),
369+
):
370+
result = bt._download_wheel_from_cache(
371+
req=Requirement("test-package"),
372+
resolved_version=Version("1.0.0"),
373+
)
374+
375+
assert result == (None, None)
376+
assert "unexpected error checking wheel cache" in caplog.text
377+
378+
379+
@pytest.mark.parametrize(
380+
"exc_class,exc_msg,expected_log",
381+
[
382+
(
383+
requests.exceptions.ConnectionError,
384+
"connection reset",
385+
"network error checking wheel cache",
386+
),
387+
(OSError, "disk full", "unexpected error checking wheel cache"),
388+
],
389+
)
390+
def test_cache_lookup_download_wheel_error_logs_warning(
391+
tmp_context: WorkContext,
392+
caplog: pytest.LogCaptureFixture,
393+
exc_class: type[Exception],
394+
exc_msg: str,
395+
expected_log: str,
396+
) -> None:
397+
"""Errors from download_wheel (after resolve succeeds) are caught."""
398+
bt = _make_cache_bootstrapper(tmp_context)
399+
400+
with (
401+
patch(
402+
"fromager.resolver.resolve",
403+
return_value=(
404+
"https://cache.example.com/simple/test-package/test_package-1.0.0-py3-none-any.whl",
405+
"1.0.0",
406+
),
407+
),
408+
patch(
409+
"fromager.bootstrapper.wheels.extract_info_from_wheel_file",
410+
return_value=("test_package", "1.0.0", None, None),
411+
),
412+
patch(
413+
"fromager.bootstrapper.wheels.download_wheel",
414+
side_effect=exc_class(exc_msg),
415+
),
416+
):
417+
result = bt._download_wheel_from_cache(
418+
req=Requirement("test-package"),
419+
resolved_version=Version("1.0.0"),
420+
)
421+
422+
assert result == (None, None)
423+
assert expected_log in caplog.text
424+
425+
426+
def test_cache_lookup_no_cache_url_returns_none(tmp_context: WorkContext) -> None:
427+
"""When no cache URL is configured, returns (None, None) immediately."""
428+
bt = bootstrapper.Bootstrapper(tmp_context)
429+
bt.cache_wheel_server_url = ""
430+
431+
result = bt._download_wheel_from_cache(
432+
req=Requirement("test-package"),
433+
resolved_version=Version("1.0.0"),
434+
)
435+
436+
assert result == (None, None)

0 commit comments

Comments
 (0)