error printer: clear a throwing error.message getter's exception before the next property read

src/jsc/bindings/ZigException.cpp

@@ -495,16 +495,21 @@ __attribute__((minsize)) static void fromErrorInstance(ZigException& except, JSC
     }
     if (except.type == SYNTAX_ERROR_CODE) {
         except.message = Bun::toStringRef(err->sanitizedMessageString(global));
-
-    } else if (JSC::JSValue message = obj->getIfPropertyExists(global, vm.propertyNames->message)) {
-        RETURN_IF_EXCEPTION(scope, );
-        except.message = Bun::toStringRef(global, message);
-        if (!scope.clearExceptionExceptTermination()) [[unlikely]]
-            return;
     } else {
-        RETURN_IF_EXCEPTION(scope, );
-
-        except.message = Bun::toStringRef(err->sanitizedMessageString(global));
+        // This one intentionally calls getters.
+        JSC::JSValue message = obj->getIfPropertyExists(global, vm.propertyNames->message);
+        if (scope.exception()) [[unlikely]] {
+            if (!scope.clearExceptionExceptTermination())
+                return;
+            message = {};
+        }
+        if (message) {
+            except.message = Bun::toStringRef(global, message);
+            if (!scope.clearExceptionExceptTermination()) [[unlikely]]
+                return;
+        } else {
+            except.message = Bun::toStringRef(err->sanitizedMessageString(global));
+        }
     }
 
     if (!scope.clearExceptionExceptTermination()) [[unlikely]] {

test/js/bun/util/inspect-error.test.js

@@ -1,5 +1,5 @@
 import { describe, expect, jest, test } from "bun:test";
-import { bunEnv, bunExe, tempDir } from "harness";
+import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
 
 test("error.cause", () => {
   const err = new Error("error 1");
@@ -10,7 +10,7 @@ test("error.cause", () => {
       .replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"),
   ).toMatchInlineSnapshot(`
 "1 | import { describe, expect, jest, test } from "bun:test";
-2 | import { bunEnv, bunExe, tempDir } from "harness";
+2 | import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
 3 | 
 4 | test("error.cause", () => {
 5 |   const err = new Error("error 1");
@@ -20,7 +20,7 @@ error: error 2
       at <anonymous> ([dir]/inspect-error.test.js:6:20)
 
 1 | import { describe, expect, jest, test } from "bun:test";
-2 | import { bunEnv, bunExe, tempDir } from "harness";
+2 | import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
 3 | 
 4 | test("error.cause", () => {
 5 |   const err = new Error("error 1");
@@ -237,6 +237,210 @@ test("error.stack throwing an error doesn't lead to a crash", () => {
   }).toThrow();
 });
 
+// Debug builds verify JSC exception checks in the printer under this flag;
+// release builds compile the verification out and ignore it.
+const validateExceptionChecksEnv = { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" };
+
+describe.concurrent("error.message getter that throws", () => {
+  // The printer reads `message` through a real [[Get]]. The getter's exception
+  // must not stay pending while the printer goes on to read other properties.
+  // Each sink runs in its own process: the failure mode is an abort.
+  const code = sink =>
+    [
+      `class E extends Error { get message() { throw new RangeError("from the getter"); } }`,
+      sink,
+      `console.log("after");`,
+    ].join("\n");
+
+  async function run(sink) {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", code(sink)],
+      env: validateExceptionChecksEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return { stdout, stderr: normalizeBunSnapshot(stderr), exitCode };
+  }
+
+  test("console.error", async () => {
+    const { stdout, stderr, exitCode } = await run(`console.error(new E());`);
+    expect(stderr).toMatchInlineSnapshot(`
+      "1 | class E extends Error { get message() { throw new RangeError("from the getter"); } }
+      2 | console.error(new E());
+                        ^
+      Error: 
+            at <cwd>/[eval]:2:15"
+    `);
+    expect(stdout).toBe("after\n");
+    expect(exitCode).toBe(0);
+  });
+
+  test("Bun.inspect", async () => {
+    const { stdout, stderr, exitCode } = await run(`console.error(Bun.inspect(new E()));`);
+    expect(stderr).toMatchInlineSnapshot(`
+      "1 | class E extends Error { get message() { throw new RangeError("from the getter"); } }
+      2 | console.error(Bun.inspect(new E()));
+                                    ^
+      Error: 
+            at <cwd>/[eval]:2:27"
+    `);
+    expect(stdout).toBe("after\n");
+    expect(exitCode).toBe(0);
+  });
+
+  test("uncaught exception", async () => {
+    const { stdout, stderr, exitCode } = await run(`throw new E();`);
+    expect(stderr).toMatchInlineSnapshot(`
+      "1 | class E extends Error { get message() { throw new RangeError("from the getter"); } }
+      2 | throw new E();
+                ^
+      Error: 
+            at <cwd>/[eval]:2:7
+
+      Bun v<bun-version>"
+    `);
+    expect(stdout).toBe("");
+    expect(exitCode).toBe(1);
+  });
+
+  test("unhandled rejection", async () => {
+    const { stdout, stderr, exitCode } = await run(`Promise.reject(new E());`);
+    expect(stderr).toMatchInlineSnapshot(`
+      "1 | class E extends Error { get message() { throw new RangeError("from the getter"); } }
+      2 | Promise.reject(new E());
+                         ^
+      Error: 
+            at <cwd>/[eval]:2:16
+
+      Bun v<bun-version>"
+    `);
+    expect(stdout).toBe("after\n");
+    expect(exitCode).toBe(1);
+  });
+
+  // The getter's exception must not stay pending after the print either: the
+  // event loop does not call a callback while an exception is pending.
+  test("uncaught in a timer callback: the immediate it queued still runs", async () => {
+    const { stdout, stderr, exitCode } = await run(
+      `setTimeout(() => { setImmediate(() => console.log("immediate")); throw new E(); }, 0);`,
+    );
+    expect(stderr).toMatchInlineSnapshot(`
+      "1 | class E extends Error { get message() { throw new RangeError("from the getter"); } }
+      2 | setTimeout(() => { setImmediate(() => console.log("immediate")); throw new E(); }, 0);
+                                                                                     ^
+      Error: 
+          at <anonymous> (file:NN:NN)
+
+      Bun v<bun-version>"
+    `);
+    expect(stdout).toBe("after\nimmediate\n");
+    expect(exitCode).toBe(1);
+  });
+
+  test("thrown by a test that bun test retries: the retry and the next test still run", async () => {
+    using dir = tempDir("inspect-error-message-getter", {
+      "retry.test.js": [
+        `import { expect, test } from "bun:test";`,
+        `class E extends Error { get message() { throw new RangeError("from the getter"); } }`,
+        `let attempt = 0;`,
+        `test("throws on the first attempt", () => {`,
+        `  console.log("body: attempt", ++attempt);`,
+        `  if (attempt === 1) throw new E();`,
+        `}, { retry: 1 });`,
+        `test("fails an assertion", () => {`,
+        `  console.log("body: second test");`,
+        `  expect(1).toBe(2);`,
+        `});`,
+      ].join("\n"),
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "test", "./retry.test.js"],
+      env: validateExceptionChecksEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({
+      bodies: stdout.split("\n").filter(line => line.startsWith("body: ")),
+      results: normalizeBunSnapshot(stderr)
+        .split("\n")
+        .filter(line => line.startsWith("(pass) ") || line.startsWith("(fail) ")),
+      exitCode,
+    }).toEqual({
+      bodies: ["body: attempt 1", "body: attempt 2", "body: second test"],
+      results: ["(pass) throws on the first attempt (attempt 2)", "(fail) fails an assertion"],
+      exitCode: 1,
+    });
+  });
+
+  // A worker formats its uncaught error on the worker thread before it
+  // dispatches the error to the parent. The parent gets a clone of the error
+  // (worker_threads) or an ErrorEvent (Web Worker); nothing is printed.
+  const workerSource = JSON.stringify(code(`throw new E();`).replace(`console.log("after");`, ""));
+
+  test("uncaught in a worker_threads Worker", async () => {
+    const { stdout, stderr, exitCode } = await run(
+      [
+        `const worker = new (require("node:worker_threads").Worker)(${workerSource}, { eval: true });`,
+        `worker.on("error", e => console.log("error event:", e.constructor.name, JSON.stringify(e.message)));`,
+        `worker.on("exit", code => console.log("exit event:", code));`,
+      ].join("\n"),
+    );
+    expect(stderr).toBe("");
+    expect(stdout).toBe(`after\nerror event: Error ""\nexit event: 1\n`);
+    expect(exitCode).toBe(0);
+  });
+
+  test("uncaught in a Web Worker", async () => {
+    const { stdout, stderr, exitCode } = await run(
+      [
+        `const url = URL.createObjectURL(new Blob([${workerSource}], { type: "application/javascript" }));`,
+        `const worker = new Worker(url);`,
+        `worker.onerror = ev => console.log("error event:", ev.constructor.name);`,
+        `worker.addEventListener("close", ev => console.log("close event:", ev.code));`,
+      ].join("\n"),
+    );
+    expect(stderr).toBe("");
+    expect(stdout).toBe(`after\nerror event: ErrorEvent\nclose event: 1\n`);
+    expect(exitCode).toBe(0);
+  });
+});
+
+// The header the printer renders for each kind of `message`. A value that
+// cannot be read as a string prints like an empty message.
+describe.concurrent.each([
+  ["a number", `e.message = 42;`, "error: 42"],
+  ["a Symbol", `e.message = Symbol("sym");`, "Error: "],
+  ["an object with toString", `e.message = { toString() { return "from toString"; } };`, "error: from toString"],
+  ["an object whose toString throws", `e.message = { toString() { throw new RangeError("ts"); } };`, "Error: "],
+  [
+    "found through a Proxy whose has trap throws",
+    `delete e.message; Object.setPrototypeOf(e, new Proxy(Error.prototype, { has() { throw new RangeError("has"); } }));`,
+    "Error: ",
+  ],
+])("error.message that is %s", (_, setup, header) => {
+  test("prints the header and the stack", async () => {
+    await using proc = Bun.spawn({
+      cmd: [
+        bunExe(),
+        "-e",
+        [`const e = new Error("boom");`, setup, `console.error(e);`, `console.log("after");`].join("\n"),
+      ],
+      env: validateExceptionChecksEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(normalizeBunSnapshot(stderr)).toBe(
+      [`1 | const e = new Error("boom");`, `                  ^`, header, `      at <cwd>/[eval]:1:15`].join("\n"),
+    );
+    expect(stdout).toBe("after\n");
+    expect(exitCode).toBe(0);
+  });
+});
+
 describe("source map remapping of the printed stack", () => {
   // The "at ..." lines that mention one of `files`, with the temp dir removed
   // from the paths.