fix(inspect): don't keep enumerating properties with an exception pending

[robobun]

What does this PR do?

Fixes a fuzzer-found debug assertion in Bun.inspect / console.log (Unexpected exception observed ... releaseAssertNoException) and, while tracking it down, a release-build segfault in the same loop.

JSC__JSValue__forEachPropertyImpl (src/jsc/bindings/bindings.cpp, the slow path used for objects with static tables, Proxies in the prototype chain, etc.) had two problems in its per-property loop:

1. When getPropertySlot() returned false because the lookup threw, it did continue before the CLEAR_IF_EXCEPTION, so the exception stayed pending while the next property was looked up. Lookups can throw through a Proxy get trap or through a lazily initialized static property whose initializer throws (the fuzzer's case: it had clobbered globalThis.Symbol, so the Bun.$ initializer threw, and the next property, Bun.Archive, is a Rust lazy property callback whose host-call exception check aborts when it is entered with an exception pending). The fix is to move the CLEAR_IF_EXCEPTION above the continue, which is what forEachPropertyOrdered already does.

2. At the bottom of the loop, iterating->getPrototype(globalObject).getObject() dereferenced the empty JSValue that ProxyObject::getPrototype returns when it throws (either its own getPrototypeOf trap, or because of the stale exception from 1). In release builds this is Segmentation fault at address 0x5:

   const target = { a: 1 };
   const proto = new Proxy(target, { get(t, k) { if (k in target) throw new Error("x"); return t[k]; } });
   console.log(Object.create(proto)); // segfaults in 1.4.0, prints {} with this PR

   Same with a getPrototypeOf trap that throws. The loop now clears the exception and stops walking the chain when getPrototype() comes back empty.

The BunObject.cpp hunk removes two #if BUN_DEBUG blocks in the Bun.sql / Bun.SQL initializers that called reportUncaughtExceptionAtEventLoop() while the exception was still pending on the VM (every other caller clears first). Once the walk above no longer aborts, Bun.inspect(Bun) with a broken global reaches these and trips Structure::storedPrototype (object->structure() == this) inside the uncaught exception handler, as does plain Bun.sql when the module fails to load; release builds just throw to the caller, which the RETURN_IF_EXCEPTION on the next line already does, so the blocks only made debug builds diverge.

JSC__JSValue__forEachPropertyOrdered ({ sorted: true }) has a related but different issue (it does not propagate exceptions thrown by the callback, see the TODO there); that is tracked separately and not changed here.

How did you verify your code works?

Added Bun.inspect ignores exceptions thrown while enumerating properties to test/js/bun/util/inspect.test.js. It runs the three scenarios in a child process: Proxy prototype with a throwing get trap, Proxy prototype with a throwing getPrototypeOf trap, and Bun.inspect(Bun) with globalThis.Symbol removed. On an unfixed build the first two segfault in release (exitCode: 139, verified with USE_SYSTEM_BUN=1) and fail under the debug/sanitizer build (null member call in JSValue::getObject), and the third aborts with the assertion from the fuzzer report on the unfixed debug build (and with the storedPrototype assertion once only the bindings.cpp change is applied); with this PR the child prints the expected output and exits 0. bun bd test test/js/bun/util/inspect.test.js passes; the fuzzer's script (with the global state it depended on) runs to completion on the debug build.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/util/inspect.test.js

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4214ea92-21b8-4e80-b50e-01ae941dcdb6

📥 Commits

Reviewing files that changed from the base of the PR and between 827475e and 088d9ce.

📒 Files selected for processing (3)

• src/jsc/bindings/BunObject.cpp
• src/jsc/bindings/bindings.cpp
• test/js/bun/util/inspect.test.js

💤 Files with no reviewable changes (1)

• src/jsc/bindings/BunObject.cpp

---

Walkthrough

The changes remove debug-only SQL exception reporting and make property inspection clear exceptions from failing property and prototype lookups. A regression test covers proxy traps, lazy properties, child-process completion, output, and stderr.

Changes

Exception safety

Layer / File(s)	Summary
SQL constructor exception cleanup src/jsc/bindings/BunObject.cpp	The Bun SQL lazy-object constructors no longer report exceptions through BUN_DEBUG. Existing exception propagation remains.
Inspection lookup exception handling src/jsc/bindings/bindings.cpp, test/js/bun/util/inspect.test.js	Property-slot and prototype lookups clear exceptions and skip unavailable values. The regression test covers throwing proxy access, getPrototypeOf, and lazy property initialization.

Possibly related PRs

• oven-sh/bun#37202: Modifies the same exception handling and inspection paths with equivalent regression coverage.
• oven-sh/bun#37307: Changes the same BunObject.cpp and bindings.cpp exception paths and adds similar inspection tests.
• oven-sh/bun#37309: Extends exception clearing in JSC__JSValue__forEachPropertyImpl with broader regression coverage.

Suggested reviewers: jarred-sumner, cirospaciari, dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the inspection exception-handling fix and matches the primary changes.
Description check	✅ Passed	The description includes both required sections and clearly explains the fix, affected code, regression coverage, and verification results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Closing as a duplicate. #37202 already contains the same two bindings.cpp hunks (the CLEAR_IF_EXCEPTION move and the empty check on getPrototype()) and the identical BunObject.cpp cleanup, plus fixes for the crashes further down the same path, and it has tests for the same scenarios. Several fuzzer findings landed on this root cause at once (#37107 and #37213 are the other open ones).

One data point from this investigation for whoever consolidates these: the getPrototype() hunk fixes a release crash, it is not limited to the debug assertion. On the current release build this dies with Segmentation fault at address 0x5:

const proto = new Proxy({}, { getPrototypeOf() { throw new Error("nope"); } });
console.log(Object.create(proto));

A prototype Proxy whose get trap throws for one of its keys crashes the same way today, because the exception left pending by the first bug makes the following getPrototype() come back empty; moving the CLEAR_IF_EXCEPTION takes care of that variant, but the getPrototypeOf trap variant above needs the second hunk. Of the open PRs, only #37202 has it; #37107 and #37213 still dereference the empty value.

[claude]

LGTM. The inline nit about napi.cpp:2072 is pre-existing N-API code and non-blocking — fine to land as-is or fold in as a follow-up.

Extended reasoning...

Overview

This PR fixes two defects in JSC__JSValue__forEachPropertyImpl (bindings.cpp), the slow-path property enumerator behind Bun.inspect / console.log: (1) CLEAR_IF_EXCEPTION now runs before the continue when getPropertySlot returns false, so a throwing Proxy get trap or lazy-property initializer no longer leaves an exception pending into the next iteration; (2) getPrototype() is captured into a local, the exception cleared, and an empty result breaks the walk instead of segfaulting on .getObject(). It also removes two #if BUN_DEBUG blocks in the Bun.sql/Bun.SQL initializers that called reportUncaughtExceptionAtEventLoop with the exception still pending — the immediately-following RETURN_IF_EXCEPTION already handles it, so these only made debug builds diverge (and assert). A subprocess test in inspect.test.js covers all three scenarios.

Security risks

None. This is the inspect/formatting path; no auth, crypto, or trust-boundary parsing is touched. The change adds exception clearing and an empty-value guard where they were missing.

Level of scrutiny

Low-to-medium. The two bindings.cpp hunks are ~10 net lines and mirror patterns already present in sibling code: the fast path at lines 5554–5564 already guards getPrototype() with if (JSValue proto = ...) and clears the exception, and forEachPropertyOrdered (line 5768) already does bool hasProperty = ...; clear; if (!hasProperty) continue;. The BunObject.cpp change is a debug-only deletion with no release-build effect. The changes are strictly defensive — they turn a crash/assert into the same "skip and keep going" behavior the surrounding code already uses for getter exceptions.

Other factors

The test follows harness conventions (subprocess isolation so a regression fails the test rather than killing the runner, Promise.all draining of stdout/stderr/exited, single combined {stdout, stderr, exitCode} assertion with exact expected output). The PR description documents USE_SYSTEM_BUN=1 failing (segfault 139) and the debug build passing. The one inline nit — the same getPrototype(globalObject).getObject() shape at napi.cpp:2072 — is pre-existing, only reachable through a native addon calling napi_get_all_property_names on an adversarial Proxy chain, and does not block this fix.

[claude]

🟡 nit: The same ->getPrototype(globalObject).getObject() pattern remains at src/jsc/bindings/napi.cpp:2072 (in napi_get_all_property_names's filter loop, with no exception check after getOwnPropertyDescriptor) — a Proxy prototype whose trap throws produces the same segfault-at-0x5. It's the sole greppable sibling of this pattern in src/; pre-existing N-API code so non-blocking, but per REVIEW.md's "grep for every sibling site" it may be worth including the one-line fix here or noting it's intentionally out of scope.

Extended reasoning...

What this is

This PR fixes iterating->getPrototype(globalObject).getObject() in JSC__JSValue__forEachPropertyImpl (bindings.cpp:5675-5680) — when ProxyObject::getPrototype throws, it returns an empty JSValue, and calling .getObject() on an empty JSValue dereferences a null cell pointer and segfaults at address 0x5. A grep for this exact pattern across src/ finds one remaining instance at src/jsc/bindings/napi.cpp:2072, inside napi_get_all_property_names:

while (!owner->getOwnPropertyDescriptor(globalObject, propKey, desc)) {
    JSObject* proto = owner->getPrototype(globalObject).getObject();
    if (!proto) break;
    owner = proto;
}

Code path that triggers it

Reachable via napi_get_all_property_names(env, obj, napi_key_include_prototypes, napi_key_enumerable | ..., ...) on an object with a Proxy in its prototype chain. The filter loop at napi.cpp:2061-2080 runs when key_filter includes any of napi_key_enumerable/writable/configurable and key_mode == napi_key_include_prototypes. Inside that loop, getOwnPropertyDescriptor (line 2071) invokes the Proxy's getOwnPropertyDescriptor trap; if the trap throws, there is no exception check before line 2072, so owner->getPrototype(globalObject) on the same Proxy hits RETURN_IF_EXCEPTION inside ProxyObject::performGetPrototype and returns {}. A stateful getPrototypeOf trap that succeeds during getPropertyNames (line 1999) but throws when re-called here produces the same empty return.

Why the existing code doesn't prevent it

The if (!proto) break; on line 2073 checks the result of .getObject(), but .getObject() itself is what crashes: on JSVALUE64 an empty JSValue encodes as 0, which satisfies isCell(), so asCell() returns nullptr and JSCell::getObject() reads m_type at offset ~5 → SIGSEGV at 0x5. This is the identical mechanism the PR description documents for the release-build segfault in forEachPropertyImpl.

Step-by-step proof

1. Native addon calls napi_get_all_property_names(env, obj, napi_key_include_prototypes, napi_key_enumerable, napi_key_numbers_to_strings, &result) where obj's prototype is new Proxy({}, { ownKeys: () => ['x'], getOwnPropertyDescriptor() { throw new Error('boom'); } }).
2. getPropertyNames at line 1999 collects 'x' via the ownKeys trap (uses jsc_key_mode = Include, so it does not call getOwnPropertyDescriptor); NAPI_RETURN_IF_EXCEPTION at 2057 passes.
3. The filter loop reaches propKey = 'x'; owner = object; object->getOwnPropertyDescriptor(...) returns false (not own), so the while body runs and owner becomes the Proxy.
4. Next iteration: owner->getOwnPropertyDescriptor(globalObject, 'x', desc) invokes the Proxy trap, which throws. Return value is false → loop body runs again with the exception still pending.
5. owner->getPrototype(globalObject) on the Proxy enters ProxyObject::performGetPrototype, hits RETURN_IF_EXCEPTION, returns JSValue().
6. JSValue().getObject() → asCell() = nullptr → nullptr->isObject() reads offset 5 → segfault.

Impact and fix

This is pre-existing N-API code the PR does not touch, only reachable through a native addon calling napi_get_all_property_names on an object with an adversarial Proxy in its chain, so it should not block this PR — merging is strictly an improvement over main. Flagging per REVIEW.md's "Fix the whole class in the same PR — grep for every sibling site sharing the pattern... If a site is intentionally excluded, say so in the PR" since this is the sole greppable sibling of the exact pattern being fixed. The fix is the same shape as this PR's hunk: capture getPrototype(globalObject) into a JSValue, check/clear the exception, break if empty, then call .getObject() — plus an exception check after getOwnPropertyDescriptor.