Bun.inspect: don't leave an exception pending when a property lookup fails

[robobun]

What does this PR do?

JSC__JSValue__forEachPropertyImpl (the property walk behind Bun.inspect / console.log) skipped a property as soon as getPropertySlot returned false. Two lookups can return false with an exception still pending: a lazily built static property (PropertyCallback) whose initializer throws, and a Proxy get trap that throws. The continue ran before the existing CLEAR_IF_EXCEPTION, so the exception stayed set while the next property was looked up.

What that did:

• For the Bun object, the next lazy property (Archive) was built with the stale exception pending. In release builds setUpStaticFunctionSlot then reported it as missing and it vanished from the output; in debug builds the exception scope assertion fired (releaseAssertNoException, the crash this PR started from, found by fuzzing: a script that broke Bun.$'s initializer and then called Bun.inspect(Bun)).
• With a Proxy in the prototype chain, the later iterating->getPrototype(globalObject) bailed out on the pending exception and returned an empty JSValue, and .getObject() on it segfaulted (Segmentation fault at address 0x5 on the current canary).

The same getPrototype line also crashed on its own whenever a Proxy getPrototypeOf trap throws, since the empty return value was never checked.

The fix is the two hunks in bindings.cpp: clear the exception right after getPropertySlot regardless of its result (this is what forEachPropertyOrdered already does), and stop the prototype walk when getPrototype returns empty instead of dereferencing it. Exceptions from these lookups were already being swallowed at the end of the function, so the output for the non-crashing cases is unchanged.

Repros that crash the current release build and pass with this change:

const proto = new Proxy({ a: 1 }, { get(t, k, r) { if (k === "a") throw new Error("x"); return Reflect.get(t, k, r); } });
Bun.inspect(Object.create(proto)); // segfault, now "{}"

const proto2 = new Proxy({ a: 1 }, { getPrototypeOf() { throw new Error("x"); } });
Bun.inspect(Object.create(proto2)); // segfault, now "{\n  a: 1,\n}"

How did you verify your code works?

Added three tests to test/js/bun/util/inspect.test.js. On the unfixed build the two Proxy tests segfault (release) or trip UBSan (debug), and the lazy property test fails with Archive missing from the output (release) or the exception scope assertion in the child process (debug). All three pass with bun bd test test/js/bun/util/inspect.test.js, along with the rest of that file and the console tests.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 291438de-2f53-4e18-be98-c22e4689ef2b

📥 Commits

Reviewing files that changed from the base of the PR and between 2f5c180 and ad64014.

📒 Files selected for processing (2)

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

---

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

[robobun]

@coderabbitai review

[coderabbitai]

Action performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Caution

CodeRabbit couldn't update its existing comment.

Error details

putComment timed out

[github-actions]

This PR may be a duplicate of:

1. Fix crash when a property lookup throws while an object is being formatted #37428 - Effectively identical: same reorder of CLEAR_IF_EXCEPTION(scope) to run unconditionally after getPropertySlot, plus the same empty-JSValue guard on getPrototype in the prototype-chain walk.
2. fix(inspect): handle Proxy trap exceptions when walking prototype chain #30099 - Same two fixes in JSC__JSValue__forEachPropertyImpl and the same file set (bindings.cpp + inspect.test.js), fixing throwing Proxy get/getPrototypeOf traps during inspect.
3. Fix Bun.inspect null deref with Proxy prototypes; stop reporting from inside the sql lazy-property builders #30245 - Contains both of this PR's hunks verbatim at the same offsets, with an additional sql lazy-property-builder fix layered on top.
4. fix(inspect): don't crash when a Proxy in the prototype chain throws #29642 - Same clear-before-continue fix and same empty-JSValue getPrototype guard, wrapped in a loop restructure plus an extra napi fix.
5. Fix stale pending exceptions in console.log property iteration #37202 - Same two fixes (unconditional exception clear after getPropertySlot; isEmpty() check before getObject()), bundled with broader ZigGlobalObject/napi changes.

🤖 Generated with Claude Code

[claude]

LGTM — focused crash fix that reorders an existing CLEAR_IF_EXCEPTION and adds an empty-value guard.

What was reviewed:

• Confirmed the getPropertySlot reordering matches what forEachPropertyOrdered already does (bindings.cpp:5788-5792), and the getPrototype empty-check mirrors the fast-path at line 5574.
• Checked that swallowing these exceptions is consistent with the function's existing tail-end tryClearException, so non-crashing output is unchanged.
• Tests follow harness conventions (subprocess for the globalThis.process clobber, combined stdout/stderr/exitCode assertion, exact-value expects).

Extended reasoning...

Overview

Two small hunks in JSC__JSValue__forEachPropertyImpl (src/jsc/bindings/bindings.cpp), the property walk behind Bun.inspect/console.log:

1. Move the existing CLEAR_IF_EXCEPTION(scope) to run before the continue when getPropertySlot returns false, so a Proxy get trap or lazy-property initializer that throws doesn't leave an exception pending into the next iteration.
2. Check iterating->getPrototype(globalObject) for an empty JSValue (and clear the exception) before calling .getObject() on it, so a throwing getPrototypeOf trap stops the walk instead of segfaulting.

Plus three tests in test/js/bun/util/inspect.test.js covering the Proxy-get-throws, Proxy-getPrototypeOf-throws, and lazy-Bun-property-throws cases.

Security risks

None. This is formatter/inspect code; the change is purely defensive (clear a pending exception, null-check an empty JSValue). No new user-controlled inputs reach a syscall or allocation.

Level of scrutiny

Low-to-medium. It's C++ in the JSC bindings, so exception-scope discipline matters, but the change is a two-line reorder plus a null guard — no new control flow, no allocation, no ownership transfer. Both hunks bring the slow path in line with what the sibling code already does: forEachPropertyOrdered clears the exception before checking hasProperty (bindings.cpp:5788-5792), and the fast-path prototype walk already guards if (JSValue proto = ...) with a trailing CLEAR_IF_EXCEPTION (bindings.cpp:5574-5584). The function already swallows exceptions at its tail (bindings.cpp:5706-5709), so the "swallow" semantics for these lookups are unchanged — only the "leave pending across the next JSC call" bug is removed.

Other factors

• The PR description explains the mechanism precisely (stale exception → setUpStaticFunctionSlot reports missing / getPrototype returns empty → .getObject() on tag-only JSValue segfaults), and the tests are the actual repros: two crash on the current release build, one drops Archive from the output / trips the debug assertion.
• Tests follow REVIEW.md conventions: added to the existing inspect.test.js, subprocess-isolated where they mutate process globals, pipes drained concurrently with Promise.all, combined {stdout, stderr, exitCode} assertion, exact toBe on inspect output.
• The lazy-property test's reliance on Bun.$ reading process and Archive being adjacent is documented in the comment above it; if the Bun object's property set changes later the test may need a tweak, but that's an acceptable coupling for a regression test of this specific assertion.
• No outstanding reviewer comments; only a CodeRabbit rate-limit notice on the timeline.

[robobun]

^{Updated 10:42 AM PT - Aug 14th, 2026}

⏳ @robobun, your commit ad64014 is still building in Build #96315, but has 2 failures so far (All Failures):

• test/js/bun/s3/s3.test.ts - code 1 on 🐧 25.04 x64
• test/js/bun/s3/s3.test.ts - code 1 on 🐧 3.23 aarch64
• test/js/bun/util/inspect.test.js - code 1 on 🪟 2019 x64
• test/js/bun/util/inspect.test.js - code 1 on 🪟 11 aarch64

[robobun]

Closing as a duplicate. #37428 already has the same two bindings.cpp hunks (and a lazy property test that triggers Bun.$ through Symbol instead of clobbering process), and #37202, #30245, #30099 and #29642 carry the same fix as well.

For the record, the Windows failure on this PR's CI run (build 96315) was not caused by the fix. Setting globalThis.process = undefined in the test's child process means node:util can no longer be loaded, and on Windows Bun.inspect(Bun) reaches that through Bun.env's inspect.custom hook. The utilInspectFunction lazy initializer then returns without setting a value and JSC aborts in LazyProperty::callFunc. The same abort reproduces on every platform with:

globalThis.process = undefined;
Bun.inspect({ [Bun.inspect.custom]() { return 1; } });

That initializer is already being fixed in #37202, so nothing further is needed from this PR.