From b52ffc69825798e9126cd2517725e10558ce2158 Mon Sep 17 00:00:00 2001 From: Kinflou Date: Sun, 18 Oct 2026 04:32:13 +0800 Subject: [PATCH] feat: say why a fault happened, and stop faking ordinal 0 everywhere MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit simulator#20 left six sites that still answered a failure with `ordinal: 0` and an `{"error": "..."}` body, because each carried a message the user needs and a fault is opaque on the wire — there was nowhere else to put the text. This adds that somewhere. `SimOutcome::Fault` now carries an optional `note`. It never reaches the wire. `reply_now` pins it to the response frame it just emitted (`Frame::note`, distinct from the existing `Frame::fault`, which is what the wire's fault injection did to a frame), and `describe_frame` hands it to the inspector as `FrameDetail::note`. The simulator is both ends, so it can show what a real provider would only have logged. Every site now faults with a note: - `Forward` with no connection picked - `begin_forward`'s refusals: no such connection, a refused connection, a forwarding cycle, a target without that function - a relayed upstream fault, timeout or undecodable reply, naming the connection that failed — so a cycle reads "forwarding cycle" at the innermost hop and "forward: faulted" at each hop outward - a script that throws, errors at runtime, or does not compile - the disabled-script stub Before, every one of these reached the caller as a *schema* error: its client decodes ordinal 0 as whichever declared error happens to hold it, a wrong answer rather than a failure. Also fixes a one-way bug these sites shared: a failing script on a one-way function emitted a response frame no caller was waiting for. `fault_or_none` mirrors `ok_or_none` — a one-way call has no response to carry a fault, so it sends nothing. Behaviour change worth knowing: a script's `throw` used to raise `ordinal: 0`; it now faults, with the thrown value as the note. It could never name a declared error anyway. 137 tests pass with the `script` feature, 128 without; clippy clean on both, builds for wasm32. --- src/behavior.rs | 122 +++++++++++++++++++++++++++++---------------- src/engine.rs | 65 ++++++++++++++++-------- src/frame.rs | 6 +++ src/framedecode.rs | 38 +++++++++++++- 4 files changed, 167 insertions(+), 64 deletions(-) diff --git a/src/behavior.rs b/src/behavior.rs index d5f372b..ebdea70 100644 --- a/src/behavior.rs +++ b/src/behavior.rs @@ -21,9 +21,12 @@ pub enum SimOutcome { Ok(Value), /// A raised schema error: its schema-global `ordinal` and `data` body. Err { ordinal: u16, data: Value }, - /// The provider failed where the schema declares no error for it. Carries - /// no body: a fault is deliberately opaque on the wire. - Fault, + /// The provider failed where the schema declares no error for it. A fault + /// is opaque on the wire — no ordinal, no body — so `note`, the + /// simulator's own account of *why*, never leaves the engine: it is pinned + /// to the response frame for the inspector, the way a real provider would + /// log it server-side. + Fault { note: Option }, /// One-way — nothing to send back. None, } @@ -68,6 +71,17 @@ fn ok_or_none(function: &FnShape, value: Value) -> SimOutcome { } } +/// A fault for a normal function, `None` for a one-way one. A one-way call has +/// no response frame, so it cannot carry a fault: sending one anyway would put +/// a reply on the wire that no caller is waiting for. +fn fault_or_none(function: &FnShape, note: Option) -> SimOutcome { + if function.oneway { + SimOutcome::None + } else { + SimOutcome::Fault { note } + } +} + // ── the canned behaviours ──────────────────────────────────────────────── /// "Reply with value" — always answers with a fixed value. @@ -140,8 +154,17 @@ impl Behavior for Raise { /// schema declares no error for it. The caller sees a fault, not an ordinal. pub struct Faulting; impl Behavior for Faulting { - fn run(&mut self, _ctx: BehaviorCtx<'_>) -> BehaviorStep { - BehaviorStep::Now(SimOutcome::Fault) + fn run(&mut self, ctx: BehaviorCtx<'_>) -> BehaviorStep { + BehaviorStep::Now(fault_or_none(ctx.function, None)) + } +} + +impl SimOutcome { + /// A fault, with the simulator's reason for it. + pub fn fault(note: impl Into) -> Self { + SimOutcome::Fault { + note: Some(note.into()), + } } } @@ -161,10 +184,7 @@ pub struct Forward { impl Behavior for Forward { fn run(&mut self, ctx: BehaviorCtx<'_>) -> BehaviorStep { if self.via.is_empty() { - return BehaviorStep::Now(SimOutcome::Err { - ordinal: 0, - data: serde_json::json!({ "error": "forward: pick a connection" }), - }); + return BehaviorStep::Now(SimOutcome::fault("forward: pick a connection")); } BehaviorStep::Forward { via: self.via.clone(), @@ -180,10 +200,11 @@ impl Behavior for Forward { /// "Run a script (Rhai)" — a user-written handler. `params` (the decoded /// request) and a persistent `state` map are in scope; the last expression is -/// the reply, `throw` raises an error. +/// the reply. `throw`, a runtime error or a script that does not compile all +/// fault the call, with the error as the frame's note. #[cfg(feature = "script")] mod script { - use super::{ok_or_none, Behavior, BehaviorCtx, BehaviorStep, SimOutcome}; + use super::{fault_or_none, ok_or_none, Behavior, BehaviorCtx, BehaviorStep, FnShape}; use serde_json::Value; thread_local! { @@ -226,7 +247,7 @@ mod script { impl Behavior for Script { fn run(&mut self, ctx: BehaviorCtx<'_>) -> BehaviorStep { let Some(ast) = &self.ast else { - return err(self.error.clone().unwrap_or_default()); + return err(ctx.function, self.error.clone().unwrap_or_default()); }; ENGINE.with(|engine| { let mut scope = rhai::Scope::new(); @@ -244,17 +265,18 @@ mod script { rhai::serde::from_dynamic::(&value).unwrap_or(Value::Null); BehaviorStep::Now(ok_or_none(ctx.function, value)) } - Err(e) => err(e.to_string()), + Err(e) => err(ctx.function, e.to_string()), } }) } } - fn err(message: String) -> BehaviorStep { - BehaviorStep::Now(SimOutcome::Err { - ordinal: 0, - data: serde_json::json!({ "error": message }), - }) + /// A script that failed. This used to answer `ordinal: 0` with the message + /// as the body — a *schema* error the caller decodes as whichever error + /// happens to hold ordinal 0. The script never declared that error; it + /// failed. So it faults. + fn err(function: &FnShape, message: String) -> BehaviorStep { + BehaviorStep::Now(fault_or_none(function, Some(message))) } } @@ -262,7 +284,7 @@ mod script { /// it isn't available. #[cfg(not(feature = "script"))] mod script { - use super::{Behavior, BehaviorCtx, BehaviorStep, SimOutcome}; + use super::{fault_or_none, Behavior, BehaviorCtx, BehaviorStep}; struct Disabled; @@ -271,11 +293,11 @@ mod script { } impl Behavior for Disabled { - fn run(&mut self, _ctx: BehaviorCtx<'_>) -> BehaviorStep { - BehaviorStep::Now(SimOutcome::Err { - ordinal: 0, - data: serde_json::json!({ "error": "scripting is not enabled in this build" }), - }) + fn run(&mut self, ctx: BehaviorCtx<'_>) -> BehaviorStep { + BehaviorStep::Now(fault_or_none( + ctx.function, + Some("scripting is not enabled in this build".into()), + )) } } } @@ -794,10 +816,10 @@ mod tests { &proto.functions[0], &schema, ); - assert!(matches!( + assert_eq!( no_conn.run(ctx(&proto, Value::Null)), - BehaviorStep::Now(SimOutcome::Err { ordinal: 0, .. }) - )); + BehaviorStep::Now(SimOutcome::fault("forward: pick a connection")) + ); let mut fwd = BehaviorKind::Forward.make( &json!({ "viaConnectionId": "c2", "targetFn": "" }), @@ -891,36 +913,33 @@ mod tests { } #[test] - fn a_thrown_value_becomes_an_error() { + fn a_thrown_value_faults_the_call() { match run_script(r#"throw "boom""#, Value::Null) { - BehaviorStep::Now(SimOutcome::Err { ordinal: 0, data }) => { - assert!(data["error"].as_str().unwrap().contains("boom"), "{data}"); + BehaviorStep::Now(SimOutcome::Fault { note: Some(note) }) => { + assert!(note.contains("boom"), "{note}"); } - other => panic!("expected an error, got {other:?}"), + other => panic!("expected a fault, got {other:?}"), } } #[test] - fn a_script_that_does_not_compile_errors_at_run() { + fn a_script_that_does_not_compile_faults_at_run() { match run_script("this is ( not rhai", Value::Null) { - BehaviorStep::Now(SimOutcome::Err { data, .. }) => { - assert!( - data["error"].as_str().unwrap().contains("compile"), - "{data}" - ); + BehaviorStep::Now(SimOutcome::Fault { note: Some(note) }) => { + assert!(note.contains("compile"), "{note}"); } - other => panic!("expected a compile error, got {other:?}"), + other => panic!("expected a compile fault, got {other:?}"), } } #[test] fn an_infinite_loop_is_stopped_not_hung() { match run_script("let i = 0; while true { i += 1; } i", Value::Null) { - BehaviorStep::Now(SimOutcome::Err { data, .. }) => { - let msg = data["error"].as_str().unwrap().to_lowercase(); - assert!(msg.contains("operation") || msg.contains("limit"), "{data}"); + BehaviorStep::Now(SimOutcome::Fault { note: Some(note) }) => { + let msg = note.to_lowercase(); + assert!(msg.contains("operation") || msg.contains("limit"), "{note}"); } - other => panic!("expected a limit error, got {other:?}"), + other => panic!("expected a limit fault, got {other:?}"), } } @@ -940,5 +959,24 @@ mod tests { BehaviorStep::Now(SimOutcome::None) ); } + + #[test] + fn a_failing_one_way_script_still_sends_nothing() { + // There is no response frame for a fault to ride on: answering + // anyway would put a reply on the wire no caller is waiting for. + let mut f = fn_send(); + f.oneway = true; + f.returns = None; + let (schema, proto) = schema_with(vec![f], vec![], vec![]); + let mut b = BehaviorKind::Script.make( + &json!({ "source": r#"throw "boom""# }), + &proto.functions[0], + &schema, + ); + assert_eq!( + b.run(ctx(&proto, json!(["x"]))), + BehaviorStep::Now(SimOutcome::None) + ); + } } } diff --git a/src/engine.rs b/src/engine.rs index 7c29d0f..574a526 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -568,6 +568,7 @@ impl Engine { }; let (framing, codec) = (wire.framing, wire.codec); let mut resp = Vec::new(); + let mut note = None; match outcome { SimOutcome::Ok(value) => { let mut body = Vec::new(); @@ -583,10 +584,25 @@ impl Engine { } framing.encode_response_err(request_id, ordinal, &body, &mut resp); } - SimOutcome::Fault => framing.encode_response_fault(request_id, &mut resp), + SimOutcome::Fault { note: why } => { + framing.encode_response_fault(request_id, &mut resp); + note = why; + } SimOutcome::None => return, } self.emit(conn_id, Direction::Response, resp); + // The wire carries no reason for a fault; the provider's log does. + // `emit` records exactly one frame per call, so the reason belongs on + // the last one. + if let Some(note) = note { + if let Some(frame) = self + .wires + .get_mut(conn_id) + .and_then(|w| w.channel.tap.frames.last_mut()) + { + frame.note = Some(note); + } + } } /// Settle a call — either resume the forward waiting on it, or record it. @@ -610,8 +626,13 @@ impl Engine { // channel this fabricated `ordinal: 0`, which the outer // caller's client decodes as whichever error happens to hold // that ordinal — a wrong answer, not a failure. - CallResult::Fault | CallResult::Undecodable(_) | CallResult::Timeout => { - SimOutcome::Fault + CallResult::Fault => SimOutcome::fault(format!("forward: {} faulted", cont.via)), + CallResult::Undecodable(why) => SimOutcome::fault(format!( + "forward: {}'s reply did not decode ({why})", + cont.via + )), + CallResult::Timeout => { + SimOutcome::fault(format!("forward: {} timed out", cont.via)) } }; self.reply_now(&cont.outer_conn, cont.outer_request_id, outcome); @@ -652,14 +673,7 @@ impl Engine { _ => None, }; if let Some(message) = refusal { - self.reply_now( - outer_conn, - outer_request_id, - SimOutcome::Err { - ordinal: 0, - data: json!({ "error": message }), - }, - ); + self.reply_now(outer_conn, outer_request_id, SimOutcome::fault(message)); return; } @@ -680,10 +694,7 @@ impl Engine { self.reply_now( outer_conn, outer_request_id, - SimOutcome::Err { - ordinal: 0, - data: json!({ "error": format!("forward: {via} has no {target_fn}") }), - }, + SimOutcome::fault(format!("forward: {via} has no {target_fn}")), ); } } @@ -1025,12 +1036,24 @@ mod tests { let id = e.call(&c_in, "send", &json!(["loop"])).unwrap(); e.run(); - match e.result(id) { - Some(CallResult::Err { ordinal: 0, body }) => { - assert!(body["error"].as_str().unwrap().contains("cycle"), "{body}"); - } - other => panic!("expected a cycle error, got {other:?}"), - } + // The caller sees a fault — not `ordinal: 0`, which its client would + // have decoded as whichever declared error happened to hold it. + assert_eq!(e.result(id), Some(&CallResult::Fault)); + + // The reason is not on the wire, but it is on the frames. The refusal + // happens at the innermost hop; each outer hop relays it as a fault of + // its own, naming the connection that failed it. + let notes = |conn: &str| -> Vec { + e.tap(conn) + .unwrap() + .frames + .iter() + .filter_map(|f| f.note.clone()) + .collect() + }; + assert_eq!(notes(&c2_to_g1), ["forwarding cycle"]); + assert_eq!(notes(&c1_to_g2), [format!("forward: {c2_to_g1} faulted")]); + assert_eq!(notes(&c_in), [format!("forward: {c1_to_g2} faulted")]); } #[test] diff --git a/src/frame.rs b/src/frame.rs index 7de3d35..3b9d917 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -36,6 +36,11 @@ pub struct Frame { /// What the wire's fault spec did to this frame, if anything. #[serde(skip_serializing_if = "Option::is_none")] pub fault: Option, + /// Why the provider faulted, when this frame is a fault response. Not on + /// the wire — a fault is opaque by design — but the simulator is both + /// ends, so it can show what a real provider would only have logged. + #[serde(skip_serializing_if = "Option::is_none")] + pub note: Option, } /// Best-effort label for the log. The handshake frame is unambiguous (fixed @@ -99,6 +104,7 @@ impl Tap { at, kind, fault, + note: None, }); self.counter } diff --git a/src/framedecode.rs b/src/framedecode.rs index cd12ee8..eaef88d 100644 --- a/src/framedecode.rs +++ b/src/framedecode.rs @@ -35,6 +35,11 @@ pub struct FrameDetail { /// response: the provider faulted. No ordinal and no body to show. #[serde(skip_serializing_if = "core::ops::Not::not")] pub fault: bool, + /// response: why the provider faulted, when the simulator knows. Not + /// decoded from the bytes — a fault carries no reason on the wire — but + /// read off the recorded frame. + #[serde(skip_serializing_if = "Option::is_none")] + pub note: Option, /// handshake: the fields it carries (names resolved where known). #[serde(skip_serializing_if = "Option::is_none")] pub handshake: Option, @@ -76,6 +81,7 @@ impl FrameDetail { ok: None, err: None, fault: false, + note: None, handshake: None, } } @@ -185,7 +191,10 @@ pub fn describe_frame(frame: &Frame, ctx: &DecodeCtx<'_>) -> FrameDetail { d.framing = "undecodable".into(); } } - Envelope::Fault => d.fault = true, + Envelope::Fault => { + d.fault = true; + d.note = frame.note.clone(); + } } d } @@ -301,6 +310,33 @@ mod tests { assert_ne!(res.framing, "undecodable"); } + #[test] + fn a_fault_frame_shows_why_the_provider_faulted() { + let mut session = chat::session(); + session + .set_behavior( + &session + .instances + .iter() + .find(|i| i.role == crate::model::Role::Server) + .unwrap() + .id + .clone(), + "send", + crate::behavior::BehaviorKind::Forward, + Some(json!({ "viaConnectionId": "", "targetFn": "send" })), + ) + .unwrap(); + let mut e = Engine::new(); + e.sync(&session); + e.call(chat::CONN, "send", &json!(["hi"])).unwrap(); + e.run(); + + let res = detail(&e, chat::CONN, 4); + assert!(res.fault); + assert_eq!(res.note.as_deref(), Some("forward: pick a connection")); + } + #[test] fn a_raised_error_decodes_its_ordinal_and_body() { let mut session = chat::session();