From 6bc186f5f3e887a119e1881a15cecb38c1108e38 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Sat, 12 Sep 2026 15:26:42 -0400 Subject: [PATCH] fix: a runtime fault keeps the output that ran before it `break`, `continue`, `return`, and `exit` already carry the output a block produced before they leave it. A runtime fault propagated as a bare `anyhow::Error` and dropped that output at every block on the way up: `echo left && x=$((1/0))` printed only the error, an `if` body lost what it printed, and a function that faulted became exit 1 with neither its output nor the real cause, since the stage fold used `to_string()`. A block that faults now wraps the error in a private carrier holding its accumulated output, merging with any output an inner block already attached: if and while conditions and bodies, for and case bodies, both sides of && and ||, function bodies, `source`, and `.kai` scripts. A command substitution keeps only stderr, because its stdout was captured, never printed. The carrier renders exactly as the error it wraps, so no message text changes. At the top level, a streaming caller receives the faulting statement's partial output through `on_output` before the `Err`. `KernelError::Execution` becomes `{ error, output }` so a non-streaming caller can read the same output; this breaks `Execution(e)` patterns. A pipeline stage that faults becomes a failed result holding the carried output and the `{:#}` cause chain. Co-Authored-By: DeepSeek V4 Flash Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 + crates/kaish-kernel/src/error.rs | 104 +++++++++++--- crates/kaish-kernel/src/kernel.rs | 136 +++++++++++++++--- crates/kaish-kernel/src/scheduler/pipeline.rs | 19 ++- .../tests/error_keeps_prior_output_tests.rs | 109 ++++++++++++++ .../kaish-kernel/tests/kernel_error_tests.rs | 2 +- docs/EMBEDDING.md | 13 +- 7 files changed, 347 insertions(+), 42 deletions(-) create mode 100644 crates/kaish-kernel/tests/error_keeps_prior_output_tests.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 917dedb7..04bc59b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ breaking entries are marked **BREAKING**. ### Fixed +- A runtime fault keeps the output that ran before it. `echo left && + x=$((1/0))` shows `left`, and a function that faults exits 1 with what it + printed and the full cause instead of only the outermost message. - `grep` now reserves exit 1 for "no lines matched". An invalid pattern, an unreadable file, or a missing pattern argument exits 2, so a caller cannot read a broken search as a negative answer. `diff` argument errors exit 2 to @@ -42,6 +45,9 @@ breaking entries are marked **BREAKING**. ### Changed +- **BREAKING** (`kaish-kernel`): `KernelError::Execution` is now + `Execution { error, output }`. `output` holds what ran before the fault; a + `KernelError::Execution(e)` pattern no longer compiles. - **BREAKING** (`kaish-tool-api`): `ToolCtx` is sealed. Tool authors receive a `ToolCtx` and never implement one, so this changes no supported use, but an out-of-tree implementation no longer compiles. diff --git a/crates/kaish-kernel/src/error.rs b/crates/kaish-kernel/src/error.rs index 5f338b0c..36bcc046 100644 --- a/crates/kaish-kernel/src/error.rs +++ b/crates/kaish-kernel/src/error.rs @@ -10,6 +10,7 @@ //! message text. use std::fmt; +use crate::interpreter::ExecResult; use crate::parser::ParseError; use crate::validator::ValidationIssue; @@ -65,9 +66,16 @@ pub enum KernelError { }, /// A statement started running and something failed partway through — - /// a builtin, the evaluator, dispatch, or an IO fault. Carries the - /// original error chain unchanged; `source()` and `{:#}` still walk it. - Execution(anyhow::Error), + /// a builtin, the evaluator, dispatch, or an IO fault. + Execution { + /// The original error chain, unchanged; `source()` and `{:#}` walk it. + error: anyhow::Error, + /// What the program produced before the fault: the statements that + /// ran, and the part of the faulting statement that ran (`left` in + /// `echo left && x=$((1/0))`), stdout and stderr. Empty when the + /// fault came first. Boxed so the error stays small on the `Ok` path. + output: Box, + }, } // Display is hand-written rather than derived because the derive would drop @@ -82,11 +90,11 @@ impl fmt::Display for KernelError { KernelError::Parse { message, .. } | KernelError::Validation { message, .. } => { f.write_str(message) } - KernelError::Execution(e) => { + KernelError::Execution { error, .. } => { if f.alternate() { - write!(f, "{e:#}") + write!(f, "{error:#}") } else { - write!(f, "{e}") + write!(f, "{error}") } } } @@ -99,7 +107,7 @@ impl std::error::Error for KernelError { // `anyhow::Error` is not itself a `std::error::Error`, so the // chain is reached through its own source rather than by // returning it directly. - KernelError::Execution(e) => e.source(), + KernelError::Execution { error, .. } => error.source(), _ => None, } } @@ -117,7 +125,40 @@ impl KernelError { /// A statement began running and faulted partway through /// ([`KernelError::Execution`]). The complement of [`Self::is_rejected`]. pub fn is_execution_failure(&self) -> bool { - matches!(self, KernelError::Execution(_)) + matches!(self, KernelError::Execution { .. }) + } +} + +/// An execution fault carrying the output its program produced before it. +/// +/// The interpreter propagates `anyhow::Error`; a block that faults wraps the +/// error in this so the output survives each `?` on the way up. +/// `classify_execute_error` reads it into [`KernelError::Execution`]'s +/// `output`. It renders exactly as the error it wraps, so no message changes. +pub(crate) struct FaultWithOutput { + pub(crate) output: ExecResult, + pub(crate) error: anyhow::Error, +} + +impl fmt::Debug for FaultWithOutput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.error, f) + } +} + +impl fmt::Display for FaultWithOutput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if f.alternate() { + write!(f, "{:#}", self.error) + } else { + write!(f, "{}", self.error) + } + } +} + +impl std::error::Error for FaultWithOutput { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.error.source() } } @@ -128,15 +169,22 @@ impl KernelError { /// validation) by boxing a `KernelError` into the `anyhow::Error` it returns; /// everything else it and the deeper interpreter propagate (`?` through /// `execute_stmt_flow`, `eval_expr_async`, dispatch, tool bodies, …) stays a -/// plain `anyhow::Error`, untouched. This is the one place that downcasts: it -/// recovers a tagged rejection when the chain carries one, and falls back to -/// [`KernelError::Execution`] for everything else. Every public `execute*` -/// method applies this at its own return, so the interpreter's internal -/// `Result` (`anyhow::Result`) never has to change shape. +/// plain `anyhow::Error`, possibly wrapped in a `FaultWithOutput`. This is the +/// one place that downcasts: it recovers a tagged rejection when the chain +/// carries one, and falls back to [`KernelError::Execution`] for everything +/// else. Every public `execute*` method applies this at its own return, so the +/// interpreter's internal `Result` (`anyhow::Result`) never has to change +/// shape. pub(crate) fn classify_execute_error(e: anyhow::Error) -> KernelError { match e.downcast::() { Ok(tagged) => tagged, - Err(e) => KernelError::Execution(e), + Err(error) => { + let output = error + .downcast_ref::() + .map(|carrier| carrier.output.clone()) + .unwrap_or_default(); + KernelError::Execution { error, output: Box::new(output) } + } } } @@ -145,6 +193,10 @@ pub(crate) fn classify_execute_error(e: anyhow::Error) -> KernelError { mod tests { use super::*; + fn execution(error: anyhow::Error) -> KernelError { + KernelError::Execution { error, output: Box::default() } + } + #[test] fn is_rejected_true_for_parse_and_validation() { let parse = KernelError::Parse { errors: Vec::new(), message: "parse error:\nx".into() }; @@ -158,7 +210,7 @@ mod tests { #[test] fn is_rejected_false_for_execution() { - let exec = KernelError::Execution(anyhow::anyhow!("boom")); + let exec = execution(anyhow::anyhow!("boom")); assert!(!exec.is_rejected()); assert!(exec.is_execution_failure()); } @@ -174,7 +226,25 @@ mod tests { #[test] fn classify_falls_back_to_execution_for_untagged_errors() { let classified = classify_execute_error(anyhow::anyhow!("some deep interpreter error")); - assert!(matches!(classified, KernelError::Execution(_))); + let KernelError::Execution { output, .. } = classified else { + panic!("untagged error must classify as Execution"); + }; + assert_eq!(output.text_out(), "", "an unwrapped error carries no output"); + } + + #[test] + fn classify_reads_output_through_added_context() { + let carrier = FaultWithOutput { + output: ExecResult::success("ran\n"), + error: anyhow::anyhow!("inner cause"), + }; + let wrapped = anyhow::Error::new(carrier).context("outer context"); + let classified = classify_execute_error(wrapped); + assert_eq!(format!("{classified:#}"), "outer context: inner cause"); + let KernelError::Execution { output, .. } = classified else { + panic!("carrier must classify as Execution"); + }; + assert_eq!(output.text_out(), "ran\n"); } #[test] @@ -183,7 +253,7 @@ mod tests { // `?` converts via anyhow's blanket `From`. fn as_anyhow() -> anyhow::Result<()> { fn fails() -> Result<(), KernelError> { - Err(KernelError::Execution(anyhow::anyhow!("boom"))) + Err(execution(anyhow::anyhow!("boom"))) } fails()?; Ok(()) diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index 59fe5fe1..f5056963 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -2560,7 +2560,23 @@ impl Kernel { // sites. It runs before `execute_stmt_flow`, so a held statement // has run *nothing*: no substitution, no redirect opened, no let flow_result = self.execute_stmt_flow(&stmt).await; - let flow = flow_result?; + let flow = match flow_result { + Ok(flow) => flow, + Err(error) => { + // Earlier statements already streamed; the faulting + // statement's partial output has not. + let mut partial = ExecResult::success(""); + partial.err = { + let mut receiver = self.stderr_receiver.lock().await; + receiver.drain_lossy() + }; + let error = with_prior_output(partial, error); + if let Some(carrier) = error.downcast_ref::() { + on_output(&carrier.output); + } + return Err(with_prior_output(std::mem::take(&mut result), error)); + } + }; // Drain any stderr written by pipeline stages during this statement. // This captures stderr from intermediate pipeline stages that would @@ -2760,7 +2776,8 @@ impl Kernel { let mut result = ExecResult::success(""); let cond_value = self .eval_condition_async(&if_stmt.condition, &mut result) - .await?; + .await + .map_err(|error| with_prior_output(std::mem::take(&mut result), error))?; let branch = if is_truthy(&cond_value) { &if_stmt.then_branch @@ -2769,7 +2786,13 @@ impl Kernel { }; for stmt in branch { - let flow = self.execute_stmt_flow(stmt).await?; + let flow = match self.execute_stmt_flow(stmt).await { + Ok(flow) => flow, + Err(error) => { + self.drain_stderr_into(&mut result).await; + return Err(with_prior_output(result, error)); + } + }; match flow { ControlFlow::Normal(r) => { // Drain BEFORE accumulating, as the `while` arm @@ -2908,9 +2931,12 @@ impl Kernel { let mut flow = match self.execute_stmt_flow(stmt).await { Ok(f) => f, Err(e) => { - let mut scope = self.scope.write().await; - scope.pop_frame(); - return Err(e); + { + let mut scope = self.scope.write().await; + scope.pop_frame(); + } + self.drain_stderr_into(&mut result).await; + return Err(with_prior_output(result, e)); } }; self.drain_stderr_into(&mut result).await; @@ -2994,7 +3020,8 @@ impl Kernel { // the body's rather than arriving in one block up front. let cond_value = self .eval_condition_async(&while_loop.condition, &mut result) - .await?; + .await + .map_err(|error| with_prior_output(std::mem::take(&mut result), error))?; if !is_truthy(&cond_value) { break; @@ -3002,7 +3029,13 @@ impl Kernel { // Execute body for stmt in &while_loop.body { - let mut flow = self.execute_stmt_flow(stmt).await?; + let mut flow = match self.execute_stmt_flow(stmt).await { + Ok(flow) => flow, + Err(error) => { + self.drain_stderr_into(&mut result).await; + return Err(with_prior_output(result, error)); + } + }; self.drain_stderr_into(&mut result).await; match &mut flow { ControlFlow::Normal(r) => { @@ -3074,7 +3107,13 @@ impl Kernel { // Execute the branch body let mut result = ExecResult::success(""); for stmt in &branch.body { - let flow = self.execute_stmt_flow(stmt).await?; + let flow = match self.execute_stmt_flow(stmt).await { + Ok(flow) => flow, + Err(error) => { + self.drain_stderr_into(&mut result).await; + return Err(with_prior_output(result, error)); + } + }; match flow { ControlFlow::Normal(r) => { accumulate_result(&mut result, &r); @@ -3167,13 +3206,22 @@ impl Kernel { // value becomes the chain's value, so nothing // consumes it as a boolean and it reports exit 2. if left_result.fault { - return Err(anyhow::anyhow!("{}", left_result.err.trim_end())); + // The fault's stderr is its message; its stdout already ran. + let message = std::mem::take(&mut left_result.err); + return Err(with_prior_output( + left_result, + anyhow::anyhow!("{}", message.trim_end()), + )); } // Pending is not failure (spec §I.5) — see the // `OrChain` twin. The stash check matters here for a // hold swallowed into an apparent success below. if left_result.ok() { - let right_flow = self.execute_stmt_flow(right).await?; + let right_flow = match self.execute_stmt_flow(right).await { + Ok(flow) => flow, + // The left side already ran and printed. + Err(error) => return Err(with_prior_output(left_result, error)), + }; match right_flow { ControlFlow::Normal(mut right_result) => { self.drain_stderr_into(&mut right_result).await; @@ -3229,7 +3277,12 @@ impl Kernel { // value becomes the chain's value, so nothing // consumes it as a boolean and it reports exit 2. if left_result.fault { - return Err(anyhow::anyhow!("{}", left_result.err.trim_end())); + // The fault's stderr is its message; its stdout already ran. + let message = std::mem::take(&mut left_result.err); + return Err(with_prior_output( + left_result, + anyhow::anyhow!("{}", message.trim_end()), + )); } // Pending is not failure (spec §I.5): a fallback // written for failure must not run on a decision @@ -3243,7 +3296,11 @@ impl Kernel { // slot's result instead. Do not "fix" this by taking // the slot here: only statement boundaries take it. if !left_result.ok() { - let right_flow = self.execute_stmt_flow(right).await?; + let right_flow = match self.execute_stmt_flow(right).await { + Ok(flow) => flow, + // The left side already ran and printed. + Err(error) => return Err(with_prior_output(left_result, error)), + }; match right_flow { ControlFlow::Normal(mut right_result) => { self.drain_stderr_into(&mut right_result).await; @@ -5049,7 +5106,9 @@ impl Kernel { // 5. Propagate error or exit after cleanup if let Some(e) = exec_error { - return Err(e); + let mut prior = ExecResult::success_text_or_bytes(accumulated_out); + prior.err = accumulated_err; + return Err(with_prior_output(prior, e)); } let code = exit_code.unwrap_or(last_code); let mut result = ExecResult::success_text_or_bytes(accumulated_out).with_code(code); @@ -5144,7 +5203,17 @@ impl Kernel { } for stmt in stmts { - let flow = self.execute_stmt_flow(stmt).await?; + let flow = match self.execute_stmt_flow(stmt).await { + Ok(flow) => flow, + Err(error) => { + let drained = { + let mut receiver = self.stderr_receiver.lock().await; + receiver.drain_lossy() + }; + accumulated_err.push_str(&drained); + return Err(fault_leaving_capture(accumulated_err, error)); + } + }; // Drain pipeline stderr after each sub-statement (incremental, like // the control-structure and function-body executors). @@ -5607,7 +5676,9 @@ impl Kernel { } } Err(e) => { - return Err(e.context(format!("source: {}", path))); + let mut prior = ExecResult::success_text_or_bytes(accumulated_out); + prior.err = accumulated_err; + return Err(with_prior_output(prior, e).context(format!("source: {}", path))); } } } @@ -5808,7 +5879,9 @@ impl Kernel { // Propagate error or exit after cleanup if let Some(e) = exec_error { - return Err(e.context(format!("script: {}", script_path.display()))); + let mut prior = ExecResult::success_text_or_bytes(accumulated_out); + prior.err = accumulated_err; + return Err(with_prior_output(prior, e).context(format!("script: {}", script_path.display()))); } let code = exit_code.unwrap_or(last_code); let mut result = ExecResult::success_text_or_bytes(accumulated_out).with_code(code); @@ -7631,6 +7704,35 @@ fn accumulate_result(accumulated: &mut ExecResult, new: &ExecResult) { accumulated.baggage.clone_from(&new.baggage); } +/// Attach output a block produced before `error` to the error on its way up. +/// +/// The fault counterpart of `fold_block_output_into_flow`: a fault stops the +/// block; it does not unprint what already ran. Output the error already +/// carries ran later, inside the faulting statement, so it follows `prior`. +fn with_prior_output(prior: ExecResult, mut error: anyhow::Error) -> anyhow::Error { + if let Some(carrier) = error.downcast_mut::() { + let mut merged = prior; + accumulate_result(&mut merged, &carrier.output); + carrier.output = merged; + return error; + } + if prior.text_out().is_empty() && prior.err.is_empty() && prior.out_bytes().is_none() { + return error; + } + anyhow::Error::new(crate::error::FaultWithOutput { output: prior, error }) +} + +/// A command substitution captures stdout rather than printing it, so a fault +/// leaving one keeps only stderr: the block's own and what the error carries. +fn fault_leaving_capture(captured_err: String, mut error: anyhow::Error) -> anyhow::Error { + if let Some(carrier) = error.downcast_mut::() { + carrier.output.clear_stdout(); + } + let mut prior = ExecResult::success(""); + prior.err = captured_err; + with_prior_output(prior, error) +} + /// Fold a block's accumulated output into a signal that is leaving the block. /// /// Any block that builds up a result — a loop body, an `if`/`case` branch, the diff --git a/crates/kaish-kernel/src/scheduler/pipeline.rs b/crates/kaish-kernel/src/scheduler/pipeline.rs index 03a182d9..39ee373e 100644 --- a/crates/kaish-kernel/src/scheduler/pipeline.rs +++ b/crates/kaish-kernel/src/scheduler/pipeline.rs @@ -74,6 +74,21 @@ fn finalize_scatter_gather_error(result: ExecResult, format: Option ExecResult { + let mut result = error + .downcast_ref::() + .map(|carrier| carrier.output.clone()) + .unwrap_or_default(); + result.code = 1; + if !result.err.is_empty() && !result.err.ends_with('\n') { + result.err.push('\n'); + } + result.err.push_str(&ExecResult::terminate_diagnostic(format!("{error:#}"))); + result +} + /// Apply redirects to an execution result. /// /// Pre-execution redirects (Stdin, HereDoc) should be handled before calling. @@ -593,7 +608,7 @@ impl PipelineRunner { // Execute via dispatcher (full resolution chain) let result = match dispatch_stage(stage, ctx, dispatcher).await { Ok(result) => result, - Err(e) => ExecResult::failure(1, e.to_string()), + Err(e) => fault_result(e), }; // Apply post-execution redirects @@ -743,7 +758,7 @@ impl PipelineRunner { // Execute the stage let mut result = match dispatch_stage(&stage, &mut stage_ctx, &*task_dispatcher).await { Ok(result) => result, - Err(e) => ExecResult::failure(1, e.to_string()), + Err(e) => fault_result(e), }; // Apply post-execution redirects. Use the stage's own diff --git a/crates/kaish-kernel/tests/error_keeps_prior_output_tests.rs b/crates/kaish-kernel/tests/error_keeps_prior_output_tests.rs new file mode 100644 index 00000000..c0cd0936 --- /dev/null +++ b/crates/kaish-kernel/tests/error_keeps_prior_output_tests.rs @@ -0,0 +1,109 @@ +//! A runtime fault stops a program; it does not unprint what already ran. +//! +//! `break`, `continue`, `return`, and `exit` already carry the output a block +//! produced before they left it. A runtime fault (`x=$((1/0))`) propagated as +//! a bare error and dropped that output, so `echo left && x=$((1/0))` printed +//! nothing but the error. `KernelError::Execution { output, .. }` now holds +//! the output that ran, and a streaming caller receives it before the `Err`. +//! +//! Only builtins run here, so these need no feature gate. + +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use kaish_kernel::interpreter::ExecResult; +use kaish_kernel::{ExecuteOptions, Kernel, KernelError}; + +/// Run `program`, which must fault while running; return the error's output +/// and its full cause text. +async fn fault(program: &str) -> (ExecResult, String) { + let kernel = Kernel::transient().expect("kernel"); + let err = kernel.execute(program).await.expect_err("program must fault while running"); + let cause = format!("{err:#}"); + let KernelError::Execution { output, .. } = err else { + panic!("expected an execution fault, got {err:?}"); + }; + (*output, cause) +} + +#[tokio::test] +async fn statement_before_the_fault_is_kept() { + let (output, cause) = fault("echo first; x=$((1/0))").await; + assert_eq!(output.text_out(), "first\n"); + assert!(cause.contains("divides by zero"), "{cause}"); +} + +#[tokio::test] +async fn left_side_of_an_and_chain_is_kept() { + let (output, _) = fault("echo left && x=$((1/0))").await; + assert_eq!(output.text_out(), "left\n"); +} + +#[tokio::test] +async fn earlier_statements_and_the_faulting_statement_are_both_kept() { + let (output, _) = fault("echo first; echo left && x=$((1/0)); echo never").await; + assert_eq!(output.text_out(), "first\nleft\n"); +} + +#[tokio::test] +async fn if_body_output_is_kept() { + let (output, _) = fault("if true; then echo in-if; x=$((1/0)); fi").await; + assert_eq!(output.text_out(), "in-if\n"); +} + +#[tokio::test] +async fn loop_iterations_are_kept() { + let (output, _) = fault("for i in 1 2 3; do echo $i; if [[ $i == 2 ]]; then x=$((1/0)); fi; done").await; + assert_eq!(output.text_out(), "1\n2\n"); +} + +#[tokio::test] +async fn function_body_output_is_kept_in_its_failed_result() { + // A command that faults becomes a failed result (exit 1), and the script + // goes on; the result keeps what the function printed and names the cause. + let kernel = Kernel::transient().expect("kernel"); + let result = kernel + .execute("f() { echo in-f; x=$((1/0)); }; f") + .await + .expect("a faulting command is a failed result, not an error"); + assert_eq!(result.code, 1, "{result:?}"); + assert_eq!(result.text_out(), "in-f\n"); + assert!(result.err.contains("divides by zero"), "{:?}", result.err); +} + +#[tokio::test] +async fn stderr_before_the_fault_is_kept() { + let (output, _) = fault("echo early >&2 && x=$((1/0))").await; + assert_eq!(output.err, "early\n"); +} + +#[tokio::test] +async fn a_fault_before_any_output_carries_empty_output() { + // Control: nothing ran before the fault, so there is nothing to keep. + let (output, cause) = fault("x=$((1/0))").await; + assert_eq!(output.text_out(), ""); + assert!(cause.contains("divides by zero"), "{cause}"); +} + +#[tokio::test] +async fn streaming_caller_receives_the_faulting_statement_output() { + let kernel = Kernel::transient().expect("kernel"); + let mut streamed = String::new(); + let mut on_output = |r: &ExecResult| streamed.push_str(&r.text_out()); + let err = kernel + .execute_with_options_streaming("echo first; echo left && x=$((1/0))", ExecuteOptions::new(), &mut on_output) + .await + .expect_err("program must fault"); + assert!(err.is_execution_failure()); + assert_eq!(streamed, "first\nleft\n", "the stream must show what ran before the fault"); +} + +#[tokio::test] +async fn display_is_unchanged_by_the_carried_output() { + let kernel = Kernel::transient().expect("kernel"); + let err = kernel.execute("echo left && x=$((1/0))").await.expect_err("fault"); + assert_eq!(err.to_string(), "failed to evaluate assignment"); + let alternate = format!("{err:#}"); + assert!(alternate.starts_with("failed to evaluate assignment: "), "{alternate}"); + assert!(alternate.contains("divides by zero"), "{alternate}"); + assert!(!alternate.contains("left"), "output must not leak into the error text: {alternate}"); +} diff --git a/crates/kaish-kernel/tests/kernel_error_tests.rs b/crates/kaish-kernel/tests/kernel_error_tests.rs index 733692e1..c3c80e23 100644 --- a/crates/kaish-kernel/tests/kernel_error_tests.rs +++ b/crates/kaish-kernel/tests/kernel_error_tests.rs @@ -72,7 +72,7 @@ async fn arithmetic_division_by_zero_is_matchable_as_failed_while_running() { assert!(err.is_execution_failure(), "a runtime fault must be classified as an execution failure: {err:?}"); assert!(!err.is_rejected()); - let KernelError::Execution(inner) = err else { + let KernelError::Execution { error: inner, .. } = err else { panic!("division by zero must be KernelError::Execution"); }; // `Display` shows only the outermost `.context(...)` — unchanged from diff --git a/docs/EMBEDDING.md b/docs/EMBEDDING.md index 4b03d06c..b6d85c40 100644 --- a/docs/EMBEDDING.md +++ b/docs/EMBEDDING.md @@ -144,11 +144,14 @@ rather than parsing `Display` text: `None` when the issue isn't about a command at all (`break` outside a loop); narrow by it once `code` alone isn't specific enough, rather than parsing `message` to recover a name this field already gives you. -- **`KernelError::Execution(anyhow::Error)`** — a statement began running and - faulted: a builtin, the evaluator, an IO fault, or anything else the - interpreter propagated. The original error chain is intact — `{:?}` and - `.source()` still walk it — even though `Display` (`{}`) shows only the - outermost message, matching `anyhow`'s usual behavior. +- **`KernelError::Execution { error, output }`** — a statement began running + and faulted: a builtin, the evaluator, an IO fault, or anything else the + interpreter propagated. `error` is the original chain — `{:?}` and + `.source()` walk it, and `{:#}` prints it — while `Display` (`{}`) shows + only the outermost message, matching `anyhow`. `output` is what ran before + the fault, stdout and stderr: for `echo left && x=$((1/0))` it holds + `left`. It is empty when the fault came first. A streaming caller has + already received the same output through `on_output`. `is_rejected()` (and its complement, `is_execution_failure()`) answer the coarse question without a match statement. `KernelError` is