From b0e625f1a82ec3759acc9b629fa81305411f2060 Mon Sep 17 00:00:00 2001 From: Reshmi Aravind Date: Fri, 10 Jul 2026 11:58:14 +0200 Subject: [PATCH] Include dependency chain in resolution error messages (#1214) Resolution failures are logged with a package-name prefix derived from requirement_ctxvar, which reflects whatever package happens to be active in the logging context when the exception is finally reported - not necessarily the package whose requirement actually conflicted. When a transitive dependency several levels deep fails to resolve (e.g. its-hub -> reward-hub -> vllm -> flashinfer-python), the error message correctly names the failing package but the log prefix can show an unrelated top-level package, making it look like that top-level package is the culprit when the real conflict originates further down the chain. See #1242 for the log-prefix problem itself, tracked separately since it requires changes to shared logging infrastructure used by every bootstrap phase, not just resolution. WorkItem already tracks the full dependency chain via why_snapshot. _handle_phase_error() is the centralized handler for errors from every bootstrap phase, so a new _enrich_resolution_error() helper attaches that chain to any RESOLVE-phase failure with a non-empty why_snapshot, regardless of exception type, before test-mode/multiple-versions recording or the final raise. For example: Unable to resolve requirement specifier flashinfer-python==0.6.8.post1 with constraint flashinfer-python==0.6.11.post2 using PyPI resolver: found no match for flashinfer-python==0.6.8.post1 (dependency chain: its-hub==1.0 -> reward-hub==2.0 -> vllm==3.0 -> flashinfer-python==0.6.8.post1) type(err)(msg) isn't safe for arbitrary exception classes, so the helper uses explicit isinstance checks (ResolverException, RuntimeError) with a generic RuntimeError fallback for anything else. Top-level failures (no chain) and non-RESOLVE-phase failures are returned unchanged, preserving the original exception object's identity. The enriched exception is raised with no `from` clause, while still inside the original exception's active handler in _handle_phase_error(). Python sets its __context__ to the original exception (keeping it available for traceback/debugging) while leaving __cause__ unset, so __main__._format_exception() - which only follows __cause__ - doesn't print the original message a second time via "... because ...". _format_exception() itself is intentionally unchanged here: a related, pre-existing duplication bug in that formatter is independent of this feature and is tracked separately as #1243. Signed-off-by: Reshmi Aravind --- src/fromager/bootstrapper.py | 50 ++++++- tests/test_bootstrapper_iterative.py | 209 +++++++++++++++++++++++++++ 2 files changed, 258 insertions(+), 1 deletion(-) diff --git a/src/fromager/bootstrapper.py b/src/fromager/bootstrapper.py index 38a0bd5f..11a58bbb 100644 --- a/src/fromager/bootstrapper.py +++ b/src/fromager/bootstrapper.py @@ -2117,6 +2117,53 @@ def _dispatch_phase(self, item: WorkItem) -> list[WorkItem]: case _: raise ValueError(f"unexpected phase: {item.phase}") + def _dependency_chain_suffix(self, item: WorkItem) -> str: + """Describe the requirement chain that pulled in a failed dependency. + + Resolution failures are logged with whatever top-level package + happens to be active in the logging context at the time, which can + make an unrelated package look like the culprit when the actual + conflict is several levels down the dependency tree (see #1214). + This spells out the real chain, top-down, so the message is + accurate even when the log prefix is not. + + Callers should only invoke this when ``item.why_snapshot`` is + non-empty (i.e. not a top-level requirement). + """ + chain = " -> ".join( + f"{req.name}=={version}" for _, req, version in item.why_snapshot + ) + return f" (dependency chain: {chain} -> {item.req})" + + def _enrich_resolution_error(self, item: WorkItem, err: Exception) -> Exception: + """Attach the dependency chain to a RESOLVE-phase failure. + + Returns ``err`` unchanged for non-RESOLVE phases or top-level + requirements (no chain to add). Otherwise returns a *new* exception + of the same shape with the chain appended. The caller + (``_handle_phase_error()``) raises this new exception with no + `from` clause while still inside the original exception's active + handler; Python then sets the new exception's `__context__` to the + original one (keeping it available for traceback/debugging) while + leaving `__cause__` unset, so `__main__._format_exception()` - + which only follows `__cause__` - doesn't print the original message + a second time via "... because ...". See #1243 for the pre-existing, + independent duplication bug in that formatter. + + `type(err)(msg)` isn't guaranteed to work for every exception + class, so only the two shapes that actually reach this code path + are reconstructed directly; anything else falls back to a + `RuntimeError` that keeps the original type name in the message. + """ + if item.phase != BootstrapPhase.RESOLVE or not item.why_snapshot: + return err + msg = f"{err}{self._dependency_chain_suffix(item)}" + if isinstance(err, ResolverException): + return ResolverException(msg) + if isinstance(err, RuntimeError): + return RuntimeError(msg) + return RuntimeError(f"{type(err).__name__}: {msg}") + def _handle_phase_error( self, item: WorkItem, @@ -2129,6 +2176,7 @@ def _handle_phase_error( """ # Resolution failures: recoverable in test mode and multiple versions mode if item.phase == BootstrapPhase.RESOLVE: + err = self._enrich_resolution_error(item, err) if self.test_mode: self._record_test_mode_failure(item.req, None, err, "resolution") if self.multiple_versions: @@ -2144,7 +2192,7 @@ def _handle_phase_error( item.req, "unresolved", err, f"failed during {item.phase} phase" ) return [] - raise + raise err # Test mode: try prebuilt fallback for build-related phases if self.test_mode: diff --git a/tests/test_bootstrapper_iterative.py b/tests/test_bootstrapper_iterative.py index d4e75c52..3edef4f5 100644 --- a/tests/test_bootstrapper_iterative.py +++ b/tests/test_bootstrapper_iterative.py @@ -28,6 +28,7 @@ from packaging.requirements import Requirement from packaging.utils import canonicalize_name from packaging.version import Version +from resolvelib.resolvers import ResolverException from fromager import bootstrapper, build_environment from fromager.bootstrapper import BootstrapPhase, SourceBuildResult, WorkItem @@ -598,6 +599,145 @@ def test_routes_to_correct_handler( mock_method.assert_called_once_with(item) +class TestEnrichResolutionError: + """Tests for _enrich_resolution_error, used by _handle_phase_error.""" + + def _why_snapshot(self) -> list[tuple[RequirementType, Requirement, Version]]: + return [ + (RequirementType.TOP_LEVEL, Requirement("its-hub"), Version("1.0")), + (RequirementType.INSTALL, Requirement("reward-hub"), Version("2.0")), + (RequirementType.INSTALL, Requirement("vllm"), Version("3.0")), + ] + + def test_resolver_exception_gets_dependency_chain( + self, tmp_context: WorkContext + ) -> None: + """Regression test for #1214: name the whole chain, not just the + + top-level package. + """ + bt = bootstrapper.Bootstrapper(tmp_context) + item = _make_resolve_item( + req="flashinfer-python==0.6.8.post1", why_snapshot=self._why_snapshot() + ) + err = ResolverException("found no match") + + enriched = bt._enrich_resolution_error(item, err) + + assert isinstance(enriched, ResolverException) + assert str(enriched) == ( + "found no match (dependency chain: its-hub==1.0 -> " + "reward-hub==2.0 -> vllm==3.0 -> flashinfer-python==0.6.8.post1)" + ) + + def test_runtime_error_gets_dependency_chain( + self, tmp_context: WorkContext + ) -> None: + """Regression test for a gap in the earlier fix: a plain RuntimeError + + (e.g. "Could not resolve any versions for ...") also gets the chain, + not just ResolverException. + """ + bt = bootstrapper.Bootstrapper(tmp_context) + item = _make_resolve_item( + req="flashinfer-python==0.6.8.post1", why_snapshot=self._why_snapshot() + ) + err = RuntimeError("Could not resolve any versions for flashinfer-python") + + enriched = bt._enrich_resolution_error(item, err) + + assert isinstance(enriched, RuntimeError) + assert str(enriched) == ( + "Could not resolve any versions for flashinfer-python " + "(dependency chain: its-hub==1.0 -> reward-hub==2.0 -> " + "vllm==3.0 -> flashinfer-python==0.6.8.post1)" + ) + + def test_unknown_exception_type_falls_back_to_runtime_error( + self, tmp_context: WorkContext + ) -> None: + """`type(err)(msg)` isn't safe for arbitrary exception classes, so an + + unrecognized type is wrapped in a RuntimeError that keeps the + original type name visible, instead of risking a TypeError trying + to reconstruct it. + """ + bt = bootstrapper.Bootstrapper(tmp_context) + item = _make_resolve_item( + req="flashinfer-python==0.6.8.post1", why_snapshot=self._why_snapshot() + ) + err = ValueError("some unexpected failure") + + enriched = bt._enrich_resolution_error(item, err) + + assert isinstance(enriched, RuntimeError) + assert "ValueError: some unexpected failure" in str(enriched) + assert "dependency chain: its-hub==1.0" in str(enriched) + + def test_no_chain_returns_original_object_unchanged( + self, tmp_context: WorkContext + ) -> None: + """Top-level requirements have no chain, so the original exception + + object is returned as-is (identity preserved, nothing reconstructed). + """ + bt = bootstrapper.Bootstrapper(tmp_context) + item = _make_resolve_item() # default why_snapshot=[] + err = ResolverException("found no match") + + enriched = bt._enrich_resolution_error(item, err) + + assert enriched is err + + def test_non_resolve_phase_returns_original_object_unchanged( + self, tmp_context: WorkContext + ) -> None: + """Only RESOLVE-phase failures get a chain suffix.""" + bt = bootstrapper.Bootstrapper(tmp_context) + item = _make_build_item(phase=BootstrapPhase.BUILD) + item.why_snapshot = self._why_snapshot() + err = RuntimeError("build failed") + + enriched = bt._enrich_resolution_error(item, err) + + assert enriched is err + + def test_enriched_exception_chains_via_context_not_cause( + self, tmp_context: WorkContext + ) -> None: + """When actually raised (not just constructed), the enriched + + exception must carry the original as `__context__` (implicit + chaining, preserves the real traceback for `exc_info=True` + logging) while leaving `__cause__` unset, so + `__main__._format_exception()` - which only follows `__cause__` - + doesn't print the original message a second time via + "... because ...". See #1243 for that formatter's own, + independent duplication bug. + """ + from fromager import __main__ + + bt = bootstrapper.Bootstrapper(tmp_context) + item = _make_resolve_item( + req="flashinfer-python==0.6.8.post1", why_snapshot=self._why_snapshot() + ) + original = ResolverException("found no match") + + try: + raise original + except ResolverException as caught: + enriched = bt._enrich_resolution_error(item, caught) + try: + raise enriched + except ResolverException as raised: + final = raised + + assert final.__cause__ is None + assert final.__context__ is original + formatted = __main__._format_exception(final) + assert formatted.count("found no match") == 1 + + class TestHandlePhaseError: # -- RESOLVE phase errors -- @@ -647,6 +787,75 @@ def test_resolve_error_in_multiple_versions_mode_continues( assert key in bt._failed_versions assert bt._failed_versions[key] is err + def test_resolve_error_enriched_before_test_mode_record( + self, tmp_context: WorkContext + ) -> None: + """Gap identified in review: test mode must record the enriched + + message, not the raw one, so `failed_packages[].exception_message` + shows the real dependency chain. + """ + bt = bootstrapper.Bootstrapper(tmp_context, test_mode=True) + why_snapshot = [ + (RequirementType.TOP_LEVEL, Requirement("its-hub"), Version("1.0")), + ] + item = _make_resolve_item( + req="flashinfer-python==0.6.8.post1", why_snapshot=why_snapshot + ) + err = ResolverException("found no match") + + bt._handle_phase_error(item, err) + + assert len(bt.failed_packages) == 1 + assert ( + "dependency chain: its-hub==1.0" + in (bt.failed_packages[0]["exception_message"]) + ) + + def test_resolve_error_enriched_before_multiple_versions_record( + self, tmp_context: WorkContext + ) -> None: + """Gap identified in review: multiple-versions mode must record the + + enriched message too. + """ + bt = bootstrapper.Bootstrapper(tmp_context, multiple_versions=True) + why_snapshot = [ + (RequirementType.TOP_LEVEL, Requirement("its-hub"), Version("1.0")), + ] + item = _make_resolve_item( + req="flashinfer-python==0.6.8.post1", why_snapshot=why_snapshot + ) + err = ResolverException("found no match") + + bt._handle_phase_error(item, err) + + key = (canonicalize_name("flashinfer-python"), "unresolved") + assert "dependency chain: its-hub==1.0" in str(bt._failed_versions[key]) + + def test_resolve_error_enriched_before_normal_mode_raise( + self, tmp_context: WorkContext + ) -> None: + """Gap this feature originally targeted: a plain RuntimeError + + ("Could not resolve any versions...") also gets the chain in + normal (fail-fast) mode, not just ResolverException. + """ + bt = bootstrapper.Bootstrapper(tmp_context) + why_snapshot = [ + (RequirementType.TOP_LEVEL, Requirement("its-hub"), Version("1.0")), + ] + item = _make_resolve_item( + req="flashinfer-python==0.6.8.post1", why_snapshot=why_snapshot + ) + err = RuntimeError("Could not resolve any versions for flashinfer-python") + + with pytest.raises(RuntimeError, match=r"dependency chain: its-hub==1\.0"): + try: + raise err + except RuntimeError: + bt._handle_phase_error(item, err) + # -- Build phase errors in test mode -- def test_build_phase_test_mode_fallback_success(