fix(http): wire res.write/res.end callbacks + backpressure boolean into static dispatch (#4909) - #4954
Merged
Merged
Conversation
…to static dispatch (#4909) `ServerResponse.write(chunk, cb)` / `end(chunk, cb)` silently dropped the callback and `res.write()` returned `undefined` when called through the *static* native dispatch table (`res` typed as `ServerResponse`). The table routed `write`/`end` to the no-callback runtime entrypoints (`js_node_http_res_write` with `args: &[NA_F64]` / `ret: NR_I32`→undefined), even though the callback-aware `_with_cb` infra from #4904 already existed — it was only reachable via the dynamic (`res: any`) dispatch path in `handle_dispatch.rs`. Effect: any producer that relied on write/end callbacks firing (or on the boolean backpressure return) stalled — `while (res.write(buf)) {}` loops never saw `false`, and `res.end(cb)` callbacks never ran — which surfaced as the silent server-side hangs catalogued in #4909's outgoing-message-framing cluster. Fix: route the static table's `write`/`end` rows to new full-surface runtime functions `js_node_http_res_write_full` / `js_node_http_res_end_full` (`args: &[NA_F64, NA_JSV, NA_JSV]`): - the trailing `(encoding?, callback?)` args arrive as raw NaN-boxed JSValues; the callback is the closure-valued one (validated with `js_value_is_closure` so a `Buffer`/object chunk — also POINTER_TAG — is never mistaken for a callback), and `end(cb)` with the callback in the first slot is handled too; - `write` returns a real NaN-boxed boolean (`NR_F64`): `false` once the buffered body passes Node's default 16 KiB high-water mark, so `while (res.write(buf))` producer loops terminate instead of spinning; - `end` runs queued write callbacks → `'finish'` listeners → end callback → `'close'`, matching Node's ordering (`'finish'` never precedes the end cb). `js_node_http_res_end` is refactored to share the finalize step via `finalize_buffered_end`, which returns the listener lists without firing them so the caller controls ordering. Verified: `test-http-outgoing-message-write-callback.js` now matches Node byte-for-byte (was exit 1 — callbacks dropped → empty results); isolated repros for `write(chunk,cb)`, `write(chunk,enc,cb)`, `end(chunk,enc,cb)`, `end(cb)`, `end(Buffer)` and the finish/end-cb ordering all match Node; a throwing handler still crashes (exit 1) as before; 23 crate unit tests green; no new hangs across a res/write-focused regression batch. Part of #4909 (outgoing-message-framing sub-theme of the #2132 http tail).
proggeramlug
added a commit
that referenced
this pull request
Jun 11, 2026
…eouts, dynamic listener registration (#4909) (#4964) Sub-tickets (1) and (2) of the #4909 silent-hang triage, mirroring the #4954 server-side fix onto the perry-ext-http client path: - req.write(chunk[, enc][, cb]) queues its callback and returns a real NaN-boxed backpressure boolean (false past the 16 KiB high-water mark), so `while (req.write(buf, cb))` producer loops terminate. Buffer chunks are read through a buffer-aware reader instead of being misparsed as StringHeaders (same layout confusion as #1124). - req.end([chunk][, enc][, cb]) handles the end(cb) form and fires the queued write callbacks -> 'finish' -> end callback in Node's flush order via a new PendingHttpEvent::Flushed drained by the client pump. - req.setTimeout(ms[, cb]) arms a real timer (tokio sleep -> Timeout event) instead of only storing the delay, so 'timeout' fires even for a request that was never dispatched or whose server never responds; options.timeout arms the same timer at request creation. 'timeout' is emitted at most once and never after completion; setTimeout(0) clears the timer per Node. - req.destroy() on an in-flight request emits the coded ECONNRESET "socket hang up" then 'close' (once-only via close_emitted); the response/error drains suppress events for completed/destroyed requests; '.on("response")' listeners now fire alongside the factory callback. - Dynamic dispatch (untyped receivers): on/once/addListener/ removeListener/removeAllListeners now register/remove client-request listeners (the stdlib dispatch_http name-gate hid "on" so listeners were silently dropped), and `constructor` returns a `{ name }` object so out.constructor.name discriminates ClientRequest/ServerResponse/ IncomingMessage. - Server side: js_node_http_im_resume no longer consumes the one-shot 'end'/'data' emit flags when no listener is registered yet, fixing the canonical `req.resume(); req.on('end', cb)` drain pattern that hung every response; js_node_http_res_write_with_cb returns the same backpressure boolean as the static _full path; js_node_http_res_end_with_cb fires write cbs -> 'finish' -> end cb -> 'close' in Node order; and closure_arg validates with js_value_is_closure so a Buffer chunk in `res.end(buf, cb)` is no longer invoked as the callback (TypeError). Corpus bucket (#4909, 108 tests, baseline vs fixed on the same comparator): +8 fail->pass (outgoing-finish, client-timeout-event, agent-uninitialized x2, early-hints, client-race x2 and timeout-client-warning — the latter three byte-differ only by Node's DEP0169/pid noise the radar normalizes), 0 regressions; most remaining runtime-fails now exit with a real assertion message instead of the silent hang this issue tracks. Found while triaging (pre-existing, unchanged): test-http-response-setheaders SIGSEGVs nondeterministically on baseline and fixed builds alike. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ServerResponse.write(chunk, cb)/end(chunk, cb)silently dropped the callback, andres.write()returnedundefinedinstead of a boolean, when called through the static native dispatch table (the common case —restyped asServerResponse). The table routedwrite/endto the no-callback runtime entrypoints (js_node_http_res_write,args: &[NA_F64],ret: NR_I32→undefined), even though the callback-aware_with_cbinfra from #4904 already existed — it was only reachable via the dynamic (res: any) path inhandle_dispatch.rs.The result was silent server-side stalls:
while (res.write(buf)) {}never sawfalse, andres.end(cb)/res.write(chunk, cb)callbacks never ran — the outgoing-message-framing failure mode catalogued in #4909.Fix
Route the static table's
write/endrows to new full-surface runtime functions (args: &[NA_F64, NA_JSV, NA_JSV]):js_node_http_res_write_full— trailing(encoding?, callback?)args arrive as raw NaN-boxed JSValues; the callback is the closure-valued one (validated withjs_value_is_closure, so aBuffer/object chunk — also POINTER_TAG — is never mistaken for a callback). Returns a real NaN-boxed boolean (NR_F64):falseonce the buffered body passes Node's default 16 KiB high-water mark, sowhile (res.write(buf))loops terminate.js_node_http_res_end_full— handlesend(cb)(callback first) as well asend(chunk[, enc][, cb]). Runs queued write callbacks →'finish'listeners → end callback →'close', matching Node's ordering ('finish'never precedes the end cb).js_node_http_res_endis refactored to share the finalize step viafinalize_buffered_end, which returns the listener lists without firing them so the caller owns ordering.Verification
test-http-outgoing-message-write-callback.jsnow matches Node byte-for-byte (was exit 1 — callbacks dropped →resultsstayed empty →deepStrictEqualthrew). Clean fail→pass.write(chunk,cb),write(chunk,enc,cb),end(chunk,enc,cb),end(cb),end(Buffer), and the finish/end-cb ordering all match Node exactly.perry-ext-http-serverunit tests green; no new hangs across ares/write-focused regression batch.Scope note
#4909 is a triage meta-issue (89 tests exiting non-zero with no output). This PR fixes one concrete, common root cause in the outgoing-message-framing sub-theme and is a prerequisite for that cluster. Most of the remaining 89 tests still stall on other, independent unimplemented features (client-side
req.writebackpressure, 100-continue/writeContinue,req.setTimeout, agent abort/keep-alive lifecycle,rawHeaders/headersDistinct/uniqueHeaders,writeHeadreturningthis, …). A triage breakdown is posted on #4909 for splitting into sub-tickets per the issue's acceptance criteria.Part of #4909 / #2132.