diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index c084a885ee86..ccc234531841 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/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(); } diff --git a/test/js/bun/test/snapshot-tests/bun-snapshots.test.ts b/test/js/bun/test/snapshot-tests/bun-snapshots.test.ts index e670e343921f..a2ddecc0fefb 100644 --- a/test/js/bun/test/snapshot-tests/bun-snapshots.test.ts +++ b/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"]); + }); }); diff --git a/test/js/bun/util/inspect.test.js b/test/js/bun/util/inspect.test.js index 5c91f3dbe05f..f65f6c5b5768 100644 --- a/test/js/bun/util/inspect.test.js +++ b/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); + }); +});