Skip to content

http: propagate highWaterMark to ClientRequest OutgoingMessage#64653

Open
trivenay wants to merge 1 commit into
nodejs:mainfrom
trivenay:http-client-hwm-propagate
Open

http: propagate highWaterMark to ClientRequest OutgoingMessage#64653
trivenay wants to merge 1 commit into
nodejs:mainfrom
trivenay:http-client-hwm-propagate

Conversation

@trivenay

@trivenay trivenay commented Jul 21, 2026

Copy link
Copy Markdown

http.request({ highWaterMark }) passes the value to the TCP socket via createConnection() but does not set it on the OutgoingMessage's internal kHighWaterMark. OutgoingMessage._writeRaw() has two mutually exclusive write paths:

  • Path A (socket connected): conn.write() — uses socket's HWM (correct)
  • Path B (no socket yet): outputSize < this[kHighWaterMark] — uses OutgoingMessage's own default (64 KB) (incorrect)

Because the OutgoingMessage constructor already accepts options.highWaterMark, the fix is to set kHighWaterMark from the user's options after they are parsed in the ClientRequest constructor.

This resolves two symptoms:

  1. write() returning the wrong boolean for pre-socket writes (the user's highWaterMark was silently ignored on all Node versions).
  2. A deadlock on Node >= 24.16.0 where the incorrect false return sets kNeedDrain, but drain never fires because the socket was never backpressured (introduced by the stricter drain gate in http: emit 'drain' on OutgoingMessage only after buffers drain #62936).

Test results (compiled from source)

=== UNPATCHED Node v26.5.0 ===
FAIL: write() returned false (expected true)

=== PATCHED Node v27.0.0-pre (this PR) ===
PASS: write() returned true
test-http-client-highwatermark.js passes (exit 0)
test-http-server-options-highwatermark.js passes
test-http-outgoing-drain-writable-length.js passes
test-http-outgoing-end-cork.js passes
test-http-highwatermark.js passes
test-http-response-drain-cork.js passes

Minimal reproduction

node -e "require('http').createServer((req,res)=>{req.resume();req.on('end',()=>res.end('ok'))}).listen(9001)"
const http = require('http');
const req = http.request({ hostname: '127.0.0.1', port: 9001, method: 'POST', highWaterMark: 100_000 });
const ok = req.write(Buffer.alloc(64 * 1024));
console.log(`write() returned ${ok}, expected true (64KB < 100KB highWaterMark)`);
if (!ok) {
  setTimeout(() => { console.error('DEADLOCK: drain never fired'); process.exit(1); }, 3000);
  req.on('drain', () => { console.log('drain fired'); req.end(); });
} else {
  req.end();
  req.on('response', (res) => { res.resume(); res.on('end', () => process.exit(0)); });
}
Node version write() returns Drain fires? Outcome
v22.x false (wrong) Yes (old eager drain) HWM ignored but no hang
v26.5.0 false (wrong) Never DEADLOCK
This PR true (correct) N/A (no backpressure) Works correctly

Fixes: #64645
Refs: #62936

`http.request({ highWaterMark })` passes the value to the TCP socket
via createConnection() but does not set it on the OutgoingMessage
internal kHighWaterMark.  OutgoingMessage._writeRaw() has two mutually
exclusive write paths:

  Path A (socket connected): conn.write() — uses socket HWM ✓
  Path B (no socket yet):    outputSize < this[kHighWaterMark] — uses
                             OutgoingMessage own default (64 KB) ✗

Because the OutgoingMessage constructor already accepts
options.highWaterMark, the fix is to set kHighWaterMark from the
user options after they are parsed in the ClientRequest constructor.

This resolves two symptoms:

  1. write() returning the wrong boolean for pre-socket writes (the
     user highWaterMark was silently ignored on all Node versions).

  2. A deadlock on Node >= 24.16.0 where the incorrect false return
     sets kNeedDrain, but drain never fires because the socket was
     never backpressured (introduced by the stricter drain gate in
     nodejs#62936).

Signed-off-by: Trivikram Narayanan Kamasamudram <trivenay@amazon.com>
Fixes: nodejs#64645
Refs: nodejs#62936
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http: ClientRequest highWaterMark option not propagated to OutgoingMessage

2 participants