Skip to content

Commit e6784f8

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 297ed76 commit e6784f8

2 files changed

Lines changed: 156 additions & 1 deletion

File tree

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,
@@ -1141,11 +1143,23 @@ def _download_wheel_from_cache(
11411143
req, resolved_version, cached_wheel
11421144
)
11431145
return cached_wheel, unpack_dir
1144-
except Exception:
1146+
except ResolverException:
11451147
logger.info(
11461148
f"did not find wheel for {resolved_version} in {self.cache_wheel_server_url}"
11471149
)
11481150
return None, None
1151+
except requests.exceptions.RequestException as err:
1152+
logger.warning(
1153+
f"network error checking wheel cache for {resolved_version} "
1154+
f"at {self.cache_wheel_server_url}: {err}"
1155+
)
1156+
return None, None
1157+
except Exception as err:
1158+
logger.warning(
1159+
f"unexpected error checking wheel cache for {resolved_version} "
1160+
f"at {self.cache_wheel_server_url}: {err}"
1161+
)
1162+
return None, None
11491163

11501164
def _unpack_metadata_from_wheel(
11511165
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,9 +2,12 @@
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.utils import canonicalize_name
79
from packaging.version import Version
10+
from resolvelib.resolvers import ResolverException
811

912
from fromager import bootstrapper, requirements_file
1013
from fromager.context import WorkContext
@@ -408,3 +411,141 @@ def test_download_wheel_from_cache_bypasses_hooks(
408411
constraints=tmp_context.constraints,
409412
)
410413
mock_find_all.assert_called_once_with(mock_provider, Requirement("test-pkg==1.0.0"))
414+
415+
416+
def _make_cache_bootstrapper(
417+
tmp_context: WorkContext,
418+
) -> bootstrapper.Bootstrapper:
419+
bt = bootstrapper.Bootstrapper(tmp_context)
420+
bt.cache_wheel_server_url = "https://cache.test/simple"
421+
return bt
422+
423+
424+
def test_cache_lookup_resolver_exception_logs_info(
425+
tmp_context: WorkContext,
426+
caplog: pytest.LogCaptureFixture,
427+
) -> None:
428+
"""ResolverException (wheel not found) returns (None, None) and logs info."""
429+
bt = _make_cache_bootstrapper(tmp_context)
430+
431+
with patch(
432+
"fromager.resolver.resolve",
433+
side_effect=ResolverException("no matching version"),
434+
):
435+
result = bt._download_wheel_from_cache(
436+
req=Requirement("test-package"),
437+
resolved_version=Version("1.0.0"),
438+
)
439+
440+
assert result == (None, None)
441+
assert "did not find wheel for" in caplog.text
442+
443+
444+
@pytest.mark.parametrize(
445+
"exc_class,exc_msg",
446+
[
447+
(requests.exceptions.ConnectionError, "DNS failure"),
448+
(requests.exceptions.Timeout, "timed out"),
449+
(requests.exceptions.HTTPError, "401 Unauthorized"),
450+
],
451+
)
452+
def test_cache_lookup_request_exception_logs_warning(
453+
tmp_context: WorkContext,
454+
caplog: pytest.LogCaptureFixture,
455+
exc_class: type[Exception],
456+
exc_msg: str,
457+
) -> None:
458+
"""RequestException subtypes return (None, None) and log warning."""
459+
bt = _make_cache_bootstrapper(tmp_context)
460+
461+
with patch(
462+
"fromager.resolver.resolve",
463+
side_effect=exc_class(exc_msg),
464+
):
465+
result = bt._download_wheel_from_cache(
466+
req=Requirement("test-package"),
467+
resolved_version=Version("1.0.0"),
468+
)
469+
470+
assert result == (None, None)
471+
assert "network error checking wheel cache" in caplog.text
472+
473+
474+
def test_cache_lookup_unexpected_exception_logs_warning(
475+
tmp_context: WorkContext,
476+
caplog: pytest.LogCaptureFixture,
477+
) -> None:
478+
"""Unexpected exceptions return (None, None) and log warning."""
479+
bt = _make_cache_bootstrapper(tmp_context)
480+
481+
with patch(
482+
"fromager.resolver.resolve",
483+
side_effect=ValueError("unexpected parsing error"),
484+
):
485+
result = bt._download_wheel_from_cache(
486+
req=Requirement("test-package"),
487+
resolved_version=Version("1.0.0"),
488+
)
489+
490+
assert result == (None, None)
491+
assert "unexpected error checking wheel cache" in caplog.text
492+
493+
494+
@pytest.mark.parametrize(
495+
"exc_class,exc_msg,expected_log",
496+
[
497+
(
498+
requests.exceptions.ConnectionError,
499+
"connection reset",
500+
"network error checking wheel cache",
501+
),
502+
(OSError, "disk full", "unexpected error checking wheel cache"),
503+
],
504+
)
505+
def test_cache_lookup_download_wheel_error_logs_warning(
506+
tmp_context: WorkContext,
507+
caplog: pytest.LogCaptureFixture,
508+
exc_class: type[Exception],
509+
exc_msg: str,
510+
expected_log: str,
511+
) -> None:
512+
"""Errors from download_wheel (after resolve succeeds) are caught."""
513+
bt = _make_cache_bootstrapper(tmp_context)
514+
515+
with (
516+
patch(
517+
"fromager.resolver.resolve",
518+
return_value=(
519+
"https://cache.test/simple/test-package/test_package-1.0.0-py3-none-any.whl",
520+
"1.0.0",
521+
),
522+
),
523+
patch(
524+
"fromager.bootstrapper.wheels.extract_info_from_wheel_file",
525+
return_value=("test_package", "1.0.0", None, None),
526+
),
527+
patch(
528+
"fromager.bootstrapper.wheels.download_wheel",
529+
side_effect=exc_class(exc_msg),
530+
),
531+
):
532+
result = bt._download_wheel_from_cache(
533+
req=Requirement("test-package"),
534+
resolved_version=Version("1.0.0"),
535+
)
536+
537+
assert result == (None, None)
538+
assert expected_log in caplog.text
539+
540+
541+
def test_cache_lookup_no_cache_url_returns_none(tmp_context: WorkContext) -> None:
542+
"""When no cache URL is configured, returns (None, None) immediately."""
543+
bt = bootstrapper.Bootstrapper(tmp_context)
544+
bt.cache_wheel_server_url = ""
545+
546+
result = bt._download_wheel_from_cache(
547+
req=Requirement("test-package"),
548+
resolved_version=Version("1.0.0"),
549+
)
550+
551+
assert result == (None, None)

0 commit comments

Comments
 (0)