diff --git a/crates/engine/src/batch_step_ref_tests.rs b/crates/engine/src/batch_step_ref_tests.rs index 451a6300..47d6e876 100644 --- a/crates/engine/src/batch_step_ref_tests.rs +++ b/crates/engine/src/batch_step_ref_tests.rs @@ -258,3 +258,25 @@ fn a_nested_batch_has_its_own_names() { ); assert!(msg.contains("`$ref` \"frame\" names no step"), "the outer names aren't visible inside: {msg}"); } + +#[test] +fn a_failing_command_names_its_step_and_rolls_back() { + let mut s = Session::new(); + let frame = id(&s.execute("node.create", &json!({"type": "frame", "width": 50, "height": 50})).unwrap()); + let text = id(&s.execute("node.create", &json!({"type": "text", "text": "orig"})).unwrap()); + let undo = s.ds().unwrap().history.undo.len(); + let name = s.doc().unwrap().node(s.page().unwrap()).unwrap().name.clone(); + let msg = batch_error( + &mut s, + json!([ + {"command": "text.set", "params": {"ids": [text.0], "text": "novo"}}, + {"command": "text.set", "params": {"ids": [frame.0], "text": "erro"}}, + {"command": "page.rename", "params": {"name": "Depois do erro"}} + ]), + ); + // Still the command's own kind of error, its message saying which step. + assert!(msg.starts_with("step 1: ") && msg.contains("is not text"), "{msg}"); + assert!(matches!(&s.doc().unwrap().node(text).unwrap().kind, figcraft_doc::NodeKind::Text(t) if t.characters == "orig")); + assert_eq!(s.doc().unwrap().node(s.page().unwrap()).unwrap().name, name); + assert_eq!(s.ds().unwrap().history.undo.len(), undo); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 99ea0718..f47e2ea2 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -117,6 +117,18 @@ impl From for EngineError { } } +impl EngineError { + /// This error from step `step` of a batch, saying so in its message; the kind stays (errors + /// without a message stay as they are). + fn at_step(self, step: usize) -> Self { + match self { + EngineError::BadParams { cmd, msg } => EngineError::BadParams { cmd, msg: format!("step {step}: {msg}") }, + EngineError::Other(msg) => EngineError::Other(format!("step {step}: {msg}")), + e => e, + } + } +} + pub type Result = std::result::Result; pub fn bad(cmd: &str, msg: impl Into) -> EngineError { @@ -1258,7 +1270,8 @@ impl Session { /// so its steps resolve references the same way, inside its own [`Session::grouped`]. pub fn batch_step(&mut self, scope: &mut BatchScope, cmd: &str, params: &Value) -> Result<()> { let p = if cmd == "command.batch" { params.clone() } else { scope.resolve(params)? }; - let v = self.execute(cmd, &p)?; + // Say which step failed; a nested batch names its own steps. + let v = self.execute(cmd, &p).map_err(|e| if cmd == "command.batch" { e } else { e.at_step(scope.next_step()) })?; scope.push(v); Ok(()) }