diff --git a/src/jsc/bindings/BunObject.cpp b/src/jsc/bindings/BunObject.cpp index 1590027c07cd..a29142d3baae 100644 --- a/src/jsc/bindings/BunObject.cpp +++ b/src/jsc/bindings/BunObject.cpp @@ -320,9 +320,6 @@ static JSValue defaultBunSQLObject(VM& vm, JSObject* bunObject) auto scope = DECLARE_THROW_SCOPE(vm); auto* globalObject = defaultGlobalObject(bunObject->globalObject()); JSValue sqlValue = globalObject->internalModuleRegistry()->requireId(globalObject, vm, InternalModuleRegistry::BunSql); -#if BUN_DEBUG - if (scope.exception()) globalObject->reportUncaughtExceptionAtEventLoop(globalObject, scope.exception()); -#endif RETURN_IF_EXCEPTION(scope, {}); RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, vm.propertyNames->defaultKeyword)); } @@ -332,9 +329,6 @@ static JSValue constructBunSQLObject(VM& vm, JSObject* bunObject) auto scope = DECLARE_THROW_SCOPE(vm); auto* globalObject = defaultGlobalObject(bunObject->globalObject()); JSValue sqlValue = globalObject->internalModuleRegistry()->requireId(globalObject, vm, InternalModuleRegistry::BunSql); -#if BUN_DEBUG - if (scope.exception()) globalObject->reportUncaughtExceptionAtEventLoop(globalObject, scope.exception()); -#endif RETURN_IF_EXCEPTION(scope, {}); auto clientData = WebCore::clientData(vm); RELEASE_AND_RETURN(scope, sqlValue.getObject()->get(globalObject, clientData->builtinNames().SQLPublicName())); diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 983a348f73a5..d288141faab6 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -5643,10 +5643,11 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC: } JSC::PropertySlot slot(object, PropertySlot::InternalMethodType::Get); - if (!object->getPropertySlot(globalObject, property, slot)) - continue; - // Ignore exceptions from "Get" proxy traps. + bool hasProperty = object->getPropertySlot(globalObject, property, slot); + // Ignore exceptions from "Get" proxy traps and lazy initializers; they also report the property as not found. CLEAR_IF_EXCEPTION(scope); + if (!hasProperty) + continue; if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) { if (property == propertyNames->underscoreProto @@ -5718,7 +5719,12 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC: break; if (iterating == globalObject) break; - iterating = iterating->getPrototype(globalObject).getObject(); + JSValue prototype = iterating->getPrototype(globalObject); + // Ignore exceptions from Proxy "getPrototypeOf" trap. + CLEAR_IF_EXCEPTION(scope); + if (!prototype) + break; + iterating = prototype.getObject(); } } diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index 6b90a8f89d58..d9102bd633f4 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -2077,7 +2077,10 @@ extern "C" napi_status napi_get_all_property_names( if (key_mode == napi_key_include_prototypes) { // Climb up the prototype chain to find inherited properties while (!owner->getOwnPropertyDescriptor(globalObject, propKey, desc)) { - JSObject* proto = owner->getPrototype(globalObject).getObject(); + NAPI_RETURN_IF_EXCEPTION(env); + JSValue protoValue = owner->getPrototype(globalObject); + NAPI_RETURN_IF_EXCEPTION(env); + JSObject* proto = protoValue ? protoValue.getObject() : nullptr; if (!proto) { break; } @@ -2085,6 +2088,7 @@ extern "C" napi_status napi_get_all_property_names( } } else { owner->getOwnPropertyDescriptor(globalObject, propKey, desc); + NAPI_RETURN_IF_EXCEPTION(env); } // V8 never applies ONLY_WRITABLE/ONLY_CONFIGURABLE to Proxy keys diff --git a/test/js/bun/util/BunObject.test.ts b/test/js/bun/util/BunObject.test.ts index 63f2ec66c9ae..246c2d9f628e 100644 --- a/test/js/bun/util/BunObject.test.ts +++ b/test/js/bun/util/BunObject.test.ts @@ -1,6 +1,7 @@ import { env } from "bun"; import { hasNonReifiedStatic } from "bun:internal-for-testing"; import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; test("hasNonReifiedStatic", () => { expect(hasNonReifiedStatic(Bun), "do not eagerly initialize the Bun object. This will make Bun much slower.").toBe( true, @@ -33,3 +34,44 @@ test("await import('bun')", async () => { } expect(BunESM.default).toBe(Bun); }); + +test("a lazy property whose builtin fails to load throws from the read", async () => { + // The shell builtin ($) and the sql module body (sql, SQL, postgres) call Symbol(), so + // breaking it makes each builder throw. The read must throw that error (debug builds used to + // report the still-pending exception from inside the sql builders and abort) and the slot + // must stay unreified so a later read runs the builder again. + // + // process.env is read first because the shell builtin reads it before calling Symbol(), and + // building it on Windows reifies another property of the Bun object; doing that in the middle + // of the throwing read trips a separate structure assertion in debug builds. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `process.env; + globalThis.Symbol = NaN; + const results = {}; + for (const name of ["$", "sql", "SQL", "postgres"]) { + results[name] = []; + for (let i = 0; i < 2; i++) { + try { Bun[name]; results[name].push("no throw"); } catch (e) { results[name].push(e.constructor.name); } + } + } + console.log(JSON.stringify(results));`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ + $: ["TypeError", "TypeError"], + sql: ["TypeError", "TypeError"], + SQL: ["TypeError", "TypeError"], + postgres: ["TypeError", "TypeError"], + }), + stderr: "", + exitCode: 0, + }); +}); diff --git a/test/js/bun/util/inspect.test.js b/test/js/bun/util/inspect.test.js index 5c91f3dbe05f..c61020fe0a37 100644 --- a/test/js/bun/util/inspect.test.js +++ b/test/js/bun/util/inspect.test.js @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, isASAN, isWindows, normalizeBunSnapshot, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isASAN, isWindows, normalizeBunSnapshot, tempDir, tmpdirSync } from "harness"; import { join } from "path"; import util from "util"; it("prototype", () => { @@ -579,6 +579,98 @@ it("Bun.inspect huge sparse array summarizes holes without iterating them", asyn }); }); +// A property lookup that throws while an object is being formatted (a Proxy trap in the +// prototype chain, a lazily initialized property whose initializer throws, a module namespace +// export that is still in its temporal dead zone) used to leave the exception pending: the +// lookups of the following properties failed and were dropped from the output or the formatter +// rethrew the exception from the next property, debug builds asserted, and moving on to the +// next prototype dereferenced the empty value returned by the throwing getPrototype. Each case +// runs in a child so a regression fails the test instead of taking down the runner. +describe.concurrent("Bun.inspect when a property lookup throws", () => { + async function runChild(args, cwd) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + env: bunEnv, + cwd, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + const inspectInChild = code => runChild(["-e", code]); + + it("skips a prototype property whose Proxy get trap throws and keeps the rest", async () => { + const result = await inspectInChild(` + const proto = new Proxy({ a: 1, b: 2, c: 3 }, { + get(target, key, receiver) { + if (key === "b") throw new Error("get trap"); + return Reflect.get(target, key, receiver); + }, + }); + const obj = Object.create(proto); + obj.own = 0; + console.log(Bun.inspect(obj)); + `); + expect(result).toEqual({ stdout: "{\n own: 0,\n a: 1,\n c: 3,\n}\n", stderr: "", exitCode: 0 }); + }); + + it("skips a prototype getter that throws behind a Proxy and keeps the rest", async () => { + const result = await inspectInChild(` + const proto = new Proxy({ a: 1, get b() { throw new Error("getter"); }, c: 3 }, {}); + console.log(Bun.inspect(Object.create(proto))); + `); + expect(result).toEqual({ stdout: "{\n a: 1,\n c: 3,\n}\n", stderr: "", exitCode: 0 }); + }); + + it("stops walking the prototype chain at a Proxy whose getPrototypeOf trap throws", async () => { + const result = await inspectInChild(` + const proto = new Proxy({ a: 1 }, { + getPrototypeOf() { + throw new Error("getPrototypeOf trap"); + }, + }); + const obj = Object.create(proto); + obj.own = 0; + console.log(Bun.inspect(obj)); + `); + expect(result).toEqual({ stdout: "{\n own: 0,\n a: 1,\n}\n", stderr: "", exitCode: 0 }); + }); + + it("skips a lazily initialized Bun property whose initializer throws and keeps the rest", async () => { + // Bun.$ is the first property of the Bun object and is built by a builtin that calls + // Symbol(), as are Bun.sql and Bun.SQL further down, so breaking Symbol makes those + // initializers throw while Bun is formatted. Custom inspect functions (Bun.env has one on + // Windows) load node:util the first time one runs, which also needs Symbol, so load it first. + const result = await inspectInChild(` + Bun.inspect({ [Bun.inspect.custom]() { return ""; } }); + globalThis.Symbol = 0; + const out = Bun.inspect(Bun); + console.log(JSON.stringify(["$", "Archive", "version"].map(key => out.includes("\\n " + key + ": ")))); + `); + expect(result).toEqual({ stdout: "[false,true,true]\n", stderr: "", exitCode: 0 }); + }); + + it("skips a module namespace export that is in its temporal dead zone and keeps the rest", async () => { + // b.mjs runs while a.mjs is still evaluating, so reading `later` off the namespace throws a + // ReferenceError. console.log used to rethrow it; util.inspect prints such an export as + // ``, this formatter leaves it out. + using dir = tempDir("inspect-tdz-namespace", { + "a.mjs": ` + import "./b.mjs"; + export const later = 1; + export function hoisted() {} + `, + "b.mjs": ` + import * as a from "./a.mjs"; + console.log(a); + `, + }); + const result = await runChild(["a.mjs"], String(dir)); + expect(result).toEqual({ stdout: "Module {\n hoisted: [Function: hoisted],\n}\n", stderr: "", exitCode: 0 }); + }); +}); + describe("console.logging function displays async and generator names", async () => { const cases = [ function () {}, diff --git a/test/napi/napi-app/js_test_helpers.cpp b/test/napi/napi-app/js_test_helpers.cpp index 65796e7a78e8..2121201a9809 100644 --- a/test/napi/napi-app/js_test_helpers.cpp +++ b/test/napi/napi-app/js_test_helpers.cpp @@ -376,7 +376,8 @@ static napi_value create_latin1_string(const Napi::CallbackInfo &info) { } // get_all_property_names(object, key_mode, key_filter, key_conversion) -// returns { status, keys } +// returns { status, keys, exception }; a pending exception is cleared and +// returned as `exception` so callers can observe `status` alongside it. static napi_value get_all_property_names(const Napi::CallbackInfo &info) { napi_env env = info.Env(); napi_value object = info[0]; @@ -391,6 +392,15 @@ static napi_value get_all_property_names(const Napi::CallbackInfo &info) { static_cast(key_filter), static_cast(key_conversion), &keys); + bool is_pending = false; + NODE_API_CALL(env, napi_is_exception_pending(env, &is_pending)); + napi_value exception; + if (is_pending) { + NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &exception)); + } else { + NODE_API_CALL(env, napi_get_undefined(env, &exception)); + } + napi_value result; NODE_API_CALL(env, napi_create_object(env, &result)); napi_value status_val; @@ -400,6 +410,8 @@ static napi_value get_all_property_names(const Napi::CallbackInfo &info) { NODE_API_CALL(env, napi_get_undefined(env, &keys)); } NODE_API_CALL(env, napi_set_named_property(env, result, "keys", keys)); + NODE_API_CALL(env, + napi_set_named_property(env, result, "exception", exception)); return result; } diff --git a/test/napi/napi-app/module.js b/test/napi/napi-app/module.js index b0aece155716..337e3e8ef506 100644 --- a/test/napi/napi-app/module.js +++ b/test/napi/napi-app/module.js @@ -321,6 +321,75 @@ nativeTests.test_get_all_property_names_proxy_and_string_wrapper = () => { show("frozen writable:", apn(Object.freeze({ a: 1, b: 2 }), napi_key_writable)); }; +nativeTests.test_get_all_property_names_throwing_proxy_traps = () => { + const napi_key_include_prototypes = 0; + const napi_key_own_only = 1; + const napi_key_enumerable = 1 << 1; + const napi_key_keep_numbers = 0; + + const show = (label, { status, keys, exception }) => + console.log(label, `status=${status}`, `keys=${JSON.stringify(keys)}`, `exception=${exception?.message}`); + + // ownKeys succeeds so key collection completes; the per-key descriptor walk + // required by napi_key_enumerable is what invokes the throwing trap. + const throwingDescriptor = new Proxy( + {}, + { + ownKeys: () => ["a"], + getOwnPropertyDescriptor() { + throw new Error("gopd trap"); + }, + }, + ); + show( + "own_only gopd throws:", + nativeTests.get_all_property_names( + throwingDescriptor, + napi_key_own_only, + napi_key_enumerable, + napi_key_keep_numbers, + ), + ); + show( + "include_prototypes gopd throws on prototype:", + nativeTests.get_all_property_names( + Object.create(throwingDescriptor), + napi_key_include_prototypes, + napi_key_enumerable, + napi_key_keep_numbers, + ), + ); + +}; + +nativeTests.test_get_all_property_names_get_prototype_throws_in_descriptor_walk = () => { + const napi_key_include_prototypes = 0; + const napi_key_enumerable = 1 << 1; + const napi_key_keep_numbers = 0; + + // Key collection asks the proxy for its prototype once and must succeed so + // the descriptor walk is reached; the walk asks again (the target does not + // own "a") and that second call throws. + let calls = 0; + const proxy = new Proxy( + {}, + { + ownKeys: () => ["a"], + getPrototypeOf() { + if (calls++ > 0) throw new Error("getPrototypeOf trap"); + return null; + }, + }, + ); + const { status, keys, exception } = nativeTests.get_all_property_names( + Object.create(proxy), + napi_key_include_prototypes, + napi_key_enumerable, + napi_key_keep_numbers, + ); + console.log(`status=${status} keys=${JSON.stringify(keys)} exception=${exception?.message} calls=${calls}`); +}; + nativeTests.test_set_property = () => { const objects = [ {}, diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 9bede65a492e..df6bf6e46d99 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -867,6 +867,22 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { it("handles accessor properties when filtering by napi_key_writable", async () => { await checkSameOutput("test_get_all_property_names_accessor", []); }); + it("returns napi_pending_exception when a Proxy trap throws during the descriptor walk", async () => { + const output = await checkSameOutput("test_get_all_property_names_throwing_proxy_traps", []); + expect(output).toContain("own_only gopd throws: status=10 keys=undefined exception=gopd trap"); + expect(output).toContain( + "include_prototypes gopd throws on prototype: status=10 keys=undefined exception=gopd trap", + ); + }); + it("returns napi_pending_exception when getPrototypeOf throws during the descriptor walk", async () => { + // Not checkSameOutput: V8 filters proxy keys while collecting them and + // only calls getPrototypeOf once, so a trap that throws on the second + // call never throws under Node. + const output = (await runOn(bunExe(), "test_get_all_property_names_get_prototype_throws_in_descriptor_walk", [])) + .replaceAll(/^\[\w+\].+$/gm, "") + .trim(); + expect(output).toBe("status=10 keys=undefined exception=getPrototypeOf trap calls=2"); + }); it("matches Node for Proxy and String wrapper with napi_key_writable/napi_key_configurable", async () => { const output = await checkSameOutput("test_get_all_property_names_proxy_and_string_wrapper", []); expect(output).toContain(`proxy own_only writable: status=0 keys=["x","y"]`);