error: gate the <parse> stack frame on the parser having recorded a location, not on sourceURL

scripts/build/deps/webkit.ts

@@ -3,7 +3,9 @@
  * for local mode. Override via `--webkit-version=<hash>` to test a branch.
  * From https://github.com/oven-sh/WebKit releases.
  */
-export const WEBKIT_VERSION = "447082ab6897278727b44e1ba3c326ae6e1504c3";
+// Preview build of https://github.com/oven-sh/WebKit/pull/407 (ErrorInstance::hasParseLocation()).
+// Replace with the main sha once it lands.
+export const WEBKIT_VERSION = "autobuild-preview-pr-407-f6d8e2a0";
 
 /**
  * WebKit (JavaScriptCore) — the JS engine.

src/jsc/bindings/FormatStackTraceForJS.cpp

@@ -177,7 +177,10 @@ WTF::String formatStackTrace(
 
     if (errorInstance) {
         if (JSC::ErrorInstance* err = dynamicDowncast<JSC::ErrorInstance>(errorInstance)) {
-            if (err->errorType() == ErrorType::SyntaxError && (stackTrace.isEmpty() || stackTrace.at(0).sourceURL(vm) != err->sourceURL())) {
+            // The <parse> frame shows the URL and line addErrorInfo() recorded for a source JSC's parser rejected.
+            // A sourceURL alone doesn't mean that (structured clones and errors whose frames GC already flushed
+            // have one too), and a parser error for a source without a URL (new Function) has nothing to show.
+            if (err->hasParseLocation() && !err->sourceURL().isEmpty() && (stackTrace.isEmpty() || stackTrace.at(0).sourceURL(vm) != err->sourceURL())) {
                 // There appears to be an off-by-one error.
                 // The following reproduces the issue:
                 // /* empty comment */
@@ -193,15 +196,12 @@ WTF::String formatStackTrace(
                 String sourceURLForFrame = err->sourceURL();
 
                 // If it's not a Zig::GlobalObject, don't bother source-mapping it.
-                if (globalObject && !sourceURLForFrame.isEmpty()) {
-                    // https://github.com/oven-sh/bun/issues/3595
-                    if (!sourceURLForFrame.isEmpty()) {
-                        remappedFrame.source_url = Bun::toStringRef(sourceURLForFrame);
-                        // This ensures the lifetime of the sourceURL is accounted for correctly
-                        Bun__remapStackFramePositions(getBunVM(), &remappedFrame, 1);
-
-                        sourceURLForFrame = remappedFrame.source_url.toWTFString();
-                    }
+                if (globalObject) {
+                    remappedFrame.source_url = Bun::toStringRef(sourceURLForFrame);
+                    // This ensures the lifetime of the sourceURL is accounted for correctly
+                    Bun__remapStackFramePositions(getBunVM(), &remappedFrame, 1);
+
+                    sourceURLForFrame = remappedFrame.source_url.toWTFString();
                 }
 
                 // there is always a newline before each stack frame line, ensuring that the name + message

test/js/bun/test/stack.test.ts

@@ -1,7 +1,9 @@
 import { $ } from "bun";
-import { expect, test } from "bun:test";
-import { bunEnv, bunExe, bunRun, normalizeBunSnapshot } from "harness";
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, bunRun, normalizeBunSnapshot, tempDir } from "harness";
 import { join } from "node:path";
+import { pathToFileURL } from "node:url";
+import vm from "node:vm";
 
 test("name property is used for function calls in Error.stack", () => {
   function WRONG() {
@@ -149,3 +151,219 @@ test("Async functions frame should be included in stack trace", async () => {
         at async <anonymous> (file:NN:NN)"
   `);
 });
+
+// When JSC's parser rejects a source it records that source's URL and line on the
+// SyntaxError, and .stack renders them as a synthetic "at <parse> (url:line)" frame.
+// Every other SyntaxError must format like node's: the header followed by the real frames.
+describe("SyntaxError .stack", () => {
+  test("new SyntaxError() has no <parse> frame", () => {
+    const err = new SyntaxError("user made");
+    expect(normalizeBunSnapshot(err.stack!)).toMatchInlineSnapshot(`
+      "SyntaxError: user made
+          at <anonymous> (file:NN:NN)"
+    `);
+  });
+
+  const caught = (fn: () => unknown) => {
+    try {
+      fn();
+    } catch (e) {
+      return e as Error;
+    }
+    throw new Error("expected fn to throw");
+  };
+
+  test.each([
+    ["SyntaxError() called without new", () => SyntaxError("user made")],
+    ["a SyntaxError subclass", () => new (class MySyntaxError extends SyntaxError {})("user made")],
+    ["JSON.parse()", () => caught(() => JSON.parse("{"))],
+    ["new RegExp()", () => caught(() => new RegExp("[a-0]"))],
+  ])("%s has no <parse> frame", (_, make) => {
+    const stack = make().stack!;
+    expect(stack).not.toContain("<parse>");
+    // The real frames are still there; the outermost one is this test callback.
+    expect(stack.split("\n").at(-1)).toMatch(/^    at .*stack\.test\.ts:\d+:\d+\)?$/);
+  });
+
+  test("Error.captureStackTrace() on a SyntaxError adds no <parse> frame", () => {
+    const err = new SyntaxError("captured");
+    Error.captureStackTrace(err);
+    expect(normalizeBunSnapshot(err.stack!)).toMatchInlineSnapshot(`
+      "SyntaxError: captured
+          at <anonymous> (file:NN:NN)"
+    `);
+  });
+
+  test("a SyntaxError with no frames at all is just the header", () => {
+    const err = new SyntaxError("no frames");
+    // Math.max is not on the stack, so every frame is dropped.
+    Error.captureStackTrace(err, Math.max);
+    expect(err.stack).toBe("SyntaxError: no frames");
+  });
+
+  test("Bun.inspect() of a SyntaxError whose .stack was already read shows the real frame", () => {
+    const err = new SyntaxError("inspected");
+    // Once .stack is materialized the printer works from the string instead of the frames.
+    void err.stack;
+    const printed = Bun.inspect(err);
+    expect(printed).toContain("SyntaxError: inspected");
+    expect(printed).not.toContain("<parse>");
+    expect(printed).toMatch(/stack\.test\.ts:\d+:\d+/);
+  });
+
+  test("an uncaught SyntaxError whose .stack was already read still reports where it was created", async () => {
+    using dir = tempDir("syntax-error-stack", {
+      "index.js": ['const err = new SyntaxError("boom");', "void err.stack;", "throw err;", ""].join("\n"),
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "index.js"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(stdout).toBe("");
+    expect(stderr).toContain("SyntaxError: boom");
+    expect(stderr).not.toContain("<parse>");
+    expect(stderr).toMatch(/index\.js:1:\d+/);
+    expect(exitCode).toBe(1);
+  });
+
+  // A SyntaxError can carry a sourceURL without coming from the parser: structured clone
+  // recreates an error with its original's line/column/sourceURL, and a GC that collects a
+  // function on the error's stack records the first frame's position on the error instead.
+  // When such an error later gets frames from Error.captureStackTrace(), that location must
+  // not be rendered as a <parse> frame. The cases below capture from a different file than
+  // the one recorded, since a frame from the recorded file would hide the <parse> line anyway.
+  const stackLines = (err: Error) => err.stack!.split("\n");
+
+  test("a structured clone of a SyntaxError captured from another file gets no <parse> frame", () => {
+    const original = new SyntaxError("cloned");
+    expect((structuredClone(original) as any).sourceURL).toEndWith("stack.test.ts");
+
+    const clone = structuredClone(original);
+    const captureElsewhere = new vm.Script("err => Error.captureStackTrace(err)", {
+      filename: "capture-site.js",
+    }).runInThisContext();
+    captureElsewhere(clone);
+    expect(stackLines(clone).slice(0, 3)).toEqual([
+      "SyntaxError: cloned",
+      expect.stringMatching(/^    at .*capture-site\.js:1:\d+\)?$/),
+      expect.stringMatching(/^    at .*stack\.test\.ts:\d+:\d+\)?$/),
+    ]);
+  });
+
+  test("a SyntaxError posted by a worker gets no <parse> frame when the parent captures a stack for it", async () => {
+    using dir = tempDir("worker-syntax-error", {
+      "worker.js": 'postMessage(new SyntaxError("made in worker"));\n',
+    });
+    const worker = new Worker(pathToFileURL(join(String(dir), "worker.js")).href);
+    try {
+      const { promise, resolve, reject } = Promise.withResolvers<SyntaxError>();
+      worker.onmessage = event => resolve(event.data);
+      worker.onerror = reject;
+      const err = await promise;
+      Error.captureStackTrace(err);
+      expect(stackLines(err).slice(0, 2)).toEqual([
+        "SyntaxError: made in worker",
+        expect.stringMatching(/^    at .*stack\.test\.ts:\d+:\d+\)?$/),
+      ]);
+    } finally {
+      worker.terminate();
+    }
+  });
+
+  test.each([
+    ["new SyntaxError()", '(() => new SyntaxError("collected"))()', /^SyntaxError: collected$/],
+    // JSC flags a syntax error in eval code as a parse error but records no location for it, so
+    // once GC has recorded one it must not be rendered either. (Unlike new Function(), eval's
+    // parse-error path does not materialize .stack, so it is fine under BUN_JSC_validateExceptionChecks.)
+    ["eval()", '(() => { try { eval("{"); } catch (e) { return e; } })()', /^SyntaxError: /],
+    ["indirect eval", '(() => { try { (0, eval)("{"); } catch (e) { return e; } })()', /^SyntaxError: /],
+  ])("a SyntaxError from %s whose frames GC collected gets no <parse> frame when captured", (_, source, header) => {
+    // Nothing but the error refers to the function that created it, so a full GC collects the
+    // function and flushes the error's frames, recording gc-me.js as the error's sourceURL.
+    // A few rounds in case a stale pointer on the native stack keeps the function alive once.
+    for (let i = 0; i < 4; i++) {
+      const err: SyntaxError = new vm.Script(source, { filename: "gc-me.js" }).runInThisContext();
+      // The VM holds on to the most recently thrown exception, frames included, until the next throw.
+      try {
+        throw new Error("displaces the eval error as the VM's last exception");
+      } catch {}
+      Bun.gc(true);
+      Error.captureStackTrace(err);
+      expect(stackLines(err).slice(0, 2)).toEqual([
+        expect.stringMatching(header),
+        expect.stringMatching(/^    at .*stack\.test\.ts:\d+:\d+\)?$/),
+      ]);
+    }
+  });
+
+  test("a parser SyntaxError keeps its <parse> frame", () => {
+    const err = caught(() => new vm.Script("\n\n/[a-0]/", { filename: "my-script.js" }));
+    expect(err.stack!.split("\n")).toContain("    at <parse> (my-script.js:3)");
+  });
+
+  test("a parser SyntaxError in a source without a URL has no <parse> frame", () => {
+    // There is no file to point at, so an "at <parse> (:1)" line would only be noise.
+    // Not new Function("{"): it materializes .stack from inside JSC's own parse-error path, which
+    // never checks for an exception from the stack hook, so it aborts under
+    // BUN_JSC_validateExceptionChecks (see #30823). node:vm's path checks.
+    const stack = caught(() => vm.compileFunction("{")).stack!;
+    expect(stack).not.toContain("<parse>");
+    expect(stack.split("\n").at(-1)).toMatch(/^    at .*stack\.test\.ts:\d+:\d+\)?$/);
+
+    // The same source with a URL keeps the frame.
+    const named = caught(() => vm.compileFunction("{", [], { filename: "named.js" })).stack!;
+    expect(named.split("\n")).toContain("    at <parse> (named.js:1)");
+  });
+
+  test("a parser SyntaxError from importing a module keeps its <parse> frame", async () => {
+    // Bun's transpiler accepts these modules; JSC's parser rejects the regex. A top-level
+    // await import() fails while no JS frame is on the stack, so the errors only get frames
+    // (and a .stack) from Error.captureStackTrace.
+    const badModule = ["module.exports = 1;", '"".match(/[a-0]/);', ""].join("\n");
+    using dir = tempDir("parse-error-import", {
+      "bad-a.cjs": badModule,
+      "bad-b.cjs": badModule,
+      "index.mjs": `
+        let a, b;
+        try {
+          await import("./bad-a.cjs");
+        } catch (e) {
+          a = e;
+        }
+        try {
+          await import("./bad-b.cjs");
+        } catch (e) {
+          b = e;
+        }
+        // Math.max is not on the stack, so no frames are captured for a.
+        Error.captureStackTrace(a, Math.max);
+        Error.captureStackTrace(b);
+        console.log(JSON.stringify([a.stack, b.stack]));
+      `,
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "index.mjs"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(stderr).toBe("");
+    const [noFrames, withFrames] = JSON.parse(stdout) as [string, string];
+    expect(noFrames.split("\n")).toEqual([
+      expect.stringMatching(/^SyntaxError: /),
+      expect.stringMatching(/^    at <parse> \(.*bad-a\.cjs:\d+\)$/),
+    ]);
+    expect(withFrames.split("\n").slice(0, 3)).toEqual([
+      expect.stringMatching(/^SyntaxError: /),
+      expect.stringMatching(/^    at <parse> \(.*bad-b\.cjs:\d+\)$/),
+      expect.stringMatching(/^    at .*index\.mjs:\d+:\d+\)?$/),
+    ]);
+    expect(exitCode).toBe(0);
+  });
+});