Skip to content

Commit 3d0214e

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 3d0214e

2 files changed

Lines changed: 191 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,
@@ -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: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
import json
2+
import logging
23
import pathlib
34
from unittest.mock import Mock, patch
45

6+
import pytest
7+
import requests.exceptions
58
from packaging.requirements import Requirement
69
from packaging.version import Version
10+
from resolvelib.resolvers import ResolverException
711

812
from fromager import bootstrapper, requirements_file
913
from fromager.context import WorkContext
@@ -293,3 +297,175 @@ def test_build_from_source_returns_dataclass(tmp_context: WorkContext) -> None:
293297
assert result.sdist_root_dir == mock_sdist_root
294298
assert result.build_env is not None
295299
assert result.source_type == SourceType.SDIST
300+
301+
302+
class TestDownloadWheelFromCacheErrorHandling:
303+
"""Tests for _download_wheel_from_cache exception differentiation."""
304+
305+
def test_resolver_exception_returns_none(
306+
self, tmp_context: WorkContext, caplog: pytest.LogCaptureFixture
307+
) -> None:
308+
"""ResolverException (wheel not found) returns (None, None) and logs info."""
309+
bt = bootstrapper.Bootstrapper(tmp_context)
310+
bt.cache_wheel_server_url = "https://cache.example.com/simple"
311+
312+
with patch(
313+
"fromager.resolver.resolve",
314+
side_effect=ResolverException("no matching version"),
315+
):
316+
result = bt._download_wheel_from_cache(
317+
req=Requirement("test-package"),
318+
resolved_version=Version("1.0.0"),
319+
)
320+
321+
assert result == (None, None)
322+
log_record = next(
323+
r for r in caplog.records if "did not find wheel for" in r.message
324+
)
325+
assert log_record.levelno == logging.INFO
326+
327+
@pytest.mark.parametrize(
328+
"exc_class,exc_msg",
329+
[
330+
(requests.exceptions.ConnectionError, "DNS failure"),
331+
(requests.exceptions.Timeout, "timed out"),
332+
(requests.exceptions.HTTPError, "401 Unauthorized"),
333+
],
334+
)
335+
def test_request_exception_returns_none_with_warning(
336+
self,
337+
tmp_context: WorkContext,
338+
caplog: pytest.LogCaptureFixture,
339+
exc_class: type[Exception],
340+
exc_msg: str,
341+
) -> None:
342+
"""RequestException subtypes return (None, None) and log warning."""
343+
bt = bootstrapper.Bootstrapper(tmp_context)
344+
bt.cache_wheel_server_url = "https://cache.example.com/simple"
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+
log_record = next(
357+
r
358+
for r in caplog.records
359+
if "network error checking wheel cache" in r.message
360+
)
361+
assert log_record.levelno == logging.WARNING
362+
363+
def test_unexpected_exception_returns_none_with_warning(
364+
self, tmp_context: WorkContext, caplog: pytest.LogCaptureFixture
365+
) -> None:
366+
"""Unexpected exceptions (e.g. ValueError, OSError) return (None, None) and log warning."""
367+
bt = bootstrapper.Bootstrapper(tmp_context)
368+
bt.cache_wheel_server_url = "https://cache.example.com/simple"
369+
370+
with patch(
371+
"fromager.resolver.resolve",
372+
side_effect=ValueError("unexpected parsing error"),
373+
):
374+
result = bt._download_wheel_from_cache(
375+
req=Requirement("test-package"),
376+
resolved_version=Version("1.0.0"),
377+
)
378+
379+
assert result == (None, None)
380+
log_record = next(
381+
r
382+
for r in caplog.records
383+
if "unexpected error checking wheel cache" in r.message
384+
)
385+
assert log_record.levelno == logging.WARNING
386+
387+
def test_download_wheel_network_error_returns_none_with_warning(
388+
self, tmp_context: WorkContext, caplog: pytest.LogCaptureFixture
389+
) -> None:
390+
"""RequestException from download_wheel (after resolve succeeds) is caught."""
391+
bt = bootstrapper.Bootstrapper(tmp_context)
392+
bt.cache_wheel_server_url = "https://cache.example.com/simple"
393+
394+
with (
395+
patch(
396+
"fromager.resolver.resolve",
397+
return_value=(
398+
"https://cache.example.com/simple/test-package/test_package-1.0.0-py3-none-any.whl",
399+
"1.0.0",
400+
),
401+
),
402+
patch(
403+
"fromager.bootstrapper.wheels.extract_info_from_wheel_file",
404+
return_value=("test_package", "1.0.0", None, None),
405+
),
406+
patch(
407+
"fromager.bootstrapper.wheels.download_wheel",
408+
side_effect=requests.exceptions.ConnectionError("connection reset"),
409+
),
410+
):
411+
result = bt._download_wheel_from_cache(
412+
req=Requirement("test-package"),
413+
resolved_version=Version("1.0.0"),
414+
)
415+
416+
assert result == (None, None)
417+
log_record = next(
418+
r
419+
for r in caplog.records
420+
if "network error checking wheel cache" in r.message
421+
)
422+
assert log_record.levelno == logging.WARNING
423+
424+
def test_download_wheel_unexpected_error_returns_none_with_warning(
425+
self, tmp_context: WorkContext, caplog: pytest.LogCaptureFixture
426+
) -> None:
427+
"""Unexpected exception from download_wheel (after resolve succeeds) is caught."""
428+
bt = bootstrapper.Bootstrapper(tmp_context)
429+
bt.cache_wheel_server_url = "https://cache.example.com/simple"
430+
431+
with (
432+
patch(
433+
"fromager.resolver.resolve",
434+
return_value=(
435+
"https://cache.example.com/simple/test-package/test_package-1.0.0-py3-none-any.whl",
436+
"1.0.0",
437+
),
438+
),
439+
patch(
440+
"fromager.bootstrapper.wheels.extract_info_from_wheel_file",
441+
return_value=("test_package", "1.0.0", None, None),
442+
),
443+
patch(
444+
"fromager.bootstrapper.wheels.download_wheel",
445+
side_effect=OSError("disk full"),
446+
),
447+
):
448+
result = bt._download_wheel_from_cache(
449+
req=Requirement("test-package"),
450+
resolved_version=Version("1.0.0"),
451+
)
452+
453+
assert result == (None, None)
454+
log_record = next(
455+
r
456+
for r in caplog.records
457+
if "unexpected error checking wheel cache" in r.message
458+
)
459+
assert log_record.levelno == logging.WARNING
460+
461+
def test_no_cache_url_returns_none(self, tmp_context: WorkContext) -> None:
462+
"""When no cache URL is configured, returns (None, None) immediately."""
463+
bt = bootstrapper.Bootstrapper(tmp_context)
464+
bt.cache_wheel_server_url = ""
465+
466+
result = bt._download_wheel_from_cache(
467+
req=Requirement("test-package"),
468+
resolved_version=Version("1.0.0"),
469+
)
470+
471+
assert result == (None, None)

0 commit comments

Comments
 (0)