Merge pull request #3230 from vangel76/script-expression-error-3219

Scripting: expressionError reports evaluation errors, as After Effects

crates/engine/src/cmd/expr_tools.rs

@@ -73,15 +73,7 @@ pub fn errors_in(ctx: &EvalCtx, check: Option<fn(&str) -> std::result::Result<()
             if !keep_going() {
                 return None;
             }
-            let msg = if e.enabled {
-                match &ctx.expr {
-                    Some(h) => h.eval(ctx, l, p, &p.value_at(l.layer_time(ctx.time))).err(),
-                    None => None,
-                }
-            } else {
-                check.and_then(|check| check(&e.text).err())
-            };
-            if let Some(message) = msg {
+            if let Some(message) = property_error(ctx, check, l, p) {
                 out.push(error_of(ctx.comp_id, i, l, p, message, !e.enabled));
             }
         }
@@ -89,6 +81,20 @@ pub fn errors_in(ctx: &EvalCtx, check: Option<fn(&str) -> std::result::Result<()
     keep_going().then_some(out)
 }
 
+/// Why property `p` of layer `l` fails at `ctx.time` (comp time), as `expr.errors` reports it and
+/// scripts read it as `Property.expressionError`: the evaluation error (an exception or a
+/// non-finite result) of an enabled expression, or the syntax error that disabled one. `None`
+/// without an expression or when it evaluates. Evaluates this one expression; renders nothing.
+/// Without an evaluator an enabled expression gets the syntax check, so a broken one (from a
+/// project file) still reports why.
+pub fn property_error(ctx: &EvalCtx, check: Option<fn(&str) -> std::result::Result<(), String>>, l: &Layer, p: &Property) -> Option<String> {
+    let e = p.expr.as_ref().filter(|e| !e.text.trim().is_empty())?;
+    match ctx.expr {
+        Some(h) if e.enabled => h.eval(ctx, l, p, &p.value_at(l.layer_time(ctx.time))).err(),
+        _ => check.and_then(|check| check(&e.text).err()),
+    }
+}
+
 fn error_of(cid: ItemId, i: usize, l: &Layer, p: &Property, message: String, disabled: bool) -> ExprError {
     ExprError {
         comp: cid.0,

crates/script/src/model.rs

@@ -33,6 +33,7 @@ fn layer_of<'a>(s: &'a Session, a: &J) -> Result<(ItemId, &'a Comp, &'a Layer),
     Ok((cid, c, l))
 }
 
+/// Answers a script model query of the given `kind` against the session, dispatching to the matching reader, and errors on an unknown kind.
 pub fn query(s: &mut Session, kind: &str, a: &J) -> R {
     match kind {
         "project" => Ok(project(s)),
@@ -52,13 +53,7 @@ pub fn query(s: &mut Session, kind: &str, a: &J) -> R {
             let i = u(a, "index").unwrap_or(0) as usize;
             Ok(effectcraft_engine::command_specs().get(i.wrapping_sub(1)).map(|c| json!(c.id)).unwrap_or(J::Null))
         }
-        "exprError" => {
-            let code = a.get("code").and_then(J::as_str).unwrap_or("");
-            Ok(match s.expr_check {
-                Some(check) => json!(check(code).err().unwrap_or_default()),
-                None => json!(""),
-            })
-        }
+        "exprError" => expr_error(s, a),
         "labels" => Ok(json!(Label::ALL.iter().map(|l| l.name()).collect::<Vec<_>>())),
         "effects" => Ok(json!(
             effectcraft_engine::effects::all()
@@ -549,6 +544,21 @@ fn value(s: &Session, a: &J) -> R {
     Ok(js_value(c, &v))
 }
 
+/// `Property.expressionError`: why the property's expression fails at the comp's current time
+/// (where After Effects evaluates it; the prelude sends no `time`, a direct query may give one
+/// in comp seconds). The message is the one `expr.errors` gives: an evaluation error (an
+/// exception or a non-finite result) or the syntax error that disabled it; `""` when it
+/// evaluates (#3219).
+fn expr_error(s: &Session, a: &J) -> R {
+    let (cid, c, l) = layer_of(s, a)?;
+    let uid = u(a, "uid").ok_or("missing uid")?;
+    let p = l.props.find(uid).ok_or("the property no longer exists (it was deleted)")?;
+    let t = f(a, "time").map(Tick::from_seconds_f64).unwrap_or_else(|| s.time_of(cid));
+    // With the footage source, like `value`: `sampleImage` must not fail for want of it (#1000).
+    let ctx = EvalCtx { footage: Some(s.footage.as_ref()), expr: s.expr.as_deref(), ..EvalCtx::new(&s.project, cid, c, t) };
+    Ok(json!(effectcraft_engine::commands::expr_tools::property_error(&ctx, s.expr_check, l, p).unwrap_or_default()))
+}
+
 fn interp(i: Interp) -> &'static str {
     match i {
         Interp::Linear => "LINEAR",

crates/script/src/prelude.js

@@ -868,8 +868,8 @@ Property.prototype.property = function () { return null; };
     __call("prop.setExpression", this.__ref({ enabled: !!v }));
   });
   def("expressionError", function () {
-    var e = this.expression;
-    return e ? __get("exprError", { code: e }) : "";
+    // Evaluated at the comp's current time, so a runtime error or a non-finite result shows too.
+    return this.expression ? __get("exprError", { comp: this.__layer.__comp, layer: this.__layer.__id, uid: this.__uid }) : "";
   });
   def("canSetExpression", function () { return !!this.__info().canSetExpression; });
   def("canVaryOverTime", function () { return !!this.__info().canVaryOverTime; });

crates/script/src/tests.rs

@@ -1306,6 +1306,41 @@ fn sample_image_reads_footage_pixels_in_scripts() {
     assert_eq!(out.result, json!([0, 1, 0, 1, ""]), "green footage sampled, not transparent black");
 }
 
+/// `expressionError` reports what evaluating the expression at the comp's current time does, as
+/// After Effects: a non-finite result and an exception, not only a syntax error (#3219).
+#[test]
+fn expression_error_reports_evaluation_errors() {
+    let mut s = session();
+    let out = ok(
+        &mut s,
+        r#"
+        var c = app.project.items.addComp("E", 320, 240, 1, 5, 30);
+        var p = c.layers.addSolid([1, 1, 1], "S", 320, 240, 1).transform.opacity;
+        p.expression = "Math.sqrt(-1)";
+        var nan = p.expressionError;
+        p.expression = 'throw new Error("boom")';
+        var thrown = p.expressionError;
+        p.expression = "value";
+        var fine = p.expressionError;
+        p.expression = "thisLayer.nope(";
+        var syntax = p.expressionError;
+        p.expression = "time > 1 ? Math.sqrt(-1) : value";
+        c.time = 0;
+        var early = p.expressionError;
+        c.time = 2;
+        var late = p.expressionError;
+        [nan, thrown, fine, syntax.length > 0, early, late];
+        "#,
+    );
+    let v = out.result;
+    assert!(v[0].as_str().is_some_and(|e| e.contains("not a finite number")), "{v}");
+    assert!(v[1].as_str().is_some_and(|e| e.contains("boom")), "{v}");
+    assert_eq!(v[2], json!(""), "{v}");
+    assert_eq!(v[3], json!(true), "a syntax error is still reported: {v}");
+    assert_eq!(v[4], json!(""), "evaluated at the current time (0), where it is fine: {v}");
+    assert!(v[5].as_str().is_some_and(|e| e.contains("not a finite number")), "evaluated at the current time (2): {v}");
+}
+
 #[test]
 fn binary_file_io_preserves_all_byte_values() {
     let dir = std::env::temp_dir().join(format!("ec-script-binary-{}", std::process::id()));