Stop the ordered property walk when the inspect callback throws

src/jsc/bindings/bindings.cpp

@@ -5799,8 +5799,9 @@ extern "C" [[ZIG_EXPORT(nothrow)]] bool JSC__isBigIntInInt64Range(JSC::EncodedJS
         ZigString key = toZigString(name);
 
         JSC::EnsureStillAliveScope ensureStillAliveScope(propertyValue);
-        // TODO: properly propagate exception upwards
         iter(globalObject, arg2, &key, JSC::JSValue::encode(propertyValue), property.isSymbol(), property.isPrivateName());
+        // Propagate exceptions from callbacks.
+        RETURN_IF_EXCEPTION(scope, void());
     }
     properties.releaseData();
 }

test/js/bun/test/snapshot-tests/bun-snapshots.test.ts

@@ -61,4 +61,47 @@ describe("toMatchSnapshot errors", () => {
       expect({ a: 4 }).toMatchSnapshot({ a: expect.any("not a constructor") });
     }).toThrow();
   });
+
+  it("should throw if formatting a nested value throws, instead of leaving that property out", () => {
+    // The snapshot formatter reads `$$typeof` off every object to detect React elements.
+    const reads: string[] = [];
+    const formattable = (name: string) => ({
+      get $$typeof(): unknown {
+        reads.push(name);
+        return undefined;
+      },
+    });
+    const value = {
+      x: {
+        a: {
+          get $$typeof(): unknown {
+            reads.push("a");
+            throw new Error("boom");
+          },
+        },
+        b: formattable("b"),
+      },
+      y: formattable("y"),
+    };
+
+    // The matcher reports formatting failures with its own message rather than
+    // rethrowing the getter's error.
+    expect(() => {
+      // This is what used to get recorded: `a` dropped, both walks carried on.
+      expect(value).toMatchInlineSnapshot(`
+        {
+          "x": {
+            "b": {
+              "$$typeof": [native code],
+            },
+          },
+          "y": {
+            "$$typeof": [native code],
+          },
+        }
+      `);
+    }).toThrow("Failed to pretty format value");
+    // Neither the rest of `x` nor the rest of `value` was formatted.
+    expect(reads).toEqual(["a"]);
+  });
 });

test/js/bun/util/inspect.test.js

@@ -928,3 +928,78 @@ describe.skipIf(!isASAN)("object mutated while being formatted", () => {
     expect(exitCode).toBe(0);
   });
 });
+
+describe("a nested value throws while formatting with { sorted: true }", () => {
+  const custom = Symbol.for("nodejs.util.inspect.custom");
+  const throwing = (visited, name) => ({
+    [custom]() {
+      visited.push(name);
+      throw new Error("boom");
+    },
+  });
+  const formattable = (visited, name) => ({
+    [custom]() {
+      visited.push(name);
+      return name;
+    },
+  });
+
+  it("throws and stops the walk at the throwing property, like the unsorted walk", () => {
+    const visited = [];
+    // `a` is neither first in insertion order nor last in sort order.
+    const obj = { c: formattable(visited, "c"), a: throwing(visited, "a"), b: formattable(visited, "b") };
+
+    expect(() => Bun.inspect(obj, { sorted: true })).toThrow("boom");
+    expect(visited).toEqual(["a"]);
+
+    visited.length = 0;
+    expect(() => Bun.inspect(obj)).toThrow("boom");
+    expect(visited).toEqual(["c", "a"]);
+  });
+
+  it("also stops the walks of the enclosing objects", () => {
+    const visited = [];
+    const obj = {
+      x: { a: throwing(visited, "a"), b: formattable(visited, "b") },
+      y: formattable(visited, "y"),
+    };
+
+    expect(() => Bun.inspect(obj, { sorted: true })).toThrow("boom");
+    expect(visited).toEqual(["a"]);
+  });
+
+  it("Bun.inspect.table", () => {
+    const visited = [];
+    const rows = [{ cell: { a: throwing(visited, "a"), b: formattable(visited, "b") } }];
+
+    expect(() => Bun.inspect.table(rows, { sorted: true })).toThrow("boom");
+    expect(visited).toEqual(["a"]);
+  });
+
+  it("does not read the following properties while the exception is pending", async () => {
+    // Most of the Bun object's properties are initialized lazily by a native
+    // callback the first time they are read. "A0" sorts right before one of them
+    // (Bun.Archive), so reading on with the exception from "A0" still pending
+    // trips the native callback's exception check in debug/ASAN builds.
+    const fixture = `
+      Bun.A0 = { [Symbol.for("nodejs.util.inspect.custom")]() { throw new Error("boom"); } };
+      try {
+        Bun.inspect(Bun, { sorted: true });
+        console.log("returned");
+      } catch (e) {
+        console.log("caught:", e.message);
+      }
+    `;
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", fixture],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(stdout).toBe("caught: boom\n");
+    expect(stderr).toBe("");
+    expect(exitCode).toBe(0);
+  });
+});