napi_get_prototype: return null for a Proxy without running its trap, like Node

[robobun]

Problem

• napi_get_prototype on a Proxy runs the Proxy's getPrototypeOf trap. Node does not: V8's Object::GetPrototype, which Node's napi_get_prototype wraps, cannot run JS and returns null for any Proxy (verified with Node 26.3, output in the details below).
• Running the trap also makes the failure mode wrong: when the trap throws (or returns a non-object, or the proxy is revoked) the call returns napi_ok, writes a NULL napi_value into *result, and leaves the exception pending without telling the addon. Current canary on the new fixture: trap throws: status=0 pending=true result=null handle exception=the trap's error.
• This is an unreleased regression from jsc: delete javascript_core_c_api.rs and remove all JSC C API usage from Rust #33731 (86caf6e): napi_get_prototype used to call the JSC C API's JSObjectGetPrototype, which is getPrototypeDirect(), and JSC creates Proxy structures with a null prototype, so 1.3.14 already returned null for a Proxy without running anything. jsc: delete javascript_core_c_api.rs and remove all JSC C API usage from Rust #33731 replaced it with JSValue::get_prototype, JSC's full [[GetPrototypeOf]] (traps included), which returned a bare JSValue that is empty (encoded 0, i.e. a NULL handle) when it threw, so the call site in src/runtime/napi/napi_body.rs could not tell success from failure. Affects 1.4.0 canary only; the napi_get_prototype line in the napi: align symbol/buffer/coercion/reference/tsfn semantics with Node.js #36801 audit ("already avoids the trap") describes the pre-jsc: delete javascript_core_c_api.rs and remove all JSC C API usage from Rust #33731 state.
• The other getPrototype exception-handling sites found by the same reading (the inspect property walk, napi_get_all_property_names, util.isError) are separate fixes in Fix crash and dropped properties in Bun.inspect when a property lookup or getPrototypeOf throws during the property walk #29642, napi: propagate Proxy trap exceptions from napi_get_all_property_names descriptor filter #32263 and util.isError: propagate a throwing getPrototypeOf trap instead of crashing #37202; this PR merges cleanly in either order with them.

Fix

• napi_get_prototype returns null for a ProxyObject without touching its handler, the same result Node produces and the same result 1.3.14 produced. Every other object type's [[GetPrototypeOf]] in JSC is a plain read (JSGlobalProxy forwards to the global object, ImportMetaObject returns null, primitives get their wrapper prototype), so the function no longer runs JS for any input, like Node's.
• JSValue::get_prototype (src/jsc/JSValue.rs) now returns JsResult<JSValue> through host_fn::from_js_host_call, the wrapper JSValue::call and unwrap_boxed_primitive already use for JSC calls that return empty exactly when they threw. This is what let the empty value reach *result, and the remaining callers (ConsoleObject.rs, pretty_format.rs, ScopeFunctions.rs, all already returning JsResult) now propagate with ? instead of holding a possibly-empty value. They only ever see ordinary objects (the formatters print a Proxy's target), so their output is unchanged. In napi_get_prototype the Err arm maps to napi_pending_exception as the generic Node-API protocol; with the Proxy case handled above it is not reachable with today's object types.
• Test: test/napi/napi.test.ts (napi_get_prototype) runs fixture test_napi_get_prototype_proxy (napi-app/module.js) through checkSameOutput, so the output is compared byte for byte with Node, and then pins the lines. The helper perform_get_prototype (napi-app/js_test_helpers.cpp) pre-fills *result with a sentinel so the output distinguishes a real null from "not written" and from a NULL handle. Cases: proxy without traps, callable proxy, a trap that counts its calls (must stay 0), a trap that throws, a trap returning a number, a revoked proxy, plus plain / null-prototype objects and an object whose prototype is a proxy (returned as-is, only the object itself is special-cased).
• Fails under USE_SYSTEM_BUN=1 (1.4.0-canary.1: 7 of 11 lines differ from Node, see details), passes with bun bd test test/napi/napi.test.ts.
• Also run on the debug build: the rest of test/napi/napi.test.ts, Node's own test_general suite (test/napi/node-napi-tests/.../test_general/do.test.ts, which asserts napi_get_prototype matches Object.getPrototypeOf for ordinary objects), test/js/bun/util/inspect.test.js, test/js/bun/test/printing/diffexample.test.ts, jest-each.test.ts, describe.test.ts, pretty-format-overflow.test.ts, console-table.test.ts; plus a script formatting classes, functions, null-prototype objects and proxies, and a bun test file exercising describe.each / test.each / skipIf and class-instance toEqual diffs, both clean under BUN_JSC_validateExceptionChecks=1.
• Noted while reviewing the other Proxy-receiver paths, not changed here (different function and file): napi_remove_wrap on a Proxy or on globalThis reports success but leaves the wrap attached, because src/jsc/bindings/napi.cpp removes it with the virtual deleteProperty (which ProxyObject refuses for private names and JSGlobalProxy forwards to its target) while napi_wrap / napi_unwrap use putDirect / getDirect; Node removes it. Probe output is in the second details block.

Background

• [[GetPrototypeOf]] is the spec operation behind Object.getPrototypeOf. For ordinary objects it reads a field and cannot fail; a Proxy implements it by calling its handler's getPrototypeOf trap, so it can run arbitrary JS and throw. V8's Object::GetPrototype returns a plain Local<Value> rather than a MaybeLocal, so it cannot run a trap, and it reports null for a Proxy; that is the behavior Node-API addons are written against.
• getPrototypeDirect() is JSC's raw read of the prototype stored in an object's Structure, bypassing any override; getPrototype() is the full operation that dispatches to ProxyObject's trap. The JSC C API's JSObjectGetPrototype is the former.
• JSC signals a throw from a value-returning operation by returning the empty JSValue (encoding 0) and leaving the exception on the VM. host_fn::from_js_host_call maps that to Err(JsError::Thrown) and, in debug builds, asserts the value is empty if and only if an exception is pending.
• A napi_value is an encoded JSValue, so storing the empty value hands the addon a NULL handle.
• napi_pending_exception is the status a Node-API call returns when JS it ran threw; the exception stays pending for napi_get_and_clear_last_exception and out-params are left unwritten.

Fixture output: Node 26.3 (and this branch, identical) vs current canary

Node v26.3.0, and this branch:

plain object: status=0 pending=false result=Object.prototype exception=none
null prototype: status=0 pending=false result=null exception=none
proxy without traps: status=0 pending=false result=null exception=none
callable proxy: status=0 pending=false result=null exception=none
trap returns Array.prototype: status=0 pending=false result=null exception=none
getPrototypeOf trap calls: 0
trap throws: status=0 pending=false result=null exception=none
trap returns a number: status=0 pending=false result=null exception=none
revoked proxy: status=0 pending=false result=null exception=none
object whose prototype is a proxy: status=0 pending=false result=the proxy exception=none
plain object again: status=0 pending=false result=Object.prototype exception=none

Bun 1.4.0-canary.1 (USE_SYSTEM_BUN=1):

plain object: status=0 pending=false result=Object.prototype exception=none
null prototype: status=0 pending=false result=null exception=none
proxy without traps: status=0 pending=false result=Array.prototype exception=none
callable proxy: status=0 pending=false result=Function.prototype exception=none
trap returns Array.prototype: status=0 pending=false result=Array.prototype exception=none
getPrototypeOf trap calls: 1
trap throws: status=0 pending=true result=null handle exception=the trap's error
trap returns a number: status=0 pending=true result=null handle exception=TypeError
revoked proxy: status=0 pending=true result=null handle exception=TypeError
object whose prototype is a proxy: status=0 pending=false result=the proxy exception=none
plain object again: status=0 pending=false result=Object.prototype exception=none

napi_remove_wrap follow-up: probe output (existing try_wrap / try_remove_wrap / try_unwrap helpers from napi-app, wrapping the number 6, then trying to re-wrap with 7)

Node v26.3.0:

plain: wrap=true remove=6 unwrap_after=undefined rewrap=true
proxy: wrap=true remove=6 unwrap_after=undefined rewrap=true
callable proxy: wrap=true remove=6 unwrap_after=undefined rewrap=true
globalThis: wrap=true remove=6 unwrap_after=undefined rewrap=true

Bun (this branch, unchanged in this respect):

plain: wrap=true remove=6 unwrap_after=undefined rewrap=true
proxy: wrap=true remove=6 unwrap_after=6 rewrap=false
callable proxy: wrap=true remove=6 unwrap_after=6 rewrap=false
globalThis: wrap=true remove=6 unwrap_after=6 rewrap=false

Earlier version of this PR

The first revision kept running the trap and changed only the failure report: napi_pending_exception with *result untouched when the trap threw, tested on Bun alone since Node never reaches that path. Review asked to match Node instead and not run JS, which is the current shape; the JsResult change to JSValue::get_prototype is unchanged from that revision.

---

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

[robobun]

Status: reproduced with the test_napi_get_prototype_proxy fixture on the current 1.4.0 canary (7 of 11 lines differ from Node: proxies return their target's prototype, the trap runs, and a throwing trap yields napi_ok plus a NULL handle with the exception left pending). Unreleased regression from #33731; 1.3.14 is unaffected. This branch matches Node's output byte for byte; the comparison is test/napi/napi.test.ts (napi_get_prototype).

[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: 65651861-c59e-4efb-985f-6959063f7c33

📥 Commits

Reviewing files that changed from the base of the PR and between 3d369b4 and ac7ae28.

📒 Files selected for processing (8)

• src/jsc/ConsoleObject.rs
• src/jsc/JSValue.rs
• src/runtime/napi/napi_body.rs
• src/runtime/test_runner/ScopeFunctions.rs
• src/runtime/test_runner/pretty_format.rs
• test/napi/napi-app/js_test_helpers.cpp
• test/napi/napi-app/module.js
• test/napi/napi.test.ts

Included review availability: Your plan includes up to 5 reviews per rolling hour; 0 remain after this review.

---

Walkthrough

Changes

Prototype exception handling

Layer / File(s)	Summary
Fallible prototype lookup src/jsc/JSValue.rs, src/jsc/ConsoleObject.rs, src/runtime/test_runner/ScopeFunctions.rs, src/runtime/test_runner/pretty_format.rs	JSValue::get_prototype now returns JsResult<JSValue>. Internal callers propagate errors from Proxy traps and invalid prototype lookups.
N-API exception contract src/runtime/napi/napi_body.rs, test/napi/napi-app/js_test_helpers.cpp, test/napi/napi-app/module.js, test/napi/napi.test.ts	napi_get_prototype returns pending-exception status without modifying its output on failure. Tests cover successful lookups, throwing traps, invalid traps, and revoked proxies.

Suggested reviewers: jarred-sumner, dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly describes the primary behavior change: returning null for Proxy objects without invoking their getPrototypeOf trap, matching Node.
Description check	✅ Passed	The description explains the problem, fix, technical background, verification steps, test results, and scope, although it does not use the template headings exactly.

---

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

[claude]

LGTM — small, well-scoped fix that brings napi_get_prototype in line with the Node-API pending-exception protocol.

What was reviewed:

• get_prototype signature change to JsResult<JSValue> via from_js_host_call; confirmed the C++ binding (JSC__JSValue__getPrototype → JSValue::getPrototype) follows the empty-iff-threw contract that wrapper requires.
• Grepped all 6 get_prototype call sites — every one is updated in this diff, and the non-napi callers already return JsResult so ? propagates cleanly.
• napi_get_prototype: *result is left untouched on Err, env.pending_exception() sets last error — matches the pattern neighboring functions use and what the existing test_pending_exception_gate test expects.
• Test fixture pre-fills *result with a sentinel to distinguish "untouched" from "NULL written", covers throwing trap / non-object trap return / revoked proxy plus controls; Bun-only (not checkSameOutput) is justified in-comment since V8 never runs the trap.

Extended reasoning...

Overview

This PR fixes napi_get_prototype to return napi_pending_exception (leaving *result unwritten) when JSC's [[GetPrototypeOf]] throws — which happens when a Proxy's getPrototypeOf trap throws, returns a non-object, or the proxy is revoked. Previously it returned napi_ok with a NULL napi_value written to *result and the exception silently pending.

The change has two parts: (1) JSValue::get_prototype in src/jsc/JSValue.rs now returns JsResult<JSValue> via host_fn::from_js_host_call (the standard wrapper for JSC calls that return empty-iff-threw), and (2) napi_get_prototype in src/runtime/napi/napi_body.rs matches on that result and returns env.pending_exception() on Err. The four other callers (ConsoleObject.rs ×3, pretty_format.rs, ScopeFunctions.rs) mechanically add ? — they already return JsResult and, per the PR description, only ever see ordinary objects (formatters unwrap proxies to their target first), so no observable behavior change there.

Security risks

None. This is exception-propagation plumbing in the N-API compatibility layer; no auth, crypto, permissions, or untrusted-input parsing involved. If anything it's a hardening: addons no longer receive a NULL napi_value handle they might dereference.

Level of scrutiny

Low-to-medium. The core change is a one-line signature change plus a match arm, both following patterns already used pervasively in the same files (unwrap_boxed_primitive uses the identical from_js_host_call wrapper immediately above; env.pending_exception() is used by dozens of neighboring napi functions). I verified via grep that all 6 get_prototype call sites in src/ are updated in this diff, and confirmed the underlying C++ binding JSC__JSValue__getPrototype is a direct pass-through to JSC's JSValue::getPrototype, which follows the empty-iff-threw contract from_js_host_call encodes.

Other factors

• The test is thorough and follows repo conventions closely: it mirrors the existing perform_instanceof helper structure, pre-fills *result with a sentinel string so it can distinguish "untouched" from "NULL handle written" (proving the *result-ordering invariant #36849 established), covers all three throwing paths plus positive controls and a trailing call showing nothing is left pending, and asserts identity on the trap's thrown error object.
• The test is Bun-only (runOn(bunExe(), ...)) rather than checkSameOutput, which is well-justified in the test comment and PR description: V8's Object::GetPrototype never runs proxy traps, so Node never reaches this code path and can't be used as the reference output here.
• PR description states verification under USE_SYSTEM_BUN=1 (fails), bun bd test (passes), and BUN_JSC_validateExceptionChecks=1 on the affected formatter/test-runner paths.
• No prior review comments on this PR; no outstanding concerns to address.

[dylan-conway]

should we instead match node and not run js?

[robobun]

Done in 6ebf519: napi_get_prototype now returns null for a ProxyObject without looking at its handler, which is what V8's Object::GetPrototype gives Node, and nothing in the function runs JS any more. The test is now a checkSameOutput comparison with Node (proxy without traps, callable proxy, a trap that counts its calls and must stay at 0, throwing / non-object / revoked cases, and an object whose prototype is a proxy, which is still returned as-is). The JsResult change to JSValue::get_prototype stays, since that is what let the empty value reach *result in the first place; in napi_get_prototype its Err arm is no longer reachable with today's object types. PR description updated.

[robobun]

^{Updated 2:18 AM PT - Aug 17th, 2026}

✅ @robobun, your commit d515c0145cb6f1efc1b020a4ae6cc801c87c23b6 passed in Build #99939! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 39418

That installs a local version of the PR into your bun-39418 executable, so you can run:

bun-39418 --bun

[claude]

LGTM — focused Node-compat fix with byte-for-byte parity testing against Node.

What was reviewed:

• napi_get_prototype Proxy special-case: js_type() is safe on non-cells (returns Cell, won't match ProxyObject), so primitives still fall through to get_prototype.
• JSValue::get_prototype signature change to JsResult: all 5 Rust callers updated with ? in already-JsResult contexts; from_js_host_call follows the same zero-is-throw pattern as unwrap_boxed_primitive right above it, and JSValue::NULL (valid null-prototype result) is not empty so it returns Ok.
• Test uses checkSameOutput plus pinned lines, covers callable/revoked/throwing-trap proxies and a proxy in the prototype chain; sentinel distinguishes "untouched" vs NULL handle vs real null.

Extended reasoning...

Overview

This PR fixes napi_get_prototype to match Node.js/V8 behavior when called on a Proxy: return null without running the getPrototypeOf trap. Previously Bun ran the trap, and when it threw, wrote a NULL napi_value handle to *result while returning napi_ok with the exception left pending — a broken contract for addons. The fix is two-part: (1) a ProxyObject type check in napi_get_prototype that returns JSValue::NULL early, and (2) changing JSValue::get_prototype to return JsResult<JSValue> via from_js_host_call so no caller can silently receive an empty value. Five Rust callers (three in ConsoleObject.rs, one each in pretty_format.rs and ScopeFunctions.rs) are updated to propagate with ?. Test coverage adds a C++ helper perform_get_prototype (with sentinel pre-fill of *result), a JS fixture covering 11 cases, and a checkSameOutput test that also pins the expected lines.

Security risks

None. This is a Node-API compat fix affecting how prototype lookup handles Proxy objects. If anything it reduces attack surface: user JS (proxy traps) no longer runs during napi_get_prototype.

Level of scrutiny

Medium. It touches a shared JSValue helper signature, but the change is compile-time-enforced (missing a caller would fail to build) and I confirmed via grep that all callers are updated. I checked that js_type() is safe on non-cell values (returns JSType::Cell), that from_js_host_call uses the zero-is-throw contract (so a valid null prototype returns Ok(null)), and that JSC__JSValue__getPrototype in bindings.cpp is a plain forward to JSC's getPrototype (which follows empty-iff-threw). The Err arm in napi_get_prototype maps to pending_exception() — unreachable with the Proxy case handled but correct as a defensive fallback.

Other factors

The maintainer (dylan-conway) requested the current approach ("match node and not run js") and the PR was reworked to do exactly that; the comment-cop feedback about long comments was also addressed and both threads are resolved. The test quality is high: checkSameOutput gives byte-for-byte Node parity, the pinned assertions prevent a shared regression from passing, the trap-call counter proves the trap never runs, and the sentinel/"null handle" distinction proves *result receives a real null. The PR description confirms USE_SYSTEM_BUN=1 fails (7/11 lines differ) and the debug build passes the full napi suite plus formatter/test-runner tests under BUN_JSC_validateExceptionChecks=1.