diff --git a/AGENTS.md b/AGENTS.md index c9c3596..3f145f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,7 +17,7 @@ People trust CADCraft with their drawings; a crash loses their work. **This outr - **Errors are `Result`** through the crate's error type and `?`. An unfinished feature returns an error or reports "not available yet"; it never panics. - **Input-derived numbers are hostile** (DXF/DWG files, command-line text, MCP/control params): `get()` not `[i]`, checked arithmetic, no NaN casts, cap input-sized allocations and loop counts. - **Bound recursion** (nested/cyclic block references: `MAX_BLOCK_DEPTH`). -- **Last-resort guard:** `Session::execute` and interactive command input run under `catch_unwind`; an escaped panic restores the drawing and reports an error. +- **Last-resort guard:** `Session::execute` and every step of an interactive command (its factory, `begin`, input, `prompt` and `preview`; `crates/engine/src/guard.rs`) run under `catch_unwind`; an escaped panic restores the drawing and reports an error. - **Prove it:** every crash fix lands with a regression test (see `hostile_params_never_panic`, `hostile_dxf_does_not_panic`). - Every production crate root carries `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]`. diff --git a/CLAUDE.md b/CLAUDE.md index 1ed44c1..4ab0e3c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,7 +17,7 @@ People trust CADCraft with their drawings; a crash loses their work. **This outr - **Errors are `Result`** through the crate's error type and `?`. An unfinished feature returns an error or reports "not available yet"; it never panics. - **Input-derived numbers are hostile** (DXF/DWG files, command-line text, MCP/control params): `get()` not `[i]`, checked arithmetic, no NaN casts, cap input-sized allocations and loop counts. - **Bound recursion** (nested/cyclic block references: `MAX_BLOCK_DEPTH`). -- **Last-resort guard:** `Session::execute` and interactive command input run under `catch_unwind`; an escaped panic restores the drawing and reports an error. +- **Last-resort guard:** `Session::execute` and every step of an interactive command (its factory, `begin`, input, `prompt` and `preview`; `crates/engine/src/guard.rs`) run under `catch_unwind`; an escaped panic restores the drawing and reports an error. - **Prove it:** every crash fix lands with a regression test (see `hostile_params_never_panic`, `hostile_dxf_does_not_panic`). - Every production crate root carries `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]`. diff --git a/crates/engine/src/guard.rs b/crates/engine/src/guard.rs new file mode 100644 index 0000000..39a2e6c --- /dev/null +++ b/crates/engine/src/guard.rs @@ -0,0 +1,176 @@ +//! Last-resort panic guard around interactive commands. `Session::execute` and interactive input +//! already catch a panicking command; this wrapper does the same for the rest of a command +//! machine: its factory, `begin` (the first step of `Session::start`), `prompt` and `preview` +//! (called by the UI every frame). A panic ends the command with an internal error instead of +//! taking down the app: from `begin` or `input` at once (the session restores the drawing), from +//! `prompt` or `preview` (which can't end the command) at the next input, with a fallback prompt +//! and no preview until then. + +use std::any::Any; +use std::cell::Cell; +use std::panic::{AssertUnwindSafe, catch_unwind}; + +use cadcraft_doc::EntityKind; +use cadcraft_geom::Vec2; + +use crate::cmd::Factory; +use crate::{Accept, EngineError, Input, Interactive, Prompt, Result, Session, Step}; + +pub(crate) struct Guarded { + inner: Box, + /// What panicked in `prompt` or `preview`; the command ends at the next input. + broken: Cell>, +} + +fn message(p: &(dyn Any + Send)) -> String { + p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) +} + +impl Guarded { + /// Create the machine of command `id` and wrap it. + pub(crate) fn create(id: &str, factory: Factory, s: &Session) -> Result> { + let inner = catch_unwind(AssertUnwindSafe(|| factory(s))).map_err(|p| EngineError::Internal(id.into(), message(&*p)))??; + Ok(Box::new(Guarded { inner, broken: Cell::new(None) })) + } + + fn internal(&self, p: &(dyn Any + Send)) -> EngineError { + EngineError::Internal(self.inner.name().to_ascii_lowercase(), message(p)) + } +} + +impl Interactive for Guarded { + fn name(&self) -> &'static str { + self.inner.name() + } + fn prompt(&self, s: &Session) -> Prompt { + if self.broken.get().is_none() { + match catch_unwind(AssertUnwindSafe(|| self.inner.prompt(s))) { + Ok(p) => return p, + Err(_) => self.broken.set(Some("panic in prompt")), + } + } + Prompt::new("Internal error; press Enter to end the command", Accept::TEXT) + } + fn input(&mut self, s: &mut Session, i: Input) -> Result { + if let Some(m) = self.broken.get() { + return Err(EngineError::Internal(self.inner.name().to_ascii_lowercase(), m.into())); + } + catch_unwind(AssertUnwindSafe(|| self.inner.input(s, i))).unwrap_or_else(|p| Err(self.internal(&*p))) + } + fn begin(&mut self, s: &mut Session) -> Result { + catch_unwind(AssertUnwindSafe(|| self.inner.begin(s))).unwrap_or_else(|p| Err(self.internal(&*p))) + } + fn preview(&self, s: &Session, cursor: Vec2) -> Vec { + if self.broken.get().is_some() { + return Vec::new(); + } + catch_unwind(AssertUnwindSafe(|| self.inner.preview(s, cursor))).unwrap_or_else(|_| { + self.broken.set(Some("panic in preview")); + Vec::new() + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::Running; + + fn dot(p: Vec2) -> EntityKind { + EntityKind::Point(cadcraft_doc::Point { p: p.to3(0.0), angle: 0.0 }) + } + + /// Panics in the step named by `at` ("factory", "begin", "prompt", "preview" or "input"). + struct PanicM(&'static str); + + impl Interactive for PanicM { + fn name(&self) -> &'static str { + "PANICTEST" + } + fn prompt(&self, _s: &Session) -> Prompt { + assert_ne!(self.0, "prompt", "prompt panicked"); + Prompt::new("Specify a point", Accept::POINT) + } + fn input(&mut self, s: &mut Session, _i: Input) -> Result { + s.add_entity(dot(Vec2::ZERO))?; + assert_ne!(self.0, "input", "input panicked"); + Ok(Step::Continue) + } + fn begin(&mut self, s: &mut Session) -> Result { + s.add_entity(dot(Vec2::ZERO))?; + assert_ne!(self.0, "begin", "begin panicked"); + Ok(Step::Continue) + } + fn preview(&self, _s: &Session, cursor: Vec2) -> Vec { + assert_ne!(self.0, "preview", "preview panicked"); + vec![dot(cursor)] + } + } + + /// Start a PanicM the way `Session::start` starts an interactive command. + fn start(s: &mut Session, at: &'static str) -> Result<()> { + fn factory_panics(_: &Session) -> Result> { + panic!("factory panicked") + } + let factory: Factory = match at { + "factory" => factory_panics, + "begin" => |_| Ok(Box::new(PanicM("begin"))), + "prompt" => |_| Ok(Box::new(PanicM("prompt"))), + "preview" => |_| Ok(Box::new(PanicM("preview"))), + _ => |_| Ok(Box::new(PanicM("input"))), + }; + let machine = Guarded::create("panictest", factory, s)?; + let st = s.state()?; + let (before, selection_before) = (st.doc.clone(), st.selection.clone()); + s.running = Some(Running { id: "panictest".into(), machine, before, selection_before }); + s.feed(None) + } + + fn points(s: &Session) -> usize { + s.doc().unwrap().model.iter().count() + } + + #[test] + fn panics_in_a_command_machine_end_the_command_not_the_app() { + // Factory and begin: an internal error, nothing running, the drawing as it was. + for at in ["factory", "begin"] { + let mut s = Session::new(); + let e = start(&mut s, at).unwrap_err(); + assert!(matches!(&e, EngineError::Internal(_, m) if m.contains("panicked")), "{at}: {e}"); + assert!(s.running.is_none() && s.current_prompt().is_none(), "{at}"); + assert_eq!(points(&s), 0, "{at}: the drawing is restored"); + s.cmdline("line 0,0 1,0").unwrap(); + assert!(s.running.is_some(), "{at}: the session still works"); + } + + // Prompt and preview: a fallback prompt and no preview from then on; the next input ends + // the command and restores the drawing. + let at = Vec2::new(1.0, 1.0); + let mut s = Session::new(); + start(&mut s, "prompt").unwrap(); + assert_eq!(points(&s), 1, "begin ran"); + assert_eq!(s.preview(at).len(), 1, "the preview works until the prompt panics"); + assert!(s.current_prompt().unwrap().message.contains("Internal error")); + assert!(s.prompt_text().starts_with("PANICTEST Internal error"), "{}", s.prompt_text()); + assert!(s.preview(at).is_empty()); + assert!(matches!(s.input(Input::Enter), Err(EngineError::Internal(_, m)) if m == "panic in prompt")); + assert!(s.running.is_none()); + assert_eq!(points(&s), 0, "the drawing is restored"); + + let mut s = Session::new(); + start(&mut s, "preview").unwrap(); + assert!(s.preview(at).is_empty()); + assert!(s.preview(at).is_empty(), "not called again"); + assert!(s.current_prompt().unwrap().message.contains("Internal error")); + assert!(matches!(s.input(Input::Point(at)), Err(EngineError::Internal(_, m)) if m == "panic in preview")); + assert!(s.running.is_none()); + assert_eq!(points(&s), 0, "the drawing is restored"); + + // Input (already guarded by the session; the wrapper agrees). + let mut s = Session::new(); + start(&mut s, "input").unwrap(); + assert!(matches!(s.input(Input::Point(Vec2::ZERO)), Err(EngineError::Internal(..)))); + assert!(s.running.is_none()); + assert_eq!(points(&s), 0); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 456e2b7..18400bd 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -11,6 +11,7 @@ pub mod assoc; pub mod cmd; pub mod grips; +mod guard; pub mod prompt; pub mod sample; pub mod select; @@ -526,7 +527,7 @@ impl Session { self.echo(format!("Command: {}", spec.id.to_ascii_uppercase())); match spec.interactive { Some(factory) => { - let machine = factory(self)?; + let machine = guard::Guarded::create(spec.id, factory, self)?; let st = self.state()?; let before = st.doc.clone(); let selection_before = st.selection.clone();