Merge pull request #846 from andre-motta/fix-845-package-name-in-exception

fix: include package name in exception messages

Author: mergify[bot]

e2e/test_bootstrap_constraints.sh

@@ -28,7 +28,7 @@ fromager \
 pass=true
 
 # Check for log message that the override is loaded
-if ! grep -q "ERROR: Unable to resolve requirement specifier stevedore==5.2.0 with constraint stevedore==4.0.0" "$OUTDIR/bootstrap.log"; then
+if ! grep -q "ERROR.*stevedore: Unable to resolve requirement specifier stevedore==5.2.0 with constraint stevedore==4.0.0" "$OUTDIR/bootstrap.log"; then
   echo "FAIL: did not throw expected error when constraint and requirement conflict" 1>&2
   pass=false
 fi

src/fromager/__main__.py

@@ -275,7 +275,7 @@ def invoke_main() -> None:
             err,
             exc_info=True,
         )  # log the full traceback details to the debug log file, if any
-        logger.error(f"ERROR: {_format_exception(err)}")
+        logger.error(_format_exception(err))
         if _DEBUG:
             raise
         sys.exit(1)

src/fromager/commands/build.py

@@ -680,14 +680,15 @@ def update_progressbar_cb(future: concurrent.futures.Future) -> None:
                     try:
                         entry = future.result()
                     except Exception as e:
-                        logger.error("failed to build: %s", e)
-                        raise
+                        logger.error(f"Failed to build {node.key}: {e}")
+                        raise RuntimeError(f"Failed to build {node.key}") from e
                     else:
                         # success
                         built_entries.append(entry)
                     finally:
                         # mark node as done, progress bar is updated in callback.
                         topo.done(node)
+                        # Re-raise with package context since context var is lost across threads
 
     metrics.summarize(wkctx, "Building in parallel")
     _summary(wkctx, built_entries)

src/fromager/commands/download_sequence.py

@@ -83,7 +83,8 @@ def download_one(entry: dict[str, typing.Any]):
             except Exception as err:
                 logger.error(f"failed to download sdist for {req}: {err}")
                 if not ignore_missing_sdists:
-                    raise
+                    # Re-raise with package context since context var is lost across threads
+                    raise RuntimeError(f"Failed to download sdist for {req}") from err
         else:
             logger.info(
                 f"{entry['dist']}: uses a {entry['source_url_type']} downloader, skipping"

tests/test_external_commands.py

@@ -131,3 +131,48 @@ def test_external_command_output_prefix(caplog: pytest.LogCaptureFixture) -> Non
     finally:
         # Restore the original log record factory
         logging.setLogRecordFactory(old_factory)
+
+
+def test_external_commands_error_includes_package_name(caplog) -> None:
+    """Test that package name is included in error logs when context var is set"""
+    logging.setLogRecordFactory(log.FromagerLogRecord)
+
+    req = Requirement("test-package==1.0.0")
+    version = Version("1.0.0")
+
+    with log.req_ctxvar_context(req, version):
+        with caplog.at_level(logging.ERROR):
+            with pytest.raises(subprocess.CalledProcessError):
+                external_commands.run(["sh", "-c", "exit 1"])
+
+    error_logs = [
+        record.message for record in caplog.records if record.levelname == "ERROR"
+    ]
+    assert len(error_logs) > 0
+    assert any("test-package-1.0.0:" in msg for msg in error_logs), (
+        f"Expected package name in error logs, got: {error_logs}"
+    )
+
+
+def test_format_exception_formats_chained_exceptions() -> None:
+    """Test that _format_exception formats chained exceptions correctly"""
+    from fromager import __main__
+
+    # Test basic exception formatting
+    exception_without_cause = subprocess.CalledProcessError(
+        1, ["command"], output="some output"
+    )
+    message = __main__._format_exception(exception_without_cause)
+    assert "Command '['command']' returned non-zero exit status 1" in message
+
+    # Test chained exception formatting with "because"
+    try:
+        try:
+            raise ValueError("Root cause")
+        except ValueError as e:
+            raise RuntimeError("Higher level error") from e
+    except RuntimeError as chained_exc:
+        formatted = __main__._format_exception(chained_exc)
+        assert "Higher level error" in formatted
+        assert "because" in formatted
+        assert "Root cause" in formatted