Plug-ins: a passed reference check keeps a failed run's error

A painted effect's cached result passed its swatch and pattern check and cleared the error of a failed run, so plugin.info reported none while the effect still failed on another object. A run and a reference check now record their errors separately, and each clears only its own.

crates/engine/src/tests/plugins.rs

@@ -439,6 +439,24 @@ fn a_painted_effect_on_a_fill_and_refused_results() {
     }
 }
 
+/// A result refused for a swatch the document doesn't have stays cached: once the document has the
+/// swatch, the next check of that result clears the error.
+#[test]
+fn a_refused_swatch_s_error_clears_once_the_document_has_the_swatch() {
+    let mut s = session();
+    let id = rect(&mut s, 10.0, "#ff0000");
+    let mut later = solid("#00ff00");
+    later["swatch"] = json!("Later");
+    let output = json!({"objects": [{"path": square_at(40.0), "fills": [later]}]}).to_string();
+    install_wat(&mut s, &tpl::returning(&painting("test.engine.laterswatch", true), &output));
+    run(&mut s, "effect.apply", json!({"ids": [id.0], "effect": "plugin.test.engine.laterswatch"}));
+    drawn(&s);
+    assert!(run(&mut s, "plugin.info", json!({"id": "test.engine.laterswatch"}))["lastError"].is_string());
+    run(&mut s, "swatch.new", json!({"name": "Later", "color": "#00ff00", "global": true}));
+    drawn(&s);
+    assert_eq!(run(&mut s, "plugin.info", json!({"id": "test.engine.laterswatch"}))["lastError"], Value::Null);
+}
+
 /// A painted effect on a group may link its pieces to the document's swatches.
 #[test]
 fn a_painted_group_effect_links_to_the_document_s_swatches() {

crates/plugins/src/effect.rs

@@ -174,7 +174,23 @@ fn weight(key: &Key, out: Option<&Out>) -> usize {
     };
     anchors(&key.path) + out + (key.params.len() + key.paints.len()) / 50 + 1
 }
-static ERRORS: Mutex<Option<HashMap<String, String>>> = Mutex::new(None);
+static ERRORS: Mutex<Option<HashMap<String, Failures>>> = Mutex::new(None);
+
+/// The errors recorded for a live-effect plug-in, one for each kind of outcome that can clear it.
+#[derive(Default)]
+struct Failures {
+    /// From a run of the plug-in.
+    run: Option<String>,
+    /// From checking the swatches and patterns a painted result names against the document.
+    references: Option<String>,
+}
+
+/// Which of a plug-in's [`Failures`] an outcome replaces.
+#[derive(Clone, Copy)]
+enum Origin {
+    Run,
+    References,
+}
 
 /// Where an installation ([`crate::Plugin::serial`]) stands as a live effect in this process.
 enum Runs {
@@ -227,21 +243,30 @@ fn end_run(serial: u64, cancelled: bool) {
     }
 }
 
-/// The last error of live-effect plug-in `id` (cleared when it next succeeds).
+/// The last error of live-effect plug-in `id`: a refused reference if there is one, else a failed
+/// run's. A run that succeeds clears a failed run's error, and a reference check that passes clears
+/// a refused reference's.
 pub fn last_error(id: &str) -> Option<String> {
-    ERRORS.lock().unwrap_or_else(|e| e.into_inner()).as_ref()?.get(id).cloned()
+    let errors = ERRORS.lock().unwrap_or_else(|e| e.into_inner());
+    let failures = errors.as_ref()?.get(id)?;
+    failures.references.clone().or_else(|| failures.run.clone())
 }
 
-/// Records how a run of plug-in `id` went: a failure as its last error (a painted effect's named
-/// after the plug-in, [`painted::named`]), a success clearing it.
-fn note(id: &str, plugin: &crate::Plugin, r: std::result::Result<(), &Error>) {
+/// Records how a run of plug-in `id`, or a check of what its result names, went: a failure as that
+/// origin's error (a painted effect's named after the plug-in, [`painted::named`]), a success
+/// clearing that origin's error only.
+fn note(id: &str, plugin: &crate::Plugin, origin: Origin, r: std::result::Result<(), &Error>) {
+    let message = r.err().map(|e| if plugin.manifest().paint_output { painted::named(&plugin.manifest().name, e) } else { e.to_string() });
     let mut errors = ERRORS.lock().unwrap_or_else(|e| e.into_inner());
     let errors = errors.get_or_insert_with(HashMap::new);
-    match r {
-        Ok(()) => errors.remove(id),
-        Err(e) if plugin.manifest().paint_output => errors.insert(id.to_string(), painted::named(&plugin.manifest().name, e)),
-        Err(e) => errors.insert(id.to_string(), e.to_string()),
-    };
+    let failures = errors.entry(id.to_string()).or_default();
+    match origin {
+        Origin::Run => failures.run = message,
+        Origin::References => failures.references = message,
+    }
+    if failures.run.is_none() && failures.references.is_none() {
+        errors.remove(id);
+    }
 }
 
 /// Does live-effect plug-in `id` return painted pieces (its manifest declares `paintOutput`)?
@@ -294,7 +319,7 @@ pub fn apply_painted(
 fn references_found(id: &str, plugin: &crate::Plugin, pieces: &[Piece], doc: Option<&Document>) -> Option<()> {
     let checked = painted::check_references(pieces, doc);
     if doc.is_some() {
-        note(id, plugin, checked.as_ref().map(|_| ()));
+        note(id, plugin, Origin::References, checked.as_ref().map(|_| ()));
     }
     checked.ok()
 }
@@ -334,7 +359,7 @@ fn evaluate(id: &str, params: &Value, path: &PathData, bounds: Rect, rule: FillR
     if cancelled {
         return None;
     }
-    note(id, &plugin, r.as_ref().map(|_| ()));
+    note(id, &plugin, Origin::Run, r.as_ref().map(|_| ()));
     let out = r.ok();
     let size = weight(&key, out.as_ref());
     if size > CACHE_ANCHORS / 4 {
@@ -377,6 +402,7 @@ fn run(plugin: &crate::Plugin, mut params: Value, path: &PathData, bounds: Rect,
 mod tests {
     use vectorcraft_color::{Color, Paint};
     use vectorcraft_doc::{FillLayer, StrokeLayer};
+    use vectorcraft_geom::{Point, SubPath};
 
     use super::*;
 
@@ -400,6 +426,34 @@ mod tests {
         assert_eq!((none.fills, none.strokes, none.item), (vec![], vec![], json!("object")));
     }
 
+    /// A result taken from the cache passes its reference check without clearing the error of a
+    /// failed run, and the next run that succeeds clears that error.
+    #[test]
+    fn a_passed_reference_check_keeps_a_failed_run_s_error() {
+        let _lock = crate::safe_mode::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner());
+        crate::safe_mode::set_active(None);
+        const ID: &str = "test.effect.lasterror";
+        let manifest = format!(r#"{{"id":"{ID}","name":"Test","version":"1","kind":"effect","api":2,"paintOutput":true,"params":{{}}}}"#);
+        // Traps on a long input.
+        let first = "(if (i32.gt_u (local.get $il) (i32.const 4096)) (then unreachable))";
+        let src = crate::wat::with_imports(&manifest, "", first, r#"{"objects":[]}"#);
+        registry::install(crate::Plugin::load(&wat::parse_str(&src).unwrap(), Limits::default()).unwrap());
+        let doc = Document::new(100.0, 100.0);
+        let line = |n: usize| PathData::single(SubPath::polyline(&(0..n).map(|i| Point::new(i as f64, 0.0)).collect::<Vec<_>>(), false));
+        let draw =
+            |path: &PathData| apply_painted(ID, &json!({}), path, Rect::new(0.0, 0.0, 1.0, 1.0), FillRule::NonZero, Paints::default(), Some(&doc));
+        let short = line(2);
+        assert!(draw(&short).is_some());
+        assert_eq!(last_error(ID), None);
+        assert!(draw(&line(1000)).is_none());
+        let failed = last_error(ID).unwrap();
+        assert!(draw(&short).is_some(), "from the cache");
+        assert_eq!(last_error(ID), Some(failed));
+        assert!(draw(&line(3)).is_some(), "a new run");
+        assert_eq!(last_error(ID), None);
+        registry::remove(ID);
+    }
+
     /// A first run that is cancelled leaves the next run to be the first: the launch marker names
     /// the plug-in during that one.
     #[test]

docs/plugins.md

@@ -233,7 +233,9 @@ Every run gets a **fresh instance**, so no state survives between calls except w
 [settings and cache](#settings-and-cache). Traps, exhausted budgets, malformed
 modules, bad manifests, out-of-range pointers, negative return codes and invalid output all become errors, and the
 document is left untouched: a filter's output is checked completely before one edit applies it. A live effect that
-fails draws the object's geometry unchanged; `plugin.info` reports why (`lastError`).
+fails draws the object's geometry unchanged; `plugin.info` reports why (`lastError`). A run that succeeds clears the
+error of a failed run. A check that finds every swatch and pattern a painted result names clears only the error of a
+refused swatch or pattern.
 
 wasmi is built with its *portable* (loop) dispatcher: the default tail-call dispatcher relies on LLVM
 sibling-call optimisation, and when that doesn't happen a long-running module overflows the host stack, which aborts