node:events: return boolean from EventEmitterAsyncResource#emit

[robobun]

What does this PR do?

EventEmitterAsyncResource.prototype.emit() always returned undefined instead of the boolean that EventEmitter#emit is documented to return (true if the event had listeners, false otherwise). This breaks the common Node idiom if (!this.emit('error', e)) throw e;, which would treat every event as unhandled even when a listener ran.

Repro

import { EventEmitterAsyncResource } from "node:events";
const r = new EventEmitterAsyncResource({ name: "R" });
r.on("e", () => {});
console.log(r.emit("e"));     // node: true,  bun before: undefined
console.log(r.emit("none"));  // node: false, bun before: undefined

Cause

emit() called this.asyncResource.runInAsyncScope(() => super.emit(...args)) without return. runInAsyncScope already forwards the callback's return value; the return keyword was missing.

Fix

Added return in front of runInAsyncScope.

How did you verify your code works?

Added a test to test/js/node/async_hooks/EventEmitterAsyncResource.test.ts asserting emit() returns true when listeners exist (including error) and false when none do. Fails on released bun, passes with this change.

[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: bd58e850-40a5-4c5a-a9ac-eb1598e333ea

📥 Commits

Reviewing files that changed from the base of the PR and between 3f5d816 and 243992e.

📒 Files selected for processing (2)

• src/js/node/events.ts
• test/js/node/async_hooks/EventEmitterAsyncResource.test.ts

---

Walkthrough

EventEmitterAsyncResource.emit(...) now returns the value from runInAsyncScope, propagating super.emit's boolean result. A test was added verifying the return value matches Node's EventEmitter#emit semantics for listened/unlistened events and the error event.

Changes

Emit Return Value Fix

Layer / File(s)	Summary
Return value propagation and test src/js/node/events.ts, test/js/node/async_hooks/EventEmitterAsyncResource.test.ts	emit() now returns the result of runInAsyncScope(() => super.emit(...args)) instead of discarding it, and a new test verifies the returned boolean matches EventEmitter#emit for events with listeners, without listeners, and for the error event.

Related Issues: None referenced.

Related PRs: None referenced.

Suggested labels: node:events, node:async_hooks

Suggested reviewers: None identified.

🐰 A boolean once lost in the async fray,
Now returns home, propagated its way,
Emit tells the truth, whether heard or ignored,
A tiny fix, quietly restored.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change: restoring the boolean return from EventEmitterAsyncResource#emit.
Description check	✅ Passed	The description follows the required template and includes both the change summary and verification details.
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]

^{Updated 3:20 PM PT - Jul 7th, 2026}

❌ @robobun, your commit 243992e has 1 failures in Build #70000 (All Failures):

• test/js/sql/sql-prepare-false.test.ts - timeout on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 33688

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

bun-33688 --bun

[github-actions]

This PR may be a duplicate of:

1. node:events: make EventEmitterAsyncResource match Node.js #33689 - Also fixes EventEmitterAsyncResource.emit() not returning a boolean, with the same return addition plus additional refactoring of the class
2. async_hooks,events,http,http2,perf_hooks: port Node.js async compatibility tests and fix the gaps they surface — ALS run/disable + withScope/defaultValue, http client ALS across reused agent sockets, http2 ALS context, AsyncResource.bind, EventEmitterAsyncResource, timerify (+22 tests) #31825 - Large async compatibility PR that includes the identical return fix for EventEmitterAsyncResource.emit() among many other changes

🤖 Generated with Claude Code

[robobun]

Overlap noted:

• async_hooks,events,http,http2,perf_hooks: port Node.js async compatibility tests and fix the gaps they surface — ALS run/disable + withScope/defaultValue, http client ALS across reused agent sockets, http2 ALS context, AsyncResource.bind, EventEmitterAsyncResource, timerify (+22 tests) #31825 is a much larger async_hooks/events/http compatibility PR (31 files, +1576/-56) that has been open since early June and includes this same return among many other changes.
• node:events: make EventEmitterAsyncResource match Node.js #33689 is a broader rewrite of EventEmitterAsyncResource (back-reference, prototype accessors, options.name validation) that also includes this fix and was opened a few minutes after this one.

This PR is intentionally the minimal, single-line change for the emit() return value alone, so it can land independently if the larger PRs take longer to review. Happy to close in favor of either if a maintainer prefers to land one of those instead.

[claude]

LGTM — one-keyword fix with a matching test; the inline note is a pre-existing adjacent issue, not something this PR introduces.

Extended reasoning...

Overview

This PR adds a single return keyword to EventEmitterAsyncResource.prototype.emit in src/js/node/events.ts so that it forwards the boolean result of super.emit(...) through runInAsyncScope instead of returning undefined. A new test in test/js/node/async_hooks/EventEmitterAsyncResource.test.ts asserts true with a listener, false without, and true for a handled error event. I confirmed AsyncResource#runInAsyncScope in src/js/node/async_hooks.ts already returns fn.$apply(thisArg, args), so the forwarded value is exactly EventEmitter#emit's boolean.

Security risks

None. This is a pure return-value propagation change in a Node compat shim; no input parsing, auth, crypto, filesystem, or network paths are touched.

Level of scrutiny

Low. The diff is a single token in a well-understood method, aligning behavior with both Node's documented contract and Bun's own base EventEmitter#emit. There are no design decisions or cross-cutting changes here.

Other factors

The bug-hunter surfaced a pre-existing issue: when captureRejections: true (or the global default) is set, the base EventEmitter constructor assigns this.emit = emitWithRejectionCapture as an own property, shadowing this prototype method entirely. That's real but orthogonal — it predates this PR, is unaffected by it, and emitWithRejectionCapture already returns a boolean, so this PR's contract fix is still correct for the paths that reach it. Fixing the shadowing would be a larger structural change to how kCapture is dispatched and belongs in its own PR. No prior human reviews or outstanding comments to address.

[claude]

LGTM — one-keyword fix that makes EventEmitterAsyncResource#emit return the boolean from super.emit, matching Node.

Extended reasoning...

Overview

Single-keyword change in src/js/node/events.ts: adds return before this.asyncResource.runInAsyncScope(() => super.emit(...args)) in EventEmitterAsyncResource.prototype.emit. runInAsyncScope already forwards the callback's return value, so this correctly propagates the boolean from EventEmitter#emit (true if listeners existed, false otherwise) instead of returning undefined. A new test in test/js/node/async_hooks/EventEmitterAsyncResource.test.ts covers the with-listener, without-listener, and error-with-listener cases.

Security risks

None. Pure return-value plumbing in a Node compat class; no auth, crypto, I/O, or untrusted-input handling touched.

Level of scrutiny

Low. This is a mechanical, one-token fix to match documented Node.js behavior, with a targeted regression test. The pre-existing captureRejections shadowing issue I noted earlier is orthogonal, acknowledged by the author as out of scope, and that thread is resolved — it doesn't affect the correctness of this change (in that configuration emitWithRejectionCapture already returns a boolean anyway).

Other factors

Two overlapping PRs (#33689, #31825) contain the same fix as part of larger changes; the author has already surfaced this in the thread and offered to defer. That's a merge-sequencing decision for a maintainer, not a code-correctness concern — the change here is correct and self-contained either way. No CODEOWNERS entry covers this path.

[robobun]

CI status: the diff itself is green. Both Buildkite runs (#69984, #70000) failed only on the darwin 26 aarch64 - test-bun lane with

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
Refusing to continue with a partial download (would silently fall back to the wrong binary).

which is the CI runner failing to fetch the built binary before any test executes. No test touching node:events or node:async_hooks failed on any platform; the remaining annotations are known Windows/macOS flakes that passed on retry. Ready for a maintainer to merge or rerun the darwin lane.

[robobun]

This landed on main through #31825, which reworked EventEmitterAsyncResource in src/js/node/events.ts; its emit() now returns the result of runInAsyncScope, i.e. the boolean from EventEmitter#emit, and test/js/node/async_hooks/EventEmitterAsyncResource.test.ts on main already asserts the true / false return values.

Checked by running this PR's version of that test file against a debug build of current main (05dd45e): 2 runs, 3 pass / 0 fail each time, and the emit() returns a boolean case this PR added passes on its own as well.

Nothing left for this PR to add, so closing it.