diff --git a/crates/engine/src/id_params_behaviour_tests.rs b/crates/engine/src/id_params_behaviour_tests.rs index ae77c8a1..c5207b44 100644 --- a/crates/engine/src/id_params_behaviour_tests.rs +++ b/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()); +} diff --git a/crates/engine/src/params.rs b/crates/engine/src/params.rs index 67a5b80a..35a74e0e 100644 --- a/crates/engine/src/params.rs +++ b/crates/engine/src/params.rs @@ -88,6 +88,17 @@ pub fn req_id(cmd: &str, p: &Value, key: &str) -> Result { 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 { pub fn opt_id(cmd: &str, p: &Value, key: &str) -> Result> { 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> { 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() {