fix(http): client write/end callbacks + backpressure, real client timeouts, dynamic listener registration (#4909)#4964
Merged
Conversation
…eouts, dynamic listener registration (#4909) 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.
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
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.
Sub-tickets (1) client-side OutgoingMessage write/end callbacks + backpressure and (2) client HTTP timeouts from the #4909 triage, mirroring the #4954 server-side fix onto the
perry-ext-httpclient path — plus the cross-cutting dispatch bugs that kept the corpus shapes hanging.Root causes fixed
Client
req.write/req.end(perry-ext-http)req.write/req.endto single-arg entry points: the(encoding?, callback?)tail was dropped andwrite()returned the always-truthy handle, sowhile (req.write(buf, cb))producer loops never terminated (the dominanttest-http-outgoing-*hang). Newjs_http_client_request_write_full/_end_fullqueue the callbacks, return a real NaN-boxed backpressure boolean (16 KiB HWM), and accept Buffer chunks (previously misread asStringHeaders — same layout confusion as node:http res.write(Buffer)/res.end(Buffer): binary contents zeroed on wire (length preserved) #1124).PendingHttpEvent::Flushed: write callbacks →'finish'→ end callback.Client timeouts
req.setTimeout(ms, cb)only stored the delay; the deadline existed only once a request was dispatched. The canonicalreq.setTimeout(n); req.on('timeout', …)against a never-responding server (or beforereq.end()) hung forever. A real timer is now armed atsetTimeout()/ at request creation foroptions.timeout;'timeout'fires at most once and never after completion;setTimeout(0)clears, per Node.req.destroy()on an in-flight request emits the codedECONNRESET"socket hang up" then'close'(once-only); response/error drains are suppressed for completed/destroyed requests;.on('response')listeners now fire alongside the factory callback.Dynamic dispatch (untyped
outreceivers —function write(out) { … }corpus shape)out.on('finish', cb)silently dropped the listener: the stdlibdispatch_http.rsname-gate had no"on"in the method allowlist. Addedon/once/addListener/prependListener/removeListener/off/removeAllListenersto the gate and the ext dispatcher.out.constructor.namenow readsClientRequest/ServerResponse/IncomingMessage(plain{ name }stand-in object).Server side (perry-ext-http-server)
req.resume()consumed the one-shot'end'/'data'emit flags even with no listener registered yet, so the canonical drain patternreq.resume(); req.on('end', cb)never fired'end'and the server never responded — a second, independent cause of the silent hangs.res.write(chunk, cb)(js_node_http_res_write_with_cb) always returned1— no backpressure (the fix(http): wire res.write/res.end callbacks + backpressure boolean into static dispatch (#4909) #4954 fix only covered the static path); now returns the same HWM boolean.res.end(buf, cb)crashed withTypeError: value is not a function:closure_argaccepted any POINTER_TAG value, so the Buffer chunk was taken as theend(cb)callback form and invoked. Now validated viajs_value_is_closure.end_with_cbfired'finish'/'close'before the callbacks; reordered to Node's write-cbs →'finish'→ end-cb →'close'(matchingjs_node_http_res_end_full).Validation
origin/mainbuild vs this branch, same raw-byte comparator, 8 s cutoff): +8 fail→pass, 0 regressions —test-http-outgoing-finish(byte-for-byte),test-http-client-timeout-event,test-http-agent-uninitialized(-with-handle),test-http-early-hints, plustest-http-client-race(-2)andtest-http-timeout-client-warningwhich now differ only by Node's DEP0169-stderr/pid noise thatnode_core_subset.py's normalizer strips.(no output)hang this issue tracks — i.e. the triage acceptance ("capture the actual non-zero exit cause") holds for the residue.crates/perry/tests/issue_4909_client_write_end_timeouts.rs(both green): flush-order + backpressure round-trip, and setTimeout→'timeout'→destroy→ECONNRESET→'close'.cargo test -p perry-ext-http -p perry-ext-http-servergreen; curatednode-suitehttp/https and the in-repotest_parity_http(s)/gap http tests match baseline (no flips).test-http-outgoing-message-write-callback(fix(http): wire res.write/res.end callbacks + backpressure boolean into static dispatch (#4909) #4954's representative) still passes after theend_with_cbreorder.Out of scope / follow-ups for #4909
checkContinue/writeContinue), agent keep-alive socket reuse, streamed (multi-chunk) responses (res.flushHeaders+ late chunks), server-sidereq/res/server.setTimeouttimers (need a socket surface), raw-framing/smuggling cases.test-http-response-setheaders.jsSIGSEGVs nondeterministically (~6/8 runs) on both baseline and this branch — worth its own ticket.Code-only PR per convention — no version bump / changelog (maintainer folds at merge).
Part of #4909 / #2132.