Repository navigation
Report a ShadowRealm's unhandled rejections and uncaught errors through the realm that created it - #42097
Report a ShadowRealm's unhandled rejections and uncaught errors through the realm that created it#42097robobun wants to merge 1 commit into
Conversation
…gh the realm that created it
A ShadowRealm global is a full Zig::GlobalObject. JSC reports a rejected
promise, and an exception that escapes a microtask, to the global of the
realm they happened in. The realm global put rejected promises on its own
list, which the event loop never drains, so they were dropped. Uncaught
errors looked up the realm global's own process object, which code outside
the realm never sees, so they skipped the program's
process.on('uncaughtException') and were fatal. A rejected promise returned
by an EventTarget listener inside a realm went to the realm global's own
nextTick queue, which is never drained either.
The realm global now records its host global, the non-ShadowRealm global it
derives from, and reports through it: promiseRejectionTracker uses the
host's list, the process event emitters use the host's process object, and
the EventTarget rejection path queues on the host's nextTick queue.
Fixes #11845
|
Updated 6:36 PM PT - Sep 8th, 2026
✅ @robobun, your commit 7e3f4d977ef1c0ce9e1e616c2c3df401b815ee7d passed in 🧪 To try this PR locally: bunx bun-pr 42097That installs a local version of the PR into your bun-42097 --bun |
|
Status: ready for review. Reproduced on 1.4.3-canary (and per the report on 1.4.2 and main d745f03): const r = new ShadowRealm();
process.on("unhandledRejection", e => console.log("outer handler saw:", e.message)); // never called before this change
r.evaluate(`Promise.reject(new Error("realm-unhandled")); 1`);
await new Promise(res => setTimeout(res, 50));
console.log("still running; exitCode=" + process.exitCode); // printed, exit 0, nothing elseSame shape for
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. WalkthroughChangesShadowRealm host event routing
Suggested reviewers: Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to ShadowRealm-originated rejection and exception handling is routed to the creating global, with no outstanding concrete merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it adds a new GC-visited field to ZigGlobalObject and reroutes cross-realm error/rejection reporting through JSC bindings — with a stated merge-order interaction with #39992 on the same line in Bun__handleUncaughtException — a human look would still be worthwhile.
What was reviewed:
m_shadowRealmHostis declared viaFOR_EACH_GLOBALOBJECT_GC_MEMBER, whichvisitChildrenImplwalks via the offset table — the WriteBarrier is visited.- Each
uncheckedDowncast<Zig::GlobalObject>redirected throughhostGlobal()is still guarded by the precedinginherits(Zig::GlobalObject::info())check;promiseRejectionTracker'sstatic_castis on the Zig method-table path. deriveShadowRealmGlobalObjectsets the barrier with(vm, shadow, host)and collapses throughdefaultGlobalObject(...)->hostGlobal(), so nested realms and node:vm contexts land on the outermost drained global; no host object leaks into the realm.- Tests follow harness conventions:
bunExe()/bunEnv,-esubprocesses underdescribe.concurrent,await using, concurrent pipe drain, stderr/stdout asserted before exit code, no sleep-based waits.
Extended reasoning...
Overview
The PR routes ShadowRealm-originated unhandled rejections and uncaught exceptions to the creating (host) realm's process object. It adds a WriteBarrier<GlobalObject> m_shadowRealmHost field to Zig::GlobalObject via the FOR_EACH_GLOBALOBJECT_GC_MEMBER V-macro, an inline hostGlobal() accessor that returns the outermost non-ShadowRealm global (or this), and wires deriveShadowRealmGlobalObject to set it via defaultGlobalObject(globalObject)->hostGlobal(). Four entry points — Bun__handleUncaughtException, Bun__handleUnhandledRejection, Bun__emitHandledPromiseEvent, and GlobalObject::promiseRejectionTracker — plus jsFunctionEmitUncaughtExceptionNextTick in JSEventListener.cpp are redirected through hostGlobal(). Seven subprocess tests are added to the existing test/js/bun/jsc/shadow.test.js.
Security risks
None identified. This is error-reporting plumbing; no parsing of untrusted input, no auth/crypto/permissions. The redirection does not expose host-realm objects into the ShadowRealm (the realm's own process is untouched; only the outer realm's list/emitters receive the events), so the ShadowRealm callable-boundary invariant is preserved.
Level of scrutiny
Medium-high. The change is small and focused, but it lives in JSC bindings territory: GC-visited state on ZigGlobalObject, cross-realm error routing, and subtle lifetime concerns. REVIEW.md explicitly names "no new fields on ZigGlobalObject" as a god-file rule; the field here is per-global state whose lifetime matches the global (the accepted exception), and it is threaded through the existing visit-macro so it is GC-safe, but a maintainer should confirm this placement is acceptable rather than, say, keying off the existing ShadowRealmObject linkage. The PR description also flags a merge-order interaction with #39992 on the same statement in Bun__handleUncaughtException, which a human should be aware of when landing.
Other factors
I verified visitChildrenImpl (ZigGlobalObject.cpp:3183-3193) builds an offset table from FOR_EACH_GLOBALOBJECT_GC_MEMBER and visits every entry, so m_shadowRealmHost is visited without a hand-written visitor.append. The WriteBarrier::set(vm, shadow, host) call passes the correct owner. Each redirected uncheckedDowncast remains behind its inherits() guard. The new field creates a realm→host edge only; the host is the thread's main global (already a GC root) or an isolate file global that itself owns the realm, so no new retention cycle. Tests are well-structured per test/CLAUDE.md conventions (existing file, describe.concurrent, bunExe/bunEnv, await using, concurrent pipe drain, output asserted before exit code, event-driven waits via setImmediate/process.on('exit') rather than sleeps). No CODEOWNERS entries cover the changed files. The bug-hunt ran to a dry_streak with no findings.
Problem
ShadowRealmis silently dropped: no output, exit code 0. An exception from a microtask or anEventTargetlistener inside a realm skipsprocess.on("uncaughtException")and is fatal. Node reports all of them throughprocess.deriveShadowRealmGlobalObject(ZigGlobalObject.cpp:919) makes the realm global a fullZig::GlobalObject. JSC reports a rejected promise to its own global, so it landed on the realm global's rejection list, which the event loop never drains.Bun__handleUncaughtExceptionused the realm global's ownprocess, which code outside the realm never sees.Fix
m_shadowRealmHost, the outermost one when realms nest).hostGlobal()returns it, orthisfor other globals.promiseRejectionTrackeruses the host's list, the process-event emitters use the host'sprocess, and the rejected-listener path (JSEventListener.cpp:144) queues on the host's nextTick queue.processis the one the program observes. No host object becomes reachable inside the realm. Other globals are unchanged.test/js/bun/jsc/shadow.test.js(7 new cases, 6 fail on 1.4.3-canary) and the suites in Notes. Self-reviewed, see Notes.Background
ShadowRealm(TC39 stage 3) is a fresh global with its own intrinsics on the sameJSC::VM. Bun builds it as a fullZig::GlobalObject, so the realm has its ownprocess,queueMicrotaskandEventTarget.promiseRejectionTracker. Bun's event loop drains that list onvm.globalonly.Notes
node:vmcontexts had the same shape of bug and already forward their tracker to the thread's default global (NodeVM.cpp:946). A realm created inside anode:vmcontext maps throughdefaultGlobalObject()to that same target.promiseRejectionTrackerthrough a second method table. This change also covers theuncaughtExceptionlookup, theEventTargetrejection path and nested realms, with one field instead of a copied table. I closed Report unhandled promise rejections from ShadowRealm globals #35646 in favour of this one.processlookup fornode:vmcontexts, which left ShadowRealm out of scope): the two touch the same statement inBun__handleUncaughtException. Whichever lands second writesdefaultGlobalObject(lexicalGlobalObject)->hostGlobal()there.process.nextTickcalled by realm code (Node.js http/https response callback not called in ShadowRealm #25608, handled by ShadowRealm: route process.nextTick to the event loop's queue #37931, which can replace its!= defaultGlobalObject()check withhostGlobal()), and the workerself.onerrordispatch for a realm error when noprocesshandler claims it (the Workererrorevent on the parent side does fire, verified below).test-shadow-realm*,test-eventtarget*,test-events-*andtest-promise*-unhandled*parallel tests,test/js/node/vm/vm.test.ts,test/regression/issue/29519.test.ts(realm creation undercollectContinuously), the worker and worker_threads error tests, and the ShadowRealm (and probably node:vm) Errors not logged to the console with async functions #11845 repro (importValueplus an async function, now prints theReferenceErrorand exits 1).vm.createContext()context (reported to the main process), a realm inside aworker_threadsWorker with and without a worker-side handler (handler runs, else the Workererrorevent and worker exit code 1),worker.terminate()with 200 pending realm rejections and a throwing realm microtask underMalloc=1with and withoutBUN_JSC_collectContinuously=1(clean), 20 dropped realms with 50 pending rejections each plusBun.gc(true)undercollectContinuously(all 1000 reported, no crash), andbun test(the rejection fails the running test, the next test is clean).uncaughtExceptionhandler before this change: Bun's timers run and report onvm.global. Only the JSC-attributed paths (promise rejection, microtask exception,FinalizationRegistrycallbacks, listener functions) went to the realm global.WriteBarriermakes a realm global keep its host alive. The host is the thread's main global (already protected) or abun test --isolatefile global that is itself the only thing able to reach the realm, so this adds no retention in practice.Fixes #11845
[human-review] gate passed · iteration 0 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file
root cause · written by the author bot
A ShadowRealm's global object in Bun is a full
Zig::GlobalObject, so JSC reported realm-originated promise rejections into that realm global's own rejection list and dispatched realm uncaught errors to that global's ownprocessobject, neither of which is ever drained or observed by user listeners; the result was that errors thrown or rejected inside a realm vanished silently with a zero exit code instead of reaching the outer process as they do in Node. The fix gives each realm global a reference to the global that created it (m_shadowRealmHost, exposed viahostGlobal()) and routes…