From df5d061b30ec4acf45072daa863cfd776689e4eb Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 03:12:56 -0800 Subject: [PATCH] Scripts: an error $.evalFile could not run, caught and thrown again, is reported where it was thrown again A failure $.evalFile raised for a file it could not run was placed by matching the uncaught error's message against the last such failure. A script that caught the failure and threw it again later, or threw a new error whose message held its text, was reported at the failed file's line instead of the second throw. The failure now carries a mark (HostError::Marked) with the exception. A catch clause drops the mark, a throw raises the value without one, and a finally block that ends normally throws it on with its mark, nested finally blocks included. Realm::error_mark gives the mark of the error a run ended with, and the engine looks up the file, line and column by it. --- crates/engine/src/script/mod.rs | 51 +++++++++++---------- crates/engine/src/tests/script_include.rs | 54 +++++++++++++++++++++++ crates/js/src/host.rs | 10 +++++ crates/js/src/host_tests.rs | 32 ++++++++++++++ crates/js/src/interp.rs | 33 +++++++++++++- crates/js/src/realm.rs | 11 +++++ docs/mcp.md | 4 +- docs/target-app-parity.md | 1 + 8 files changed, 170 insertions(+), 26 deletions(-) diff --git a/crates/engine/src/script/mod.rs b/crates/engine/src/script/mod.rs index 15d6c06e3..d9fbd0cf7 100644 --- a/crates/engine/src/script/mod.rs +++ b/crates/engine/src/script/mod.rs @@ -496,14 +496,11 @@ struct Host { script_map: RefCell, /// The same for each source `$.evalFile` gave, in order. eval_maps: RefCell>, - /// The last error `$.evalFile` raised for a file it could not run, with the file, line and - /// column that error is at. - eval_failure: RefCell>, + /// The file, line and column of each error `$.evalFile` raised for a file it could not run, in + /// order: the error's mark ([`HostError::Marked`]) is its index. + eval_failures: RefCell, u32, u32)>>, } -/// An error `$.evalFile` raised, as thrown, and the file, line and column it is at. -type EvalFailure = (String, Arc, u32, u32); - /// A run's host calls by kind, as its report gives them in `hostCallCounts`. #[derive(Default)] struct Counts { @@ -580,6 +577,15 @@ impl Host { HostError::Abort(limit.to_string()) } + /// `error`, marked as an error at `file`, `line` and `column`, the place [`located`] gives when + /// the run ends with it uncaught. + fn placed(&self, error: HostError, file: Arc, line: u32, column: u32) -> HostError { + let mut places = self.eval_failures.borrow_mut(); + let Ok(mark) = u32::try_from(places.len()) else { return error }; + places.push((file, line, column)); + HostError::Marked { error: Box::new(error), mark } + } + fn charge_call(&self, bytes: u64) -> std::result::Result<(), HostError> { let calls = self.calls.get() + 1; self.calls.set(calls); @@ -987,14 +993,12 @@ fn eval_file(host: &Host, args: Vec) -> std::result::Result 0 { format!("{}:{}:{}: {}", f.file, f.line, f.column, f.message) } else { f.message }; - if f.line > 0 { - *host.eval_failure.borrow_mut() = Some((message.clone(), f.file.clone(), f.line, f.column)); - } - match f.kind { + let error = match f.kind { include::FailKind::Limit => HostError::Abort(message), include::FailKind::Syntax => HostError::Syntax(message), include::FailKind::Error => HostError::Error(message), - } + }; + if f.line > 0 { host.placed(error, f.file, f.line, f.column) } else { error } })?; // A file without preprocessor lines is parsed here first, so its syntax error names its place. let options = vectorcraft_js::Options { max_source_units: include::MAX_TOTAL_BYTES, ..vectorcraft_js::Options::default() }; @@ -1002,8 +1006,7 @@ fn eval_file(host: &Host, args: Vec) -> std::result::Result (RunError, Option>) { (RunError { kind, message, line: f.line, column: f.column }, file) } -/// `e`, which the realm raised in source `tag` ([`Realm::error_source`]), at the line and column of -/// the file it is in, with that file's name unless it is the script's own. -fn located(host: &Host, tag: u32, e: &RunError) -> (RunError, Option>) { +/// `e`, which the realm raised in source `tag` ([`Realm::error_source`]) with host error mark `mark` +/// ([`Realm::error_mark`]), at the line and column of the file it is in, with that file's name +/// unless it is the script's own. +fn located(host: &Host, tag: u32, mark: Option, e: &RunError) -> (RunError, Option>) { let map = |m: &include::SourceMap, own: bool| { let (index, file, line, column) = m.locate(e.line, e.column); let message = if e.kind == RunErrorKind::Syntax { relocated(&e.message, e.line, e.column, line, column) } else { e.message.clone() }; (RunError { kind: e.kind.clone(), message, line, column }, (!own || index > 0).then_some(file)) }; - // An error `$.evalFile` raised for a file it could not run is at that file's place. - if let Some((_, file, line, column)) = host.eval_failure.borrow().as_ref().filter(|(thrown, ..)| e.message.contains(thrown.as_str())) { - let message = if e.kind == RunErrorKind::Syntax { relocated(&e.message, e.line, e.column, *line, *column) } else { e.message.clone() }; - return (RunError { kind: e.kind.clone(), message, line: *line, column: *column }, Some(file.clone())); + // An error `$.evalFile` raised for a file it could not run, and no script caught, is at that + // file's place. + if let Some((file, line, column)) = mark.and_then(|m| host.eval_failures.borrow().get(m as usize).cloned()) { + let message = if e.kind == RunErrorKind::Syntax { relocated(&e.message, e.line, e.column, line, column) } else { e.message.clone() }; + return (RunError { kind: e.kind.clone(), message, line, column }, Some(file)); } if e.line == 0 { return (e.clone(), None); @@ -1301,7 +1306,7 @@ impl Session { files: RefCell::new(files), script_map: RefCell::default(), eval_maps: RefCell::default(), - eval_failure: RefCell::default(), + eval_failures: RefCell::default(), }); let args = to_data(&Value::Object(request.args), &DataLimits::default(), 0, &mut (0, 0)) .map_err(|place| EngineError::BadParams { cmd: "script.run".into(), msg: format!("`args{place}` is nested more than 64 levels deep") })?; @@ -1316,7 +1321,7 @@ impl Session { .and_then(|()| realm.define_host_object("$", dollar_object(&host))) .and_then(|()| dialogs::install(&mut realm, &host)) .and_then(|()| realm.start(&expanded.source)) - .map_err(|e| located(&host, realm.error_source(), &e)) + .map_err(|e| located(&host, realm.error_source(), realm.error_mark(), &e)) } }; if let Err((e, file)) = started { @@ -1388,7 +1393,7 @@ impl Session { fn script_failed(&mut self, job: Job, e: &RunError) -> Result { job.host.dialogs.stopped(); let fuel_left = job.realm.fuel_left(); - let (e, file) = located(&job.host, job.realm.error_source(), e); + let (e, file) = located(&job.host, job.realm.error_source(), job.realm.error_mark(), e); let mut report = failure_report(&job.name, &e, file.as_deref(), &job.host, fuel_left, job.realm.limits().fuel.saturating_sub(fuel_left)); close_run(self, &job.host, &mut report, ending(&e.kind)); drop(job); diff --git a/crates/engine/src/tests/script_include.rs b/crates/engine/src/tests/script_include.rs index 945e6ca16..44b2e449b 100644 --- a/crates/engine/src/tests/script_include.rs +++ b/crates/engine/src/tests/script_include.rs @@ -388,3 +388,57 @@ fn script_dialogs_in_included_and_evaluated_files() { assert_eq!(report["dialogs"][0]["answer"], true, "{report}"); let _ = std::fs::remove_dir_all(base); } + +/// The line, column and file name of a failed run's report, and its console error line (the only +/// line of a run without output that starts with the script's name). +fn place(report: &Value) -> (u64, u64, Option<&str>, &str) { + let file = report.get("file").map(|f| f.as_str().unwrap().rsplit(['/', '\\']).next().unwrap()); + let lines = report["console"]["lines"].as_array().unwrap(); + let error = lines.iter().filter_map(Value::as_str).find(|l| l.starts_with("main.jsx")).unwrap(); + (report["line"].as_u64().unwrap(), report["column"].as_u64().unwrap(), file, error) +} + +#[test] +fn script_eval_file_error_caught_and_thrown_again_is_at_the_new_throw() { + let base = folder("rethrow"); + write(&base.join("main/sub/broken.jsx"), "var a = 1;\n\nvar = 2;"); + write(&base.join("main/sub/directive.jsx"), "\n#frobnicate 1"); + write(&base.join("main/sub/rethrow.jsx"), "var r = 1;\n throw saved;"); + write(&base.join("main/sub/outer.jsx"), "var q = 1;\n$.evalFile('broken.jsx');"); + write(&base.join("main/sub/outer2.jsx"), "try { $.evalFile('broken.jsx'); } catch (e) {\n\n throw e;\n}"); + let caught = "var saved;\ntry { $.evalFile('sub/broken.jsx'); } catch (e) { saved = e; }\n"; + let cases = [ + // Thrown again in the script, or in another file $.evalFile runs. + (format!("{caught}var b = 2;\n throw saved;"), (4, 3, None)), + (format!("{caught}$.evalFile('sub/rethrow.jsx');"), (2, 3, Some("rethrow.jsx"))), + // A new error whose message holds the failure's text, or is the same text. + (format!("{caught}throw new Error(saved.message);"), (3, 1, None)), + (format!("{caught}throw new SyntaxError(saved.message);"), (3, 1, None)), + // A second failure, caught, does not move the first one. + (format!("{caught}try {{ $.evalFile('sub/directive.jsx'); }} catch (e) {{}}\n throw saved;"), (4, 2, None)), + // A second failure, uncaught, is at its own file's place. + (format!("{caught}$.evalFile('sub/directive.jsx');"), (2, 1, Some("directive.jsx"))), + // An uncaught failure is at the file's place, through a finally block and from a nested file. + ("var a = 1;\ntry { $.evalFile('sub/broken.jsx'); } finally { a = 2; }".to_string(), (3, 5, Some("broken.jsx"))), + ("$.evalFile('sub/outer.jsx');".to_string(), (3, 5, Some("broken.jsx"))), + // Caught in a nested file and thrown again there. + ("$.evalFile('sub/outer2.jsx');".to_string(), (3, 5, Some("outer2.jsx"))), + // A failure caught in a run after one that failed with it, and a different error. + ("try { $.evalFile('sub/broken.jsx'); } catch (e) {}\nthrow new Error('other');".to_string(), (2, 1, None)), + ]; + let mut s = Session::new(); + let main = base.join("main/main.jsx"); + for (text, (line, column, file)) in cases { + write(&main, &text); + assert!(s.execute("script.run", &json!({"path": main.to_string_lossy()})).is_err(), "{text}"); + let report = s.script_report().unwrap().clone(); + let (l, c, f, error) = place(&report); + assert_eq!((report["kind"].as_str(), l, c, f), (Some("thrown"), line, column, file), "{text}\n{report}"); + let at = match file { + Some(f) => format!("{f}:{line}:{column}: "), + None => format!("main.jsx:{line}:{column}: "), + }; + assert!(error.starts_with("main.jsx") && error.contains(&at), "{error}"); + } + let _ = std::fs::remove_dir_all(base); +} diff --git a/crates/js/src/host.rs b/crates/js/src/host.rs index 997826f3e..b0166dde2 100644 --- a/crates/js/src/host.rs +++ b/crates/js/src/host.rs @@ -64,6 +64,11 @@ pub enum HostError { /// it only while [`crate::Realm::wait_flag`] is set; otherwise the run ends with an internal /// error. Wait, + /// `error`, with a mark the host chose. When the run ends with the exception `error` throws, + /// or with the limit it ends the run with, [`crate::Realm::error_mark`] gives `mark`. A `catch` + /// clause that takes the exception drops the mark, so a `throw` of the caught value later + /// raises it without one; a `finally` block that ends normally keeps it. + Marked { error: Box, mark: u32 }, } /// A function the host gives a script, with its arguments converted. @@ -416,6 +421,11 @@ impl Interp { HostError::Type(message) => self.throw_host(ErrorType::Type, &message), HostError::Range(message) => self.throw_host(ErrorType::Range, &message), HostError::Syntax(message) => self.throw_host(ErrorType::Syntax, &message), + HostError::Marked { error, mark } => { + let abrupt = self.host_error(*error)?; + self.thrown_mark = Some(mark); + Ok(abrupt) + } } } diff --git a/crates/js/src/host_tests.rs b/crates/js/src/host_tests.rs index 3492cf653..963f91776 100644 --- a/crates/js/src/host_tests.rs +++ b/crates/js/src/host_tests.rs @@ -260,6 +260,38 @@ fn errors_from_the_host_can_be_caught_and_an_abort_cannot() { assert_eq!((e.kind, e.message.as_str()), (RunErrorKind::Limit(LimitKind::Host), "stopped by the host")); } +#[test] +fn a_marked_host_error_keeps_its_mark_only_while_no_catch_takes_it() { + let mut r = realm(1_000_000); + let f: HostFunction = Rc::new(|a| { + let error = match a.first() { + Some(Data::String(k)) if k == "abort" => HostError::Abort("stopped".into()), + _ => HostError::Syntax("bad file".into()), + }; + Err(HostError::Marked { error: Box::new(error), mark: 7 }) + }); + r.define_host_object("host", vec![("f".into(), HostItem::Function(f))]).unwrap(); + let cases = [ + ("host.f();", Some(7)), + ("try { host.f(); } finally { var x = 1; }", Some(7)), + ("try { try { host.f(); } finally { 1; } } finally { 2; }", Some(7)), + ("function g() { host.f(); }\ng();", Some(7)), + ("try { host.f(); } finally { try { try { host.f(); } finally { 1; } } catch (e) {} }", Some(7)), + ("var saved; try { host.f(); } catch (e) { saved = e; }\nthrow saved;", None), + ("try { host.f(); } catch (e) { throw e; }", None), + ("try { try { host.f(); } finally { 1; } } catch (e) { throw e; }", None), + ("var m; try { host.f(); } catch (e) { m = e.message; }\nthrow new SyntaxError(m);", None), + ("do { try { host.f(); } finally { break; } } while (false);\nthrow new Error('other');", None), + ("try { host.f('abort'); } catch (e) { 'caught' }", Some(7)), + ]; + for (source, mark) in cases { + assert!(r.run(source).is_err(), "{source}"); + assert_eq!(r.error_mark(), mark, "{source}"); + } + assert_eq!(r.run("1").unwrap(), JsValue::Number(1.0)); + assert_eq!(r.error_mark(), None, "a later run that ends normally has no mark"); +} + #[test] fn an_error_says_where_in_the_source_it_was_raised() { let mut r = realm(1_000_000); diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index 6c413e6d7..0e2f1e7ed 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -197,6 +197,16 @@ pub(crate) struct Interp { pub(crate) evaluated: u32, /// The tag of the source the last run's error was raised in. pub(crate) error_tag: u32, + /// The mark of the host error ([`crate::HostError::Marked`]) whose exception or limit is + /// propagating. + pub(crate) thrown_mark: Option, + /// The marked exceptions `finally` blocks are running for, innermost last, each with its mark, + /// which the block's end throws it on with. A block left by `break`, `continue` or `return` + /// leaves its entry until the run ends, so there are at most as many as the host errors the + /// run's host marked. + pub(crate) finally_marks: Vec<(ObjId, u32)>, + /// The mark of the host error the last run ended with. + pub(crate) error_mark: Option, } /// The bytes all live [`HeldBytes`] hold, and the bytes dropped holders released that the heap's @@ -394,6 +404,9 @@ impl Interp { source_tag: crate::realm::UNKNOWN_SOURCE, evaluated: 0, error_tag: crate::realm::UNKNOWN_SOURCE, + thrown_mark: None, + finally_marks: Vec::new(), + error_mark: None, }; crate::builtins::install(&mut interp); interp.fuel = limits.fuel; @@ -552,6 +565,7 @@ impl Interp { roots.values.extend(self.stack.iter()); roots.values.extend(self.temp_roots.iter()); roots.values.push(&self.last_thrown); + roots.objects.extend(self.finally_marks.iter().map(|&(id, _)| id)); for f in &self.frames { roots.values.push(&f.this); roots.values.push(&f.completion); @@ -640,6 +654,8 @@ impl Interp { loop { let Some(frame) = self.frames.last_mut() else { return Err(Abrupt::Throw(v)) }; if let Some(h) = frame.code.handler(frame.pc.saturating_sub(1)) { + // A catch clause starts with `PushCatch`; every other handler is a `finally` block's. + let caught = matches!(frame.code.ops.get(h.target as usize), Some(Op::PushCatch(_))); let target_len = frame.base + h.depth as usize; while frame.env_depth > h.env_depth { let outer = self.heap.env(frame.env).and_then(Env::outer); @@ -652,6 +668,9 @@ impl Interp { return internal("handler stack deeper than the stack"); } self.stack.truncate(target_len); + if let (Some(mark), false, Value::Object(id)) = (self.thrown_mark.take(), caught, &v) { + self.finally_marks.push((*id, mark)); + } self.stack.push(v); self.throw_site = None; return Ok(()); @@ -753,7 +772,10 @@ impl Interp { Op::Jump(t) => self.jump_to(t), Op::JumpIfFalse(t) | Op::JumpIfTrue(t) => self.op_branch(t, matches!(op, Op::JumpIfTrue(_)), false), Op::JumpIfFalseKeep(t) | Op::JumpIfTrueKeep(t) => self.op_branch(t, matches!(op, Op::JumpIfTrueKeep(_)), true), - Op::Throw => Err(Abrupt::Throw(self.pop()?)), + Op::Throw => { + self.thrown_mark = None; + Err(Abrupt::Throw(self.pop()?)) + } Op::ThrowReferenceError(i) => self.op_throw_reference_error(i), Op::EndFinally(t) => self.op_end_finally(t), Op::NewObject => self.op_new_object(), @@ -835,7 +857,14 @@ impl Interp { return self.pop().map(drop); } if kind == 1.0 { - return Err(Abrupt::Throw(self.pop()?)); + let v = self.pop()?; + if let Value::Object(o) = v + && let Some(i) = self.finally_marks.iter().rposition(|&(id, _)| id == o) + { + self.thrown_mark = self.finally_marks.get(i).map(|&(_, mark)| mark); + self.finally_marks.truncate(i); + } + return Err(Abrupt::Throw(v)); } let k = (kind - 2.0) as usize; let target = self.frame()?.code.finally_tables.get(table as usize).and_then(|t| t.get(k)).copied(); diff --git a/crates/js/src/realm.rs b/crates/js/src/realm.rs index 9ab7623dc..4754dc87b 100644 --- a/crates/js/src/realm.rs +++ b/crates/js/src/realm.rs @@ -224,6 +224,9 @@ impl Realm { interp.host_abort = None; interp.waiting = false; interp.error_tag = UNKNOWN_SOURCE; + interp.thrown_mark = None; + interp.finally_marks.clear(); + interp.error_mark = None; let strict = interp.options.strict; interp.source_tag = 0; let code = interp.encode_source(source).and_then(|units| interp.compile_source(&units, strict, CodeKind::Global)); @@ -244,6 +247,8 @@ impl Realm { let site = interp.throw_site.take(); interp.error_tag = site.as_ref().map_or(UNKNOWN_SOURCE, |(code, _)| code.source.tag); let (line, column) = site.map_or((0, 0), |(code, pc)| code.line_column(pc)); + interp.error_mark = interp.thrown_mark.take().filter(|_| matches!(result, Err(Abrupt::Throw(_) | Abrupt::Limit(LimitKind::Host)))); + interp.finally_marks.clear(); interp.stack.clear(); interp.frames.clear(); interp.temp_roots.clear(); @@ -359,6 +364,12 @@ impl Realm { self.interp.error_tag } + /// The mark of the host error ([`crate::HostError::Marked`]) the last run ended with, when no + /// `catch` clause took that error's exception on its way out; `None` otherwise. + pub fn error_mark(&self) -> Option { + self.interp.error_mark + } + /// Fuel the run has left. pub fn fuel_left(&self) -> u64 { self.interp.fuel diff --git a/docs/mcp.md b/docs/mcp.md index dabbf9dcf..261660e12 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1603,7 +1603,9 @@ VectorCraft reads them by these rules of its own: Windows device name or stream, and a computer on the network are refused, and so is every path for a script given as `source`. A `JSINCLUDE` folder outside the script's folder is passed over. - A missing or refused included file ends the run before any of it runs. An error in an included file or a file `$.evalFile` - ran is reported at that file's line and column, and the report's `file` names the file. + ran is reported at that file's line and column, and the report's `file` names the file. So is an error `$.evalFile` + throws for a file it cannot run, such as a syntax error, when no `catch` takes it. When a `catch` takes that error + and a later `throw` raises it again, the error is reported where that `throw` is, as any other thrown error is. The run is one transaction. When the script ends, each document it changed gets one undo step named Run Script. A run that changes no document adds no undo step. The journal records each command the run finished, as it records diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 764b7ebe8..54a72b907 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -731,6 +731,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - Scripts: File › Scripts › Other Script… and `script.run` run a JavaScript file against the open documents as one transaction with one undo step for each changed document. A failed or stopped run puts every document back. `crates/engine/src/tests/script_run.rs`. - Scripts: Window › Script Console shows each run's `$.write` and `$.writeln` output, `vc.log` entries, uncaught error with the script's name, line and column, and a report of its time, host calls and slowest commands, keeping at most 2,048 lines and counting the lines it dropped. `script.run`'s report carries the same data, and `script.console`, `script.console.clear` and `ui.copyScriptConsole` read, clear and copy it. `crates/engine/src/tests/script_console.rs`. - Scripts: a run reads its preprocessor lines before any of it runs: `#include` and `//@include` put another file in place of the line, looked for beside the including file, in the `#includepath` folders and in `JSINCLUDE`; `#script` names the run; `#target` and `#strict off` are taken, and `#strict on`, `#targetengine` and `#engine` are refused. `$.evalFile` runs a file with the same globals and returns its last value, and `$.fileName` and `$.includePath` give the file being run and the include folders. A script run from a file reads these files only in its folder and below, without `..` names or links that lead out, and an error in one is reported at its own file, line and column ([preprocessor directives](https://extendscript.docsforadobe.dev/extendscript-tools-features/preprocessor-directives/), [the `$` object](https://extendscript.docsforadobe.dev/extendscript-tools-features/dollar-object/); `crates/engine/src/tests/script_include.rs`). + - Scripts: an error `$.evalFile` throws for a file it cannot run is reported at that file's line and column only while no `catch` has taken it. Caught and thrown again later, in the script or in another file, it is reported where it was thrown again, and a new error whose message holds its text is reported where that error was thrown. VectorCraft's own rule. `crates/engine/src/tests/script_include.rs`, `crates/js/src/host_tests.rs`. - Scripts folder: File › Scripts lists the `.js` and `.jsx` files of the `Scripts` folder next to the preferences, each named after its file without the extension, with each subfolder as a submenu, above Check Script… and Other Script… (https://helpx.adobe.com/illustrator/desktop/automate-visualize-data/automate-actions/install-and-run-scripts.html). VectorCraft's own rules: the folder is read again when the window becomes active and when the File menu opens, links are not followed, and one read stops at 2,048 entries, 512 scripts, 128 subfolders, 4 levels or 250 ms. `script.listInstalled` returns the listing. `crates/engine/src/tests/script_folder.rs`, `crates/ui-egui/src/tests/scripts_menu.rs`. - Scripts: `alert`, `confirm` and `prompt` open dialogs during a run, each with a Stop button that ends the run and puts every document back. In the app the run waits at the call for the person's answer; a run over MCP or the command line takes its answers in order from `script.run`'s `answers`, and the report lists each dialog with its answer. `crates/engine/src/tests/script_dialogs.rs`, `crates/ui-egui/src/tests/script_dialogs.rs`. - Parameter keys: the engine refuses a key that a command's params description doesn't name before the command runs, in every build and for every caller, including the app's own calls, the commands a command runs and the steps of `command.batch` and `action.batch`. An action recorded by an older version still plays from the Actions panel and the Batch dialog (`Session::replay`). `crates/engine/tests/it/param_keys.rs`.