Skip to content

Commit 2a5acda

Browse files
committed
fix: include package name in exception messages
Add package name context to exception formatting to identify which package failed during build errors. The package name is added by FromagerLogRecord when messages are logged, and _format_exception handles formatting chained exceptions with 'because' syntax. Improve parallel build error handling to preserve package context across thread boundaries using RuntimeError wrapper. Tests added to test_external_commands.py verify error logging and exception formatting include package names and handle chained exceptions. Update e2e test pattern to match new log format that includes package names. Closes: #845 Signed-off-by: Andre Lustosa <alustosa@redhat.com>
1 parent e1479d7 commit 2a5acda

5 files changed

Lines changed: 52 additions & 5 deletions

File tree

e2e/test_bootstrap_constraints.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ fromager \
2828
pass=true
2929

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

src/fromager/__main__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ def invoke_main() -> None:
275275
err,
276276
exc_info=True,
277277
) # log the full traceback details to the debug log file, if any
278-
logger.error(f"ERROR: {_format_exception(err)}")
278+
logger.error(_format_exception(err))
279279
if _DEBUG:
280280
raise
281281
sys.exit(1)

src/fromager/commands/build.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -680,14 +680,15 @@ def update_progressbar_cb(future: concurrent.futures.Future) -> None:
680680
try:
681681
entry = future.result()
682682
except Exception as e:
683-
logger.error("failed to build: %s", e)
684-
raise
683+
logger.error(f"Failed to build {node.key}: {e}")
684+
raise RuntimeError(f"Failed to build {node.key}") from e
685685
else:
686686
# success
687687
built_entries.append(entry)
688688
finally:
689689
# mark node as done, progress bar is updated in callback.
690690
topo.done(node)
691+
# Re-raise with package context since context var is lost across threads
691692

692693
metrics.summarize(wkctx, "Building in parallel")
693694
_summary(wkctx, built_entries)

src/fromager/commands/download_sequence.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ def download_one(entry: dict[str, typing.Any]):
8383
except Exception as err:
8484
logger.error(f"failed to download sdist for {req}: {err}")
8585
if not ignore_missing_sdists:
86-
raise
86+
# Re-raise with package context since context var is lost across threads
87+
raise RuntimeError(f"Failed to download sdist for {req}") from err
8788
else:
8889
logger.info(
8990
f"{entry['dist']}: uses a {entry['source_url_type']} downloader, skipping"

tests/test_external_commands.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,3 +131,48 @@ def test_external_command_output_prefix(caplog: pytest.LogCaptureFixture) -> Non
131131
finally:
132132
# Restore the original log record factory
133133
logging.setLogRecordFactory(old_factory)
134+
135+
136+
def test_external_commands_error_includes_package_name(caplog) -> None:
137+
"""Test that package name is included in error logs when context var is set"""
138+
logging.setLogRecordFactory(log.FromagerLogRecord)
139+
140+
req = Requirement("test-package==1.0.0")
141+
version = Version("1.0.0")
142+
143+
with log.req_ctxvar_context(req, version):
144+
with caplog.at_level(logging.ERROR):
145+
with pytest.raises(subprocess.CalledProcessError):
146+
external_commands.run(["sh", "-c", "exit 1"])
147+
148+
error_logs = [
149+
record.message for record in caplog.records if record.levelname == "ERROR"
150+
]
151+
assert len(error_logs) > 0
152+
assert any("test-package-1.0.0:" in msg for msg in error_logs), (
153+
f"Expected package name in error logs, got: {error_logs}"
154+
)
155+
156+
157+
def test_format_exception_formats_chained_exceptions() -> None:
158+
"""Test that _format_exception formats chained exceptions correctly"""
159+
from fromager import __main__
160+
161+
# Test basic exception formatting
162+
exception_without_cause = subprocess.CalledProcessError(
163+
1, ["command"], output="some output"
164+
)
165+
message = __main__._format_exception(exception_without_cause)
166+
assert "Command '['command']' returned non-zero exit status 1" in message
167+
168+
# Test chained exception formatting with "because"
169+
try:
170+
try:
171+
raise ValueError("Root cause")
172+
except ValueError as e:
173+
raise RuntimeError("Higher level error") from e
174+
except RuntimeError as chained_exc:
175+
formatted = __main__._format_exception(chained_exc)
176+
assert "Higher level error" in formatted
177+
assert "because" in formatted
178+
assert "Root cause" in formatted

0 commit comments

Comments
 (0)