Show the full cause chain at three error-flattening sites - #447
Open
tobert wants to merge 2 commits into
Open
Conversation
Problem: an `anyhow::Error` built with `.context(...)` shows only its
outermost frame under `{}`/`.to_string()`; `{:#}` walks the whole
chain. `Stmt::Test`'s fault arm already uses `{e:#}`, but its sibling
`Stmt::Arith` (bare `(( expr ))` as a statement), `timeout`'s
re-dispatch fault arm, and `eval_redirect_target`'s command-dispatch
evaluation still folded the error with the outermost-frame-only form,
permanently baking the terse text into `ExecResult.err` and losing the
real cause forever.
Evidence: `(( $(x=$((1/0))) ))` reported only "failed to evaluate
assignment" and hid "arithmetic error: `1 / 0` divides by zero"
underneath. The same pattern reproduced through
`echo hi > $(x=$((1/0)))` (redirect target evaluation) and through
`timeout 5 boom` where `boom` is a user tool whose body faults the
same way.
Decision: use `format!("{e:#}")` at all three sites, matching the
`Stmt::Test` precedent already in the codebase. Four new tests in
`error_cause_chain_tests.rs` pin the fix (red before, green after) and
one pins that the plain single-frame case (`(( 1/0 ))`) is unchanged.
Two other call sites named in the same sweep —
`scheduler::pipeline::run_single` and the concurrent pipeline stage
path, both `Err(e) => ExecResult::failure(1, e.to_string())` — are
left untouched: a sibling PR changes both to also keep the output that
ran before the fault, and touching them here would conflict.
Swept `to_string())`/`"{e}"` near `Err(e)` across
`crates/kaish-kernel/src` and confirmed every other candidate site
converts a flat, non-chaining error type (`EvalError`, `BackendError`,
`WalkerError`, `TrashError`, `std::io::Error`) whose Display never
drops a chain, so `{}` and `{:#}` are always identical there — no
change needed. `spawn::hermetic_env`'s callers
(`tools/wrapped.rs`) and `scheduler::pipeline::build_tool_args` do
wrap a real `anyhow::Error`, but neither ever gains a second
`.context()` frame in the current code, so there is no reachable input
that makes `{}` and `{:#}` differ — left unchanged rather than
"fixed" with no test able to prove it matters.
Also found: `y=$(echo inner; x=$((1/0)))` doubles the same context
text ("failed to evaluate assignment: failed to evaluate assignment:
arithmetic error: ..."), because `Stmt::Assignment`'s
`.context("failed to evaluate assignment")` (kernel.rs) fires once for
the inner `x=` inside the command substitution and again for the outer
`y=` around it — the same call site, same literal string, at two
nesting levels. A one-line fix (naming the variable in the context
text) would change the message `kernel_error_tests.rs` pins exactly
("failed to evaluate assignment") for the plain single-assignment
case, so it is reported here rather than changed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`{e:#}` states its own intent, as the existing `Stmt::Test` arm does
without a comment; the reasoning lives in the previous commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An
anyhow::Errorbuilt with.context(...)shows only its outermost frame under{}or.to_string();{:#}walks the whole chain. TheStmt::Testfault arm already used{e:#}, but three sites still folded a chained fault with the outer frame only, so the result kept "failed to evaluate assignment" and lost the cause:Each now formats with
{:#}and reportsarithmetic error:1 / 0divides by zero. A test per site fails before the change, and a fourth pins that the single-frame case(( 1/0 ))is unchanged.The other candidates in the sweep convert flat error types (
EvalError,BackendError,WalkerError,TrashError,std::io::Error) whoseDisplayhas no chain to drop, so they are unchanged. The two pipeline stage folds inrun_singleand the concurrent stage path are changed in the sibling PR that keeps a fault's prior output.Not changed here:
y=$(echo inner; x=$((1/0)))repeats the context,failed to evaluate assignment: failed to evaluate assignment: ..., because the same.context()fires for the inner and the outer assignment. Naming the variable in that context would change the messagekernel_error_testspins.This stacks on #445.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
🤖 Generated with Claude Code