#675: cap the offending value echoed in id/ids errors

crates/engine/src/id_params_behaviour_tests.rs

@@ -149,3 +149,13 @@ fn an_image_and_an_instance_parent_are_checked_the_same_way() {
     let inst = failed(s.execute("instance.create", &json!({"component": 99999, "parent": {"$step": 0}})));
     assert!(inst.is_some(), "instance.create refuses both a bad component and a bad parent");
 }
+
+#[test]
+fn a_huge_bad_value_is_cut_short_in_the_error() {
+    let mut s = Session::new();
+    card(&mut s);
+    let junk = "é".repeat(10_000);
+    let msg = failed(s.execute("node.set", &json!({"ids": junk, "props": {"name": "renamed"}}))).unwrap_or_else(|| panic!("must fail"));
+    assert!(msg.contains("`ids` must be an array") && msg.ends_with("…)"), "{msg}");
+    assert!(msg.chars().count() < 200, "{} chars", msg.chars().count());
+}

crates/engine/src/params.rs

@@ -88,6 +88,17 @@ pub fn req_id(cmd: &str, p: &Value, key: &str) -> Result<NodeId> {
     id(p, key).ok_or_else(|| bad(cmd, format!("`{key}` must be a node id")))
 }
 
+/// The offending value for an error message, cut to a few dozen characters so a huge param
+/// doesn't end up in the error whole.
+fn shown(v: &Value) -> String {
+    const MAX: usize = 40;
+    let text = v.to_string();
+    match text.char_indices().nth(MAX) {
+        Some((cut, _)) => format!("{}…", &text[..cut]),
+        None => text,
+    }
+}
+
 /// `key` when it is there: a layer id, or an error naming it. Missing and `null` mean "none", so a
 /// command can tell "no parent given" (use the page) from "a parent was given that isn't an id" —
 /// which is a mistake, not the default. Without this, `{"$step": 0}` reaching a param that isn't
@@ -95,7 +106,7 @@ pub fn req_id(cmd: &str, p: &Value, key: &str) -> Result<NodeId> {
 pub fn opt_id(cmd: &str, p: &Value, key: &str) -> Result<Option<NodeId>> {
     match get(p, key) {
         None | Some(Value::Null) => Ok(None),
-        Some(v) => id_of(v).map(Some).ok_or_else(|| bad(cmd, format!("`{key}` must be a node id (got {v})"))),
+        Some(v) => id_of(v).map(Some).ok_or_else(|| bad(cmd, format!("`{key}` must be a node id (got {})", shown(v)))),
     }
 }
 
@@ -125,17 +136,17 @@ pub fn req_targets(cmd: &str, s: &Session, p: &Value) -> Result<Vec<NodeId>> {
     match get(p, "ids") {
         Some(Value::Array(a)) => {
             if let Some(v) = a.iter().find(|v| id_of(v).is_none()) {
-                return Err(bad(cmd, format!("`ids`: every entry must be a node id (got {v})")));
+                return Err(bad(cmd, format!("`ids`: every entry must be a node id (got {})", shown(v))));
             }
         }
         Some(Value::Null) | None => {
             if let Some(v) = get(p, "id").filter(|v| !v.is_null())
                 && id_of(v).is_none()
             {
-                return Err(bad(cmd, format!("`id` must be a node id (got {v})")));
+                return Err(bad(cmd, format!("`id` must be a node id (got {})", shown(v))));
             }
         }
-        Some(v) => return Err(bad(cmd, format!("`ids` must be an array (got {v})"))),
+        Some(v) => return Err(bad(cmd, format!("`ids` must be an array (got {})", shown(v)))),
     }
     let t = targets(s, p);
     if t.is_empty() {