Engine: a grouped action that panics is rolled back before the panic goes on

Session::grouped ran its action without a panic guard, so a caller outside
a command that caught the panic found the group still open: Undo and Redo
disabled, the partial edits kept, and the next grouped action nested in the
abandoned group. The group is now rolled back and the panic resumed.
Regression test.

crates/engine/src/lib.rs

@@ -680,20 +680,26 @@ impl Session {
     pub fn grouped<R>(&mut self, label: &str, f: impl FnOnce(&mut Session) -> Result<R>) -> Result<R> {
         let token = self.group_begin(label)?;
         let doc = token.doc();
-        match f(self) {
-            Ok(r) => match self.group_end(token) {
+        // A panic in `f` rolls the group back before it goes on, so a caller that catches it
+        // (outside `execute`, which drops a command's groups itself) finds no group left open.
+        match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(self))) {
+            Ok(Ok(r)) => match self.group_end(token) {
                 Ok(_) => Ok(r),
                 // `f` closed the group's document (`file.close`, or `file.open` taking the place of
                 // an untouched new workbook), and the group went with it: nothing is left to undo.
                 Err(_) if !self.docs.iter().any(|d| d.uid == doc) => Ok(r),
                 Err(e) => Err(e),
             },
-            Err(e) => {
+            Ok(Err(e)) => {
                 // The error from `f` is the one to report. A rollback fails only when the group
                 // has already ended, and then there is nothing left to undo.
                 let _ = self.group_rollback(token);
                 Err(e)
             }
+            Err(panic) => {
+                let _ = self.group_rollback(token);
+                std::panic::resume_unwind(panic)
+            }
         }
     }
 

crates/engine/src/tests/seams.rs

@@ -579,6 +579,31 @@ fn a_command_that_panics_leaves_the_history_as_it_found_it() {
     assert_eq!(v(&s, "A1").display(), "1");
 }
 
+#[test]
+fn a_grouped_action_that_panics_is_rolled_back_before_the_panic_goes_on() {
+    // Outside a command, a caller that catches the panic finds the group rolled back and no
+    // group left open, so Undo is enabled and the next grouped action does not nest.
+    let mut s = s();
+    set(&mut s, "A1", "before");
+    let caught = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        s.grouped::<()>("Doomed", |s| {
+            set(s, "A1", "half done");
+            panic!("inside the group");
+        })
+    }));
+    assert!(caught.is_err(), "the panic goes on to the caller");
+    let uid = s.doc().unwrap().uid;
+    assert!(!s.in_group(uid));
+    assert_eq!(v(&s, "A1").display(), "before");
+    assert_eq!(undo_labels(&s), ["Enter Cell"]);
+    s.grouped("Next", |s| {
+        set(s, "A1", "after");
+        Ok(())
+    })
+    .unwrap();
+    assert_eq!(undo_labels(&s), ["Enter Cell", "Next"]);
+}
+
 #[test]
 fn a_group_ended_by_a_command_that_then_panics_keeps_its_step() {
     // The command takes a token the test left for it, ends or rolls back that group, and panics.