Merge pull request #58 from storytold/pr/never-crash-guard

Never crash: last-resort guard around command execution

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<u16> = std::env::var("LIGHTCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok());
     let mut files = Vec::new();

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::<String>()) {
+        (Some(s), _) => (*s).to_string(),
+        (_, Some(s)) => s.clone(),
+        _ => "unknown panic".into(),
+    }
+}
+
+/// Run `f`; a panic inside it becomes `Err("<what> failed unexpectedly: <message>")`. On wasm32
+/// (built with `panic = "abort"`) nothing can be caught and this just runs `f`.
+pub fn catch<T>(what: &str, f: impl FnOnce() -> T) -> Result<T, String> {
+    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"));
+    }
+}

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;