Repository navigation
fix(cli): avoid Rust backtrace for test failures - #36851
Tiancheng-Xu wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new regression test should assert a couple of expected failure markers (eg. “error: Test failed” / “boom”) to avoid false positives where an unrelated failure would also omit “Stack backtrace:”.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adjusts Deno CLI’s top-level error rendering so ordinary failures (not panics) don’t print an anyhow-captured Rust backtrace when RUST_BACKTRACE=1, while still keeping useful error-chain context. This targets deno test failures that previously emitted irrelevant Stack backtrace: noise.
Changes:
- Switch
exit_for_errorto useanyhow’s alternateDisplayformatting ({error:#}) instead ofDebug({error:?}) for non-JS errors. - Add an integration regression test ensuring a failing
deno testrun withRUST_BACKTRACE=1does not includeStack backtrace:.
File summaries
| File | Description |
|---|---|
cli/lib.rs |
Changes CLI error formatting to avoid printing anyhow debug backtraces on normal errors. |
tests/integration/test_tests.rs |
Adds a regression test covering deno test failure output with RUST_BACKTRACE=1. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| output.assert_exit_code(1); | ||
| assert_not_contains!(output.combined_output(), "Stack backtrace:"); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Fixed in 09dcb649b. The regression now asserts error: Test failed and Error: boom before checking that Stack backtrace: is absent. Fresh validation: RUSTFLAGS="--cfg tokio_unstable" cargo test -p integration_tests --test integration -- test::test_failure_does_not_print_rust_backtrace — 1 passed.
|
The failed debug integration/spec jobs on the previous head exposed that alternate Fresh validation on the current implementation:
The new full CI is still running. Its current Linux and macOS ARM debug specs failures are both |
|
|
|
The remaining failing test on Could a maintainer rerun the failed Linux node-compat shard? I don't have permission to rerun the upstream workflow. I haven't changed the test or increased its timeout; a rerun would help distinguish a transient timeout from a reproducible failure before making a source change. |
With RUST_BACKTRACE=1, anyhow captures a backtrace for every error and its Debug output appends it, so an ordinary failure such as "error: Test failed" printed the CLI's own Rust frames, which say nothing about the failure. exit_for_error keeps the Debug layout (with its "Caused by:" chain) and drops only the captured backtrace suffix. Panics still print their backtrace. Squashed: 4 PR commits and a merge of main, as the net diff. (cherry picked from commit de093b9de64482df6317aaf46c262505c10db609 through d6abb273f76762f900eb90b8421fe58de38ccb24) Upstream: denoland/deno#36851
With RUST_BACKTRACE=1, anyhow captures a backtrace for every error and its Debug output appends it, so an ordinary failure such as "error: Test failed" printed the CLI's own Rust frames, which say nothing about the failure. exit_for_error keeps the Debug layout (with its "Caused by:" chain) and drops only the captured backtrace suffix. Panics still print their backtrace. Squashed: 4 PR commits and a merge of main, as the net diff. DENEXT.md item 415. Ported-from: theende/ende@4850e5c327 Upstream-Issue: denoland#36832 Upstream: denoland#36851 Co-authored-by: Tiancheng-Xu <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EAACL8TVEpJt2b5ywcrydw
With RUST_BACKTRACE=1, anyhow captures a backtrace for every error and its Debug output appends it, so an ordinary failure such as "error: Test failed" printed the CLI's own Rust frames, which say nothing about the failure. exit_for_error keeps the Debug layout (with its "Caused by:" chain) and drops only the captured backtrace suffix. Panics still print their backtrace. Squashed: 4 PR commits and a merge of main, as the net diff. DENEXT.md item 414. Ported-from: theende/ende@4850e5c327 Upstream-Issue: denoland#36832 Upstream: denoland#36851 Co-authored-by: Tiancheng-Xu <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EAACL8TVEpJt2b5ywcrydw
With RUST_BACKTRACE=1, anyhow captures a backtrace for every error and its Debug output appends it, so an ordinary failure such as "error: Test failed" printed the CLI's own Rust frames, which say nothing about the failure. exit_for_error keeps the Debug layout (with its "Caused by:" chain) and drops only the captured backtrace suffix. Panics still print their backtrace. Squashed: 4 PR commits and a merge of main, as the net diff. DENEXT.md item 414. Ported-from: theende/ende@4850e5c327 Upstream-Issue: denoland#36832 Upstream: denoland#36851 Co-authored-by: Tiancheng-Xu <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EAACL8TVEpJt2b5ywcrydw
Summary
deno testfailures #36832.RUST_BACKTRACE=1.deno testoutput before asserting thatStack backtrace:is absent.Root cause
exit_for_errorrendered non-JavaScript errors with anyhow Debug formatting. WhenRUST_BACKTRACE=1, anyhow appends its captured backtrace to that output. Using alternate Display formatting removed the backtrace but also collapsed the established multi-line error-chain layout, which broke existing CLI output expectations. The fix now preserves the Debug-rendered error chain and removes only the exact captured-backtrace suffix.Validation
RUSTFLAGS="--cfg tokio_unstable" cargo build -p denoRUSTFLAGS="--cfg tokio_unstable" cargo test -p integration_tests --test integration -- test::test_failure_does_not_print_rust_backtrace— 1 passedRUSTFLAGS="--cfg tokio_unstable" cargo test -p specs_tests --test specs -- fmt::malformed_config::fmt_with_malformed_config2— 1 passed, confirming the existing error-chain layout is preservedRUSTFLAGS="--cfg tokio_unstable" cargo clippy --manifest-path cli/Cargo.toml --bin deno --no-deps— completed with existing repository warningsrustfmt --edition 2024 --check cli/lib.rs tests/integration/test_tests.rsgit diff --checkThe new full CI run for the current head is still in progress. A separate local npm integration attempt could not complete because the test server could not download its pinned esbuild fixture from
raw.githubusercontent.com; it was not counted as passing.AI tools assisted with implementation and review. I verified the final changes and the validation results reported above.