Match V8 semantics for CallSite.getTypeName/getFunctionName/getMethodName#30941
Match V8 semantics for CallSite.getTypeName/getFunctionName/getMethodName#30941robobun wants to merge 1 commit into
Conversation
…Name getTypeName returned the string "undefined" (typeof thisValue) instead of the receiver's constructor name. getFunctionName returned "" for anonymous frames. getMethodName delegated to getFunctionName and never walked the prototype chain. All three now match V8 / Node.js return values: - getTypeName: null when the receiver is null/undefined/global/strict, otherwise the constructor name (via JSObject::calculatedClassName, which replicates V8's constructor -> proto.constructor -> @@toStringTag -> [[Class]] cascade). - getFunctionName: null for frames whose function has no name. - getMethodName: walks the receiver's own properties and its prototype chain, returning the unique property name whose value is identity-equal to the frame's function, or null. Users on tap + source-map-support saw `undefined.<anonymous>` in TAP error output because source-map-support does `typeName + "."` when formatting method-call frames. The frame class comment remains that Bun can't always recover the original receiver from captured stack frames — in that case both getTypeName and getMethodName still return null, but never the literal string "undefined".
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 2:05 PM PT - May 17th, 2026
✅ @robobun, your commit 03430ef60ba81cf238edb1ba5572310968909e9a passed in 🧪 To try this PR locally: bunx bun-pr 30941That installs a local version of the PR into your bun-30941 --bun |
There was a problem hiding this comment.
I didn't find any bugs, but this is non-trivial JSC bindings C++ (prototype-chain walk, nested exception scopes, user-visible return-value changes for getTypeName/getFunctionName/getMethodName), so it's worth a human pass on the exception-scope discipline and V8-semantics edge cases.
Extended reasoning...
Overview
This PR rewrites three CallSite prototype functions in src/jsc/bindings/CallSitePrototype.cpp to match V8 semantics: getTypeName now returns the receiver's constructor/class name (via JSObject::calculatedClassName) or null instead of typeof this; getFunctionName returns null instead of "" for anonymous frames; and getMethodName is a fresh ~80-line implementation that walks the receiver's prototype chain looking for a property identity-equal to the frame's function. Tests in capture-stack-trace.test.js add three regression cases and update two existing assertions that previously encoded the buggy behavior, plus bump two hard-coded line-number assertions.
Security risks
Low. The new code reads property names/values via VMInquiry slots (skipping accessors), bails on Proxy objects and on objects that override getPrototype, caps traversal at 128 levels, and swallows exceptions from enumeration via DECLARE_TOP_EXCEPTION_SCOPE + tryClearException(). No new attack surface beyond what Error.prepareStackTrace already exposes; no auth/crypto/permissions involved.
Level of scrutiny
Moderate-to-high. This is hand-written JSC bindings C++ on a hot-ish path (every prepareStackTrace consumer). The exception-scope nesting (an outer DECLARE_THROW_SCOPE from ENTER_PROTO_FUNC plus inner DECLARE_TOP_EXCEPTION_SCOPEs inside the loop) is the kind of thing JSC's debug-build verifier is picky about, and calculatedClassName / getOwnPropertyNames can both touch user-observable state. The return-value changes ("" → null, "undefined" → null) are intentional V8-compat fixes but are user-visible and could affect downstream code that did .length on the old string returns.
Other factors
The PR description is thorough, the author ran the directly affected test file plus five related regression tests, and the bug-hunter pass found nothing. No CODEOWNERS cover these paths. Still, ~130 lines of new prototype-walk + exception-handling logic in core bindings is beyond what I'd auto-approve without a human confirming the JSC scope discipline and the acknowledged "still-not-quite-Node" caveat is acceptable.
Problem
Reported in #30938:
async_hooks+process._fatalException+tapproduces TAP subtest errors formatted asundefined.<anonymous>instead of the receiver's class name.The literal string
"undefined"appears becausesource-map-supportdoestypeName + "."when formatting method-call frames, and Bun'sCallSite.getTypeName()was returningtypeof this(so"undefined","object","function") where V8/Node returns the receiver's constructor name — ornullwhen the receiver can't be identified.Two neighbouring methods had the same shape of mismatch:
getFunctionName()returned""for anonymous frames where V8 returnsnull.getMethodName()delegated togetFunctionName(there's even a// TODOon it) so it never actually walked the prototype chain.Fix
src/jsc/bindings/CallSitePrototype.cpp:getTypeName— null/undefined receiver →null; otherwiseJSObject::calculatedClassName, which already implements V8's cascade (ownconstructor→proto.constructor→@@toStringTag→[[Class]]→"Object"). Global object →null.getFunctionName— empty stored name →null.getMethodName— walkthis+ prototype chain, collect property names whose value is identity-equal to the frame's function, return the unique name ornull. Matches V8'sCallSiteInfo::GetMethodName.Caveat
Bun still can't fully reproduce Node's
Test.<anonymous>output becauseJSC::StackFramedoesn't persist the receiver through stack capture — by the timeError.prepareStackTraceruns the callframe is gone.getTypeName()/getMethodName()fall back tonullin that case, so the TAP output goes fromundefined.<anonymous>tonull.<anonymous>(and fromfunction: undefined.<anonymous>tofunction: null.<anonymous>). Still wrong vs. Node, but now consistent with what V8 itself returns for strict-mode frames where the receiver is unavailable, and the null return value unblocks ecosystem libraries that usetypeName && typeName !== 'Object'-style guards. Fixing it the rest of the way needsthisstashed on every captured frame — a separate, bigger change.Verification
test/js/node/v8/capture-stack-trace.test.js:async_hooks + process._fatalException: loss of function name and async context in TAP subtest errors #30938 assertnullreturn for each method."undefined","f3"forgetMethodNameof a top-level function) to the V8-correctnull— these encoded the bug.Fail-before (
USE_SYSTEM_BUN=1):Related test files still green:
Fixes #30938