Merge pull request #6653 from dotCooCoo/fix/script-throw-site-through-finally

Scripts: an uncaught error that passes through a finally block is reported where it was thrown
2026-11-12 03:05:44 +00:00 · 2026-11-12 02:53:22 +07:00
parent f9ff1caf54 1f835d94c4
commit dd6b00c228
6 changed files with 113 additions and 2 deletions
@@ -563,3 +563,19 @@ fn script_eval_file_error_caught_and_thrown_again_is_at_the_new_throw() {
    }
    let _ = std::fs::remove_dir_all(base);
 }
+
+#[test]
+fn script_error_that_passes_through_a_finally_block_is_at_its_throw() {
+    let base = folder("finally");
+    let main = base.join("main/main.jsx");
+    let mut s = Session::new();
+    for text in ["try {\n  throw new Error('x');\n} finally {\n  var z=1;\n}", "try {\n  null();\n} finally {\n  var z=1;\n}"] {
+        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"), 2, 3, None), "{text}\n{report}");
+        assert!(error.contains("main.jsx:2:3: "), "{error}");
+    }
+    let _ = std::fs::remove_dir_all(base);
+}
@@ -323,6 +323,71 @@ fn an_error_says_where_in_the_source_it_was_raised() {
    assert_eq!(e.message, "TypeError: no");
 }

+/// An error a `finally` block passes on unchanged is reported where it was raised; one the block
+/// throws itself, or a catch throws again, is reported at that new throw. Each finally block holds
+/// a statement, so its end is not at the throw's position.
+#[test]
+fn an_error_that_passes_through_a_finally_block_says_where_it_was_raised() {
+    let mut r = realm(1_000_000);
+    let cases = [
+        ("try {\n  throw new Error('x');\n} finally {\n  var z=1;\n}", (2, 3), "Error: x"),
+        ("try {\n  null();\n} finally {\n  var z=1;\n}", (2, 3), "TypeError: the value is not a function"),
+        ("try {\n  throw 5;\n} finally {\n  var z=1;\n}", (2, 3), "5"),
+        ("try {\n  try { throw new Error('x'); } finally { var z=1; }\n} catch (e) { throw e; }", (3, 15), "Error: x"),
+        ("try {\n  try { throw new Error('x'); } finally { var z=1; }\n} finally { var z=1; }", (2, 9), "Error: x"),
+        ("function f() {\n  try { throw new Error('x'); } finally { var z=1; }\n}\nf();", (2, 9), "Error: x"),
+        ("try { throw new Error('x'); } catch (e) {\n  throw e;\n}", (2, 3), "Error: x"),
+        ("try { throw new Error('x'); } finally {\n  throw new Error('y');\n}", (2, 3), "Error: y"),
+        ("var e = new Error('x');\ntry {\n  throw e;\n} finally {\n  throw e;\n}", (5, 3), "Error: x"),
+        ("try {\n  throw new Error('outer');\n} finally {\n  try { throw new Error('inner'); } catch (e) {}\n}", (2, 3), "Error: outer"),
+        ("function g(){ try { throw new Error('inner'); } catch(e) {} }\ntry { throw new Error('outer'); } finally { g(); }", (2, 7), "Error: outer"),
+        (
+            "for (var i = 0; i < 2000; i++) { try { throw 1; } finally { continue; } }\ntry {\n  throw new Error('x');\n} finally {\n  var z=1;\n}",
+            (3, 3),
+            "Error: x",
+        ),
+    ];
+    for (source, (line, column), message) in cases {
+        let e = r.run(source).unwrap_err();
+        assert_eq!((e.kind, e.line, e.column, e.message.as_str()), (RunErrorKind::Thrown, line, column, message), "{source}");
+    }
+    let mut nested = String::new();
+    for _ in 0..40 {
+        nested.push_str("try {\n");
+    }
+    nested.push_str("  throw new Error('deep');\n");
+    for _ in 0..40 {
+        nested.push_str("} finally { var z=1; }\n");
+    }
+    let e = r.run(&nested).unwrap_err();
+    assert_eq!((e.line, e.column), (41, 3), "forty finally blocks");
+    for source in [
+        "function f(){ try { throw new Error('x'); } finally { return 4; } }\nf();",
+        "while(true) { try { throw new Error('x'); } finally { break; } }\n7;",
+        "for(var i=0;i<1;i++){ try { throw new Error('x'); } finally { continue; } }\n7;",
+    ] {
+        assert!(r.run(source).is_ok(), "{source}");
+    }
+}
+
+#[test]
+fn a_resumed_run_reports_an_error_that_passes_through_a_finally_block_where_it_was_raised() {
+    let mut r = realm(1_000_000);
+    r.start("try {\n  throw new Error('x');\n} finally {\n  var a=1;\n}").unwrap();
+    let e = finish(&mut r, 1).0.unwrap_err();
+    assert_eq!((e.kind, e.line, e.column), (RunErrorKind::Thrown, 2, 3));
+}
+
+#[test]
+fn a_marked_host_error_that_passes_through_a_finally_block_says_where_it_was_raised() {
+    let mut r = realm(1_000_000);
+    let f: HostFunction = Rc::new(|_| Err(HostError::Marked { error: Box::new(HostError::Syntax("bad file".into())), mark: 7 }));
+    r.define_host_object("host", vec![("f".into(), HostItem::Function(f))]).unwrap();
+    let e = r.run("try {\n  host.f();\n} finally { var z=1; }").unwrap_err();
+    assert_eq!((e.kind, e.line, e.column), (RunErrorKind::Thrown, 2, 3));
+    assert_eq!(r.error_mark(), Some(7));
+}
+
 #[test]
 fn advancing_without_a_run_is_an_error() {
    let mut r = realm(1_000);
@@ -232,6 +232,13 @@ pub(crate) struct Interp {
    /// runs. An entry whose slot no longer holds its exception belongs to a block that `break`,
    /// `continue` or `return` left, and [`Self::drop_left_finally_marks`] removes it.
    pub(crate) finally_marks: Vec<(ObjId, u32, usize)>,
+    /// Where the exceptions `finally` blocks are running for were raised, innermost last, each with
+    /// the stack slot that holds the exception while the block runs. The block's end throws the
+    /// exception on from that site. An entry whose slot is at or above the stack's length belongs to
+    /// a block that was left, and [`Self::drop_left_finally_sites`] removes it. [`Self::unwind`]
+    /// removes the entries at or above a handler's slot before it adds one, so the slots rise and
+    /// the list is never longer than the stack.
+    pub(crate) finally_sites: Vec<(ThrowSite, usize)>,
    /// The mark of the host error the last run ended with.
    pub(crate) error_mark: Option<u32>,
    /// [`crate::bytecode::Source::strict_writes`] for the scripts the realm runs.
@@ -443,6 +450,7 @@ impl Interp {
            error_tag: crate::realm::UNKNOWN_SOURCE,
            thrown_mark: None,
            finally_marks: Vec::new(),
+            finally_sites: Vec::new(),
            error_mark: None,
            strict_writes: false,
        };
@@ -605,8 +613,15 @@ impl Interp {
        self.finally_marks.retain(|&(id, _, slot)| matches!(stack.get(slot), Some(Value::Object(o)) if *o == id));
    }

+    /// Removes the [`Self::finally_sites`] entries whose stack slot is at or above `len`: those of
+    /// `finally` blocks that were left.
+    fn drop_left_finally_sites(&mut self, len: usize) {
+        self.finally_sites.retain(|&(_, slot)| slot < len);
+    }
+
    pub(crate) fn collect_garbage(&mut self) {
        self.drop_left_finally_marks();
+        self.drop_left_finally_sites(self.stack.len());
        let mut roots = Roots::default();
        roots.values.extend(self.stack.iter());
        roots.values.extend(self.temp_roots.iter());
@@ -614,6 +629,7 @@ impl Interp {
        roots.values.push(&self.last_error);
        roots.values.push(&self.passing);
        roots.objects.extend(self.finally_marks.iter().map(|&(id, _, _)| id));
+        roots.codes.extend(self.finally_sites.iter().map(|((code, _), _)| &**code));
        for f in &self.frames {
            roots.values.push(&f.this);
            roots.values.push(&f.completion);
@@ -725,11 +741,15 @@ impl Interp {
                }
                self.stack.truncate(target_len);
                self.drop_left_finally_marks();
+                self.drop_left_finally_sites(target_len);
                if let (Some(mark), false, Value::Object(id)) = (self.thrown_mark.take(), caught, &v) {
                    self.finally_marks.push((*id, mark, target_len));
                }
                self.stack.push(v);
-                self.throw_site = None;
+                let site = self.throw_site.take();
+                if let (false, Some(site)) = (caught, site) {
+                    self.finally_sites.push((site, target_len));
+                }
                return Ok(());
            }
            let Some(frame) = self.frames.pop() else { return Err(Abrupt::Throw(v)) };
@@ -952,6 +972,11 @@ impl Interp {
                self.thrown_mark = self.finally_marks.get(i).map(|&(_, mark, _)| mark);
                self.finally_marks.truncate(i);
            }
+            let slot = self.stack.len();
+            if let Some(i) = self.finally_sites.iter().rposition(|&(_, s)| s == slot) {
+                self.throw_site = self.finally_sites.get(i).map(|(site, _)| site.clone());
+                self.finally_sites.truncate(i);
+            }
            return Err(Abrupt::Throw(v));
        }
        let k = (kind - 2.0) as usize;
@@ -235,6 +235,7 @@ impl Realm {
        interp.error_tag = UNKNOWN_SOURCE;
        interp.thrown_mark = None;
        interp.finally_marks.clear();
+        interp.finally_sites.clear();
        interp.error_mark = None;
        let strict = interp.options.strict;
        interp.source_tag = 0;
@@ -258,6 +259,7 @@ impl Realm {
        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.finally_sites.clear();
        interp.stack.clear();
        interp.frames.clear();
        interp.temp_roots.clear();
@@ -1859,7 +1859,9 @@ VectorCraft reads files by these rules of its own:
 Window › Script Console shows the session's script lines, oldest first: each run's start, its `$.write` and
 `$.writeln` output, its `vc.log` entries, its uncaught error as `name:line:column: message` (or `name: file:line:column: message` when the error is in another
 file), its end, and a report of
-its time, host calls and slowest commands. Each line shows the run's number and the script's name. Line feeds,
+its time, host calls and slowest commands. An uncaught error that a `finally` block passes on unchanged has the line
+and column where it was thrown; an error the `finally` block throws has the line and column of that throw. Each line
+shows the run's number and the script's name. Line feeds,
 carriage returns and the two together break lines, and other control characters show as their Unicode Control
 Pictures. The console keeps at most 2,048 lines and 1 MiB of text and drops the oldest lines past that. A line longer
 than 4,096 bytes, and a script's name longer than 256 bytes, is cut with `…`. When the console has dropped or cut any lines, a note at the top counts them. The panel lays out only the
@@ -736,6 +736,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
  - Scripts: the `$` object has the other members the Dollar object page lists (https://extendscript.docsforadobe.dev/extendscript-tools-features/dollar-object/), from `$.about()` to `$.version`, and scripts have the global `localize(object, ...values)` (https://extendscript.docsforadobe.dev/extendscript-tools-features/localizing-extendscript-strings/), `toSource(value)` and `Object.prototype.toSource()`. Each run starts with the interface's locale, and what a script changes in `$` lasts only for that run. `crates/engine/src/tests/script_dollar.rs`, `crates/js/src/extension_tests.rs`.
  - Scripts: the Script Console panel's report line reads as `script.console` gives it, `time unavailable` included on a platform without a clock; counts of host calls, dropped lines and cut lines take the singular for one, in each language's own plural forms; and a run that is no longer available still gets its end line (`crates/engine/src/tests/script_console.rs`, `crates/ui-egui/src/panels/tests_script_console.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: an uncaught error that one or more `finally` blocks pass on unchanged, in the script or in a function it called, is reported at the line and column where it was thrown, in `script.run`'s report and the Script Console. An error a `finally` block throws is reported at that throw, and one its `return`, `break` or `continue` discards is not reported. VectorCraft's own rule. `crates/engine/src/tests/script_include.rs`, `crates/js/src/host_tests.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, `#strict on` turns on strict write checks (below), and `#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: `File` and `Folder` ([File object](https://extendscript.docsforadobe.dev/file-system-access/file-object/), [Folder object](https://extendscript.docsforadobe.dev/file-system-access/folder-object/)) name files and folders by platform or URI path, with `name`, `fsName`, `absoluteURI`, `relativeURI`, `parent`, `Folder.current` and `encode`/`decode`, and read them: `exists`, `length`, the dates, `getFiles` with a mask or a function, and `open('r')` with `read`, `readln`, `readch`, `seek`, `tell` and `close`, decoding by byte order mark, `encoding`, UTF-8 or CP1252. VectorCraft's own rules: a script reads only in its own folder and below (and inside the automation read roots when they are set); any other path and a network or device path are refused before anything is looked up, and a link that leads out is refused as well (`false` or `null`, with `error` set); a file is read whole up to 8 MiB and a run reads up to 32 MiB; writing, the file dialogs and `execute` are not available yet. `crates/engine/src/tests/script_io.rs`.
  - Scripts: `#strict on` (and `//@strict on`) runs the script with the checks the [`$` object](https://extendscript.docsforadobe.dev/extendscript-tools-features/dollar-object/) help page gives for `$.strict`: an assignment to a read-only property throws a `TypeError`, and so does a new property on an object that takes none. VectorCraft's own rules: a getter-only property counts as read-only, the objects that take no new properties are the non-extensible ones (`Object.preventExtensions`, `seal`, `freeze`, and the frozen `vc` and `$` objects), the last `#strict` line of the script and its includes sets the whole run, a file `$.evalFile` runs keeps its own line's checks for its code, and the rest of ES5 strict mode stays with `'use strict'`. Without the line, runs are unchanged. `crates/engine/src/tests/script_include.rs` (`strict_on_*`, `strict_mode_in_eval_file_is_scoped`), `crates/js/src/interp_tests.rs` (`strict_writes_are_separate_from_es5_strict`).