diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs index 975283af4..7f7443301 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -17,6 +17,7 @@ //! window (issue #7). Their `--help` / `--version` output and diagnostics then have no console to //! go to; use `lightcraft-cli` (a console program) from a terminal, or a debug build. #![cfg_attr(all(target_os = "windows", not(debug_assertions)), windows_subsystem = "windows")] +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] mod alloc_release; mod control_server; @@ -263,6 +264,7 @@ OPTIONS: "; fn main() -> eframe::Result { + lightcraft_engine::guard::install_hook(std::env::temp_dir().join("lightcraft-panics.log")); alloc_release::install(); let mut control_port: Option = std::env::var("LIGHTCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok()); let mut files = Vec::new(); diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..4b6c79f55 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,4 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-panic-in-tests = true +allow-indexing-slicing-in-tests = true diff --git a/crates/engine/src/guard.rs b/crates/engine/src/guard.rs new file mode 100644 index 000000000..4ab53179a --- /dev/null +++ b/crates/engine/src/guard.rs @@ -0,0 +1,55 @@ +//! Last-resort panic guard (never-crash standard, `AGENTS.md`): a panic that escapes a command +//! becomes that command's error and the session (catalog, library, undo history) stays usable. +//! It's a safety net, not a way to report errors: commands return `Err` themselves. + +use std::any::Any; +use std::io::Write; +use std::panic::{self, AssertUnwindSafe}; +use std::path::PathBuf; + +/// The text of a panic payload (`panic!("…")` gives a `&str` or a `String`). +pub fn panic_message(payload: &(dyn Any + Send)) -> String { + match (payload.downcast_ref::<&str>(), payload.downcast_ref::()) { + (Some(s), _) => (*s).to_string(), + (_, Some(s)) => s.clone(), + _ => "unknown panic".into(), + } +} + +/// Run `f`; a panic inside it becomes `Err(" failed unexpectedly: ")`. On wasm32 +/// (built with `panic = "abort"`) nothing can be caught and this just runs `f`. +pub fn catch(what: &str, f: impl FnOnce() -> T) -> Result { + panic::catch_unwind(AssertUnwindSafe(f)).map_err(|p| { + let msg = format!("{what} failed unexpectedly: {}", panic_message(p.as_ref())); + log::error!("{msg}"); + msg + }) +} + +/// Install a panic hook that keeps the default report (stderr) and also appends the panic's +/// location and message to `log` (best effort), so a panic in a GUI session leaves a trace. +pub fn install_hook(log: PathBuf) { + let default = panic::take_hook(); + panic::set_hook(Box::new(move |info| { + default(info); + let at = info.location().map(|l| format!("{}:{}", l.file(), l.line())).unwrap_or_default(); + let secs = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0); + let thread = std::thread::current().name().unwrap_or("unnamed").to_string(); + if let Ok(mut f) = std::fs::OpenOptions::new().create(true).append(true).open(&log) { + let _ = writeln!(f, "{secs} [{thread}] panic at {at}: {}", panic_message(info.payload())); + } + })); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_panic_becomes_an_error() { + assert_eq!(catch("ok", || 7), Ok(7)); + let e = catch("`demo.cmd`", || -> u32 { panic!("boom {}", 1) }).unwrap_err(); + assert_eq!(e, "`demo.cmd` failed unexpectedly: boom 1"); + assert!(catch("x", || std::panic::panic_any(5u8)).unwrap_err().ends_with("unknown panic")); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ce0404f23..d7fd6be53 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -17,6 +17,7 @@ pub mod demo; pub mod devices; pub mod export; pub mod files; +pub mod guard; pub mod import; pub mod library; pub mod media; @@ -237,7 +238,8 @@ impl Session { let log_start = self.pending_log.len(); let was_active = self.active(); self.depth += 1; - let r = (spec.run)(self, params); + // last-resort guard: a panic in a command is that command's error, not a crash + let r = guard::catch(&format!("`{id}`"), || (spec.run)(self, params)).unwrap_or_else(|e| Err(EngineError::Other(e))); self.depth -= 1; if self.depth == 0 && was_active.is_some() && self.active() != was_active { self.previous_active = was_active;