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

Never crash: last-resort guard around command execution, and a logging panic hook

apps/effectcraft/src/main.rs

@@ -26,6 +26,7 @@ fn app_icon() -> egui::IconData {
 fn main() -> eframe::Result {
     // Help ▸ Enable Logging writes through this logger; warnings feed the compatibility report.
     effectcraft_engine::logging::install();
+    effectcraft_engine::logging::install_panic_hook();
     let mut control_port: Option<u16> = std::env::var("EFFECTCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok());
     let mut files = Vec::new();
     let mut demo = true;

crates/engine/src/guard.rs

@@ -0,0 +1,65 @@
+//! The last-resort guard: a panic that escapes a command (a bug, never an expected error) is
+//! turned into an [`EngineError`] instead of taking the app, and the user's open project, down
+//! with it. Edits work on a copy of the project (see [`crate::Session::edit`]), so an edit that
+//! panics leaves the project as it was. This is a safety net, not a substitute for returning
+//! errors (AGENTS.md, "Never crash").
+
+use crate::{EngineError, Result};
+
+/// Run `f`; a panic inside it becomes an error naming `what`. The panic itself is logged by
+/// the panic hook ([`crate::logging::install_panic_hook`]).
+pub fn guarded<T>(what: &str, f: impl FnOnce() -> Result<T>) -> Result<T> {
+    match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) {
+        Ok(r) => r,
+        Err(payload) => {
+            let msg = payload
+                .downcast_ref::<&str>()
+                .map(|s| s.to_string())
+                .or_else(|| payload.downcast_ref::<String>().cloned())
+                .unwrap_or_else(|| "unknown error".into());
+            log::error!("{what} panicked: {msg}");
+            Err(EngineError::Other(format!("{what} failed with an internal error ({msg}); it was stopped and your project is still open")))
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn a_panic_becomes_an_error() {
+        let r: Result<()> = guarded("command `test.panic`", || panic!("boom"));
+        let e = r.unwrap_err().to_string();
+        assert!(e.contains("test.panic") && e.contains("boom"), "{e}");
+        let r: Result<u8> = guarded("x", || {
+            let v: Vec<u8> = vec![];
+            Ok(v[3])
+        });
+        assert!(r.is_err());
+        assert_eq!(guarded("ok", || Ok(7)).unwrap(), 7);
+        assert!(matches!(guarded::<()>("err", || Err(EngineError::NoComp)), Err(EngineError::NoComp)));
+    }
+
+    #[test]
+    fn the_session_survives_a_panicking_edit() {
+        let mut s = crate::Session::new();
+        let before = s.project.clone();
+        let r: Result<()> = guarded("command `test.edit`", || {
+            s.edit("Broken", None, |p, _| {
+                p.next_id += 1;
+                panic!("bug in the middle of an edit")
+            })
+        });
+        assert!(r.is_err());
+        assert!(std::sync::Arc::ptr_eq(&before, &s.project), "the project is untouched");
+        // Later edits still work.
+        let next = s.project.next_id;
+        s.edit("Fine", None, |p, _| {
+            p.next_id += 1;
+            Ok(())
+        })
+        .unwrap();
+        assert_eq!(s.project.next_id, next + 1);
+    }
+}

crates/engine/src/lib.rs

@@ -14,6 +14,7 @@ pub mod commands;
 pub mod config;
 pub mod demo;
 pub mod footage_check;
+pub mod guard;
 pub mod history;
 pub mod jobs;
 pub mod learn;
@@ -495,7 +496,8 @@ impl Session {
                 return Err(EngineError::Disabled(id.to_string(), why));
             }
         }
-        let r = (spec.run)(self, &params)?;
+        // Last-resort guard: a panicking command reports an error instead of crashing the app.
+        let r = guard::guarded(&format!("command `{id}`"), || (spec.run)(self, &params))?;
         self.learn_observe(id, &params);
         if spec.journal {
             self.journal.push((id.to_string(), params));

crates/engine/src/logging.rs

@@ -62,6 +62,16 @@ pub fn install() {
     }
 }
 
+/// Log every panic (message and location) through the logger, then run the previous hook
+/// (which prints it). Panics in commands are then caught by [`crate::guard::guarded`].
+pub fn install_panic_hook() {
+    let previous = std::panic::take_hook();
+    std::panic::set_hook(Box::new(move |info| {
+        log::error!("panic: {info}");
+        previous(info);
+    }));
+}
+
 /// The log file inside `config_dir` (the settings folder), or the temporary folder.
 pub fn log_path(config_dir: Option<&Path>) -> PathBuf {
     config_dir.map(Path::to_path_buf).unwrap_or_else(std::env::temp_dir).join("Logs").join("EffectCraft Log.txt")