http: add optimizeEmptyRequests option for IncomingMessage._dump #59778

RafaelGSS · 2025-09-05T20:37:49Z

Backportable version of #59747

cc: @ronag

nodejs-github-bot · 2025-09-05T20:37:55Z

Review requested:

@nodejs/http
@nodejs/net

codecov · 2025-09-05T21:32:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.45%. Comparing base (c6316f9) to head (3a95d3d).
⚠️ Report is 134 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #59778   +/-   ##
=======================================
  Coverage   88.45%   88.45%           
=======================================
  Files         703      703           
  Lines      207546   207579   +33     
  Branches    40011    40011           
=======================================
+ Hits       183591   183622   +31     
- Misses      15949    15951    +2     
  Partials     8006     8006

Files with missing lines	Coverage Δ
lib/_http_incoming.js	`99.35% <100.00%> (+0.01%)`	⬆️
lib/_http_server.js	`97.35% <100.00%> (+0.05%)`	⬆️

... and 29 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.

nodejs-github-bot · 2025-09-06T11:17:44Z

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

marco-ippolito · 2025-09-06T19:37:05Z

lib/_http_server.js

+  const fastDump = options.fastDump;
+  if (fastDump !== undefined)
+    validateBoolean(fastDump, 'options.fastDump');
+  this[kfastDump] = fastDump || false;


The OR seems redudant since we are passing through validateBoolean

How so? if fastDump is undefined, this[kfastDump] will be undefined instead of false.

You could shortcut that just a bit by assigning a default:

const { fastDump = false } = options.fastDump; validateBoolean(fastDump, 'options.fastDump'); this[kfastDump] = fastDump;

doc/api/http.md

lib/_http_server.js

pimterry · 2025-09-08T10:10:25Z

Thinking more about the heuristics mentioned at the end #59747, it looks like we can go further and do even better here, very easily.

I think we can do this for all methods, quickly & safely, because it's already impossible to receive a request with a body without it being indicated it in the headers (content-length or transfer-encoding), even in the current implementation, even for POST and others etc.

On Node v24 right now, if you send:

POST / HTTP/1.1
Host: test

message body

Then Node currently fires request and end (but no data), and then returns a 400 response to the unparseable message body (which it expects to be $METHOD $PATH HTTP/1.1 starting a new request). That's because the RFC possibilities for sending request bodies always require either content-length or transfer-encoding to be set to warn you there's a body coming, and our parsing already requires this as well.

That means we could safely apply this optimization to all cases, whenever there's no content-length or transfer-encoding header set (we could look for chunked or check for != 0, but since it's primarily a performance optimization I think a quicker simpler check is fine to start with). We already have the headers parsed here so that's very quick & easy. That lets us apply the same performance boost to POSTs and all other requests with an empty body (which is reasonably common I think - there's plenty of REST APIs where the URL defines the operation, but the request is a POST because it's not safe).

This would still be breaking behaviour, because it doesn't emit end or close events, but as an opt-in option (implicitlyCloseEmptyRequests or similar) then that's fine. Or we could check for event listeners and just manually emit the events if any are registered, in which case this wouldn't be breaking at all AFAICT, so we could apply the optimization to everybody for free, but I don't know if the overhead of that negates too much of the perf boost.

RafaelGSS · 2025-09-08T16:14:16Z

Good one @pimterry. I'll implement those later today and the tests asked by @Flarna.

RafaelGSS · 2025-09-09T12:57:31Z

Just pushed a new version (amended) that replaces the fastDump with optimizeEmptyRequest option and as suggested by @pimterry, we'll dump the IncomingMessage whenever this option is true and when there's no content-length/transfer-encoding or the method is HEAD/GET.

PTAL.

Flarna · 2025-09-09T13:36:46Z

lib/_http_server.js

+    // stream.Readable life cycle rules. The downside is that this will
+    // break some servers that read bodies for methods that don't have body headers.
+    req._dumped = true;
+    req._readableState.ended = true;


Could we move the modifications of stream internals into stream files?
I wonder also that endEmitted/ closeEmitted are set here. Are they really emitted?

I wonder also that endEmitted/ closeEmitted are set here. Are they really emitted?

They are as if they were already emitted.

lib/_http_server.js

doc/api/http.md

RafaelGSS · 2025-09-09T18:50:10Z

Applied the suggestions and added a test to guarantee .end event isn't emitted if the flag is true. I will do a second round of performance comparison on Fastify (Express doesn't seem to use req object as I thought it would use it - so the result wasn't so dramatic).

PTAL @Flarna @pimterry

test/parallel/test-http-server-optimize-empty-requests.js

lib/_http_incoming.js

Flarna · 2025-09-09T20:52:49Z

@lpinca commented on the previous PR, maybe worth to ask them for a look a this one.

nodejs-github-bot · 2025-09-15T19:28:29Z

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

RafaelGSS · 2025-09-16T20:53:32Z

PTAL @pimterry @mcollina @jasnell @ronag I had to push a small fix

anonrig · 2025-09-16T20:58:16Z

doc/api/http.md

+    or `Transfer-Encoding` headers (indicating no body) will be initialized with an
+    already-ended body stream, so they will never emit any stream events
+    (like `'data'` or `'end'`). You can use `req.readableEnded` to detect this case.
+    This option is still under experimental phase.


If this is indeed true, we should emit an experimental warning whenever this flag is used. If this is no longer correct, we should remove this comment.

I don't think emitting a warning for this specific option would be good. I imagine this being enabled on frameworks like fastify and express, and having such a warning might point users away. I am confident that the documentation note is enough for now.

What do you think? @mcollina @ronag

Than I recommend emitting a warning if it's not inside node_modules?

I have added the experimental documentation here only for the next two v24 releases at least, just in case we are missing some specific edge case that would completely change how this flag works - you know, touching on http usually breaks people in many ways. Ideally, this line should be removed in a couple of releases.

I'm not sure if it's worth it to consider such a warning since we'll remove it very soon.

@anonrig I have removed the experimental comment.

lib/_http_server.js

RafaelGSS · 2025-09-24T20:26:55Z

Ping @anonrig

Signed-off-by: RafaelGSS <[email protected]> Co-Authored-By: RafaelGSS <[email protected]>

nodejs-github-bot · 2025-09-30T16:06:23Z

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

nodejs-github-bot · 2025-10-13T20:16:00Z

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

nodejs-github-bot · 2025-10-14T12:17:55Z

Landed in 0c35aaf

Signed-off-by: RafaelGSS <[email protected]> Co-Authored-By: RafaelGSS <[email protected]> PR-URL: #59778 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Tim Perry <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Notable changes: http: * (SEMVER-MINOR) add optimizeEmptyRequests server option (Rafael Gonzaga) #59778 sqlite: * (SEMVER-MINOR) allow setting defensive flag (Bart Louwers) #60217 src: * (SEMVER-MINOR) add watch config namespace (Marco Ippolito) #60178 PR-URL: #60436

nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Sep 5, 2025

RafaelGSS force-pushed the opt-in-faster-dump branch from 7961667 to 6e5fdcb Compare September 5, 2025 20:38

ronag approved these changes Sep 6, 2025

View reviewed changes

gurgunday approved these changes Sep 6, 2025

View reviewed changes

ronag requested a review from mcollina September 6, 2025 12:56

marco-ippolito reviewed Sep 6, 2025

View reviewed changes

Flarna reviewed Sep 7, 2025

View reviewed changes

doc/api/http.md Outdated Show resolved Hide resolved

jasnell reviewed Sep 7, 2025

View reviewed changes

doc/api/http.md Outdated Show resolved Hide resolved

jasnell reviewed Sep 7, 2025

View reviewed changes

lib/_http_server.js Outdated Show resolved Hide resolved

RafaelGSS force-pushed the opt-in-faster-dump branch from 6e5fdcb to 2d617ab Compare September 9, 2025 12:56

RafaelGSS requested review from Flarna, jasnell and ronag September 9, 2025 12:57

Flarna reviewed Sep 9, 2025

View reviewed changes

ronag reviewed Sep 9, 2025

View reviewed changes

lib/_http_server.js Outdated Show resolved Hide resolved

pimterry requested changes Sep 9, 2025

View reviewed changes

lib/_http_server.js Outdated Show resolved Hide resolved

doc/api/http.md Show resolved Hide resolved

RafaelGSS added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 9, 2025

RafaelGSS changed the title ~~http: optimize IncomingMessage._dump~~ http: add optimizeEmptyRequests option for IncomingMessage._dump Sep 9, 2025

ronag approved these changes Sep 9, 2025

View reviewed changes

Flarna reviewed Sep 9, 2025

View reviewed changes

test/parallel/test-http-server-optimize-empty-requests.js Outdated Show resolved Hide resolved

Flarna reviewed Sep 9, 2025

View reviewed changes

lib/_http_incoming.js Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

RafaelGSS force-pushed the opt-in-faster-dump branch from e33471d to e6cd7d2 Compare September 15, 2025 19:26

RafaelGSS added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 16, 2025

anonrig requested changes Sep 16, 2025

View reviewed changes

anonrig reviewed Sep 16, 2025

View reviewed changes

lib/_http_server.js Show resolved Hide resolved

RafaelGSS force-pushed the opt-in-faster-dump branch from e6cd7d2 to 29753ac Compare September 23, 2025 14:26

This comment was marked as outdated.

Sign in to view

ronag approved these changes Sep 23, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

http: add optimizeEmptyRequests server option

3a95d3d

Signed-off-by: RafaelGSS <[email protected]> Co-Authored-By: RafaelGSS <[email protected]>

RafaelGSS force-pushed the opt-in-faster-dump branch from 29753ac to 3a95d3d Compare September 29, 2025 18:12

This comment was marked as outdated.

Sign in to view

anonrig approved these changes Sep 30, 2025

View reviewed changes

RafaelGSS added the performance Issues and PRs related to the performance of Node.js. label Oct 13, 2025

RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 14, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 14, 2025

nodejs-github-bot merged commit 0c35aaf into nodejs:main Oct 14, 2025
60 checks passed

github-actions bot mentioned this pull request Oct 27, 2025

2025-10-28, Version 25.1.0 (Current) #60436

Merged

Uh oh!

Uh oh!

http: add optimizeEmptyRequests option for IncomingMessage._dump #59778

http: add optimizeEmptyRequests option for IncomingMessage._dump #59778

Conversation

RafaelGSS commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 5, 2025

Uh oh!

codecov bot commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Sep 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pimterry commented Sep 8, 2025

Uh oh!

RafaelGSS commented Sep 8, 2025

Uh oh!

RafaelGSS commented Sep 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RafaelGSS commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Flarna commented Sep 9, 2025

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Sep 15, 2025

Uh oh!

RafaelGSS commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

RafaelGSS commented Sep 24, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Sep 30, 2025

Uh oh!

nodejs-github-bot commented Oct 13, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

RafaelGSS commented Sep 5, 2025 •

edited

Loading

codecov bot commented Sep 5, 2025 •

edited

Loading

RafaelGSS commented Sep 9, 2025 •

edited

Loading

RafaelGSS commented Sep 16, 2025 •

edited

Loading