readline/promises: reject instead of throwing synchronously from Interface#question

[robobun]

Repro

import { createInterface } from "node:readline/promises";
import { PassThrough } from "node:stream";

const rl = createInterface({ input: new PassThrough() });
rl.close();
rl.question("q? ").catch(e => console.log("caught", e.code));

Node prints caught ERR_USE_AFTER_CLOSE. Bun throws synchronously before .catch is attached:

error: readline was closed
 code: "ERR_USE_AFTER_CLOSE"

Same for an invalid options.signal (ERR_INVALID_ARG_TYPE). question() is a promise API, so callers handle errors with .catch() or try { await } inside an async function that is usually not on the stack at the call site. Any caller racing a close() (a SIGINT handler, a 'line' handler that closes the interface, an aborted createInterface({ signal })) gets an uncaught synchronous exception where Node delivers a handled rejection.

Cause

Node evaluates the whole question() body inside new Promise((resolve, reject) => { ... }), so validateAbortSignal()'s throw and [kQuestion]'s ERR_USE_AFTER_CLOSE throw are converted to rejections by the executor. Bun's PromisesInterface.question built its capability with $newPromiseCapability and called validateAbortSignal(signal, ...) and this[kQuestion](query, cb) at the top level of the method, so both throws escaped synchronously.

Fix

Match Node: wrap the whole body in the executor. This also routes the already-aborted fast path through reject() instead of the detached PromiseReject (that receiver bug is tracked more broadly in #33343, which also covers the util.promisify(rl.question) sibling and unrelated call sites).

Verification

Three tests added to test/js/node/readline/readline_promises.node.test.ts: closed interface (ERR_USE_AFTER_CLOSE), invalid options.signal (ERR_INVALID_ARG_TYPE), and an already-aborted signal (AbortError/ABORT_ERR with cause preserved). All three fail on main (the first two with a synchronous throw, the third with TypeError: |this| is not an object) and pass with this change. The existing readline.node.test.ts suite (79 tests) continues to pass.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 14 minutes

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: 6cb9bbbb-e55e-4c88-95be-6dd79f023588

📥 Commits

Reviewing files that changed from the base of the PR and between eead2f6 and 8ec4e24.

📒 Files selected for processing (2)

• src/js/node/readline.ts
• test/js/node/readline/readline_promises.node.test.ts

---

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

[robobun]

^{Updated 1:01 AM PT - Jul 8th, 2026}

@robobun, your commit 8ec4e24 is building: #70423

[github-actions]

This PR may be a duplicate of:

1. readline: reject instead of throwing when question() gets an aborted signal #33343 - Both fix question() to reject instead of throw when called with an already-aborted signal; readline: reject instead of throwing when question() gets an aborted signal #33343 binds the PromiseReject receiver while this PR wraps the body in new Promise(executor)
2. readline: throw ERR_USE_AFTER_CLOSE from write(), pause() and resume() after close() #33344 - Uses the exact same new Promise(executor) wrapping approach for question() and covers the same three error cases (ERR_USE_AFTER_CLOSE, ERR_INVALID_ARG_TYPE, ABORT_ERR)

🤖 Generated with Claude Code

[robobun]

Closing as a duplicate of #33344, which already wraps PromisesInterface.question so ERR_USE_AFTER_CLOSE, ERR_INVALID_ARG_TYPE, and the pre-aborted ABORT_ERR all reject instead of throwing synchronously, and additionally restores the write()/pause()/resume() guards. #33343 covers the remaining unbound PromiseReject sites.

[claude]

LGTM — straightforward Node compat fix wrapping the method body in the Promise executor so validation/close errors reject instead of throwing synchronously.

Extended reasoning...

Overview

This PR modifies a single method, PromisesInterface.question() in src/js/node/readline.ts, to wrap its entire body inside new Promise((resolve, reject) => {...}) instead of using $newPromiseCapability with top-level calls. The logic is otherwise unchanged — same signal validation, same abort-listener wiring, same this[kQuestion] call — but now any synchronous throw (from validateAbortSignal or ERR_USE_AFTER_CLOSE in [kQuestion]) is captured by the executor and delivered as a rejection, matching Node.js. It also replaces the detached PromiseReject(...) on the already-aborted fast path with reject(...), fixing a latent |this| is not an object TypeError. Three tests are added to test/js/node/readline/readline_promises.node.test.ts covering the closed-interface, invalid-signal, and already-aborted-signal paths.

Security risks

None. This is a pure error-delivery-mechanism change in a user-facing readline API — no auth, crypto, filesystem, or network surface is touched, and no new inputs are accepted.

Level of scrutiny

Low. The diff is ~20 lines in one method of a Node-compat JS builtin, and the transformation is mechanical (move existing statements inside an executor). The arrow-function executor correctly preserves this for this[kQuestionCancel]() and this[kQuestion](query, cb). This is exactly how Node implements the same method, so there is no design ambiguity.

Other factors

The bug-hunting system found no issues. Tests are hermetic (PassThrough input, no network/timers), assert specific error name/code/cause, and clean up via try/finally. The PR description documents that all three tests fail on main and pass with the change, and that the existing 79-test readline suite still passes. No CODEOWNERS cover this path and there are no outstanding human review comments.