http: propagate highWaterMark to ClientRequest OutgoingMessage by trivenay · Pull Request #64653 · nodejs/node

trivenay · 2026-07-21T13:43:40Z

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:

write() returning the wrong boolean for pre-socket writes (the user's highWaterMark was silently ignored on all Node versions).
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

nodejs-github-bot · 2026-07-21T13:43:45Z

Review requested:

@nodejs/http
@nodejs/net

`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 <[email protected]> Fixes: nodejs#64645 Refs: nodejs#62936

codecov · 2026-07-21T20:09:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.13%. Comparing base (8c1d128) to head (a008632).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #64653   +/-   ##
=======================================
  Coverage   90.12%   90.13%           
=======================================
  Files         741      741           
  Lines      242091   242142   +51     
  Branches    45563    45563           
=======================================
+ Hits       218180   218250   +70     
+ Misses      15430    15394   -36     
- Partials     8481     8498   +17

Files with missing lines	Coverage Δ
lib/_http_client.js	`97.62% <100.00%> (+0.01%)`	⬆️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

trivikr

LGTM

nodejs-github-bot · 2026-07-22T04:32:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/74993/

pimterry

I think there's a missing case to watch for here with agents or createConnection. In those cases, the request often doesn't directly create or own the socket - an agent might create it independently, it might be a reuse of an existing socket from another request, or it could be a custom non-socket stream from anywhere, and in each case it may have its own independent highWaterMark option already set.

I'm not sure but I strongly suspect this won't properly handle that, since it's assuming that the request owns the socket and controls the HWM setting. I think if you reproduce the HWM mismatch in one of those contexts there's still a route to a deadlock with no drain in some cases.

I won't block this - it is still a clear improvement on the current state in the meantime, but if you have time to investigate and potentially fix that flow en route that would be fantastic.

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

trivenay force-pushed the http-client-hwm-propagate branch from b1249d0 to a008632 Compare July 21, 2026 18:13

trivikr approved these changes Jul 21, 2026

View reviewed changes

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 21, 2026

metcoder95 approved these changes Jul 21, 2026

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 21, 2026

This comment was marked as outdated.

Sign in to view

pimterry approved these changes Jul 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

http: propagate highWaterMark to ClientRequest OutgoingMessage#64653

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

trivenay commented Jul 21, 2026 •

edited

Loading

Uh oh!

nodejs-github-bot commented Jul 21, 2026

Uh oh!

codecov Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

trivikr left a comment

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Jul 22, 2026

Uh oh!

pimterry left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

trivenay commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test results (compiled from source)

Minimal reproduction

Uh oh!

nodejs-github-bot commented Jul 21, 2026

Uh oh!

codecov Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

trivikr left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Jul 22, 2026

Uh oh!

pimterry left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

trivenay commented Jul 21, 2026 •

edited

Loading

codecov Bot commented Jul 21, 2026 •

edited

Loading