FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http: avoid stream listeners on idle agent sockets by mcollina · Pull Request #64004 · nodejs/node · GitHub

/ node Public

http: avoid stream listeners on idle agent sockets - #64004

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:fix-http-agent-free-socket-onread
Jun 20, 2026
ConversationCommits (1)ChecksFiles changed
Merged

http: avoid stream listeners on idle agent sockets#64004
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:fix-http-agent-free-socket-onread

Conversation

mcollina commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes: #63989

The response-queue-poisoning guard added to idle http.Agent sockets used a public 'data' listener. That made node-fetch@2 observe socket.listenerCount('data') > 0 during response close and report false ERR_STREAM_PREMATURE_CLOSE errors.

This changes the idle-socket guard to use the socket handle's internal onread hook while the socket is in the free pool, restoring the normal stream read callback when the socket is reused. The guard still destroys sockets that receive unsolicited data while idle, but it no longer adds public stream listeners.

nodejs-github-bot commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

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

mcollina commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

@nodejs/releasers unfortunately, we would need to backport this to node 22.x and 24.x to fix node-fetch@2.

mcollina added lts-watch-v22.x PRs that may need to be released in v22.x lts-watch-v24.x PRs that may need to be released in v24.x labels Jun 19, 2026

mcollina commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

I'm sorry I should have caught this before shipping.

Renegade334 approved these changes Jun 19, 2026
View reviewed changes

Renegade334 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Side-note: I had a cursory look for guidance on Node.js internals managing EventEmitter events on emitter-derived Node.js classes, and found none. My feeling is that we don't ordinarily consider changes related to internally-managed event listeners to be semver-major, even though they're publicly observable. Maybe we should elucidate what our stance is, and make it explicit in the EventEmitter documentation?

Renegade334 added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 19, 2026

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @Renegade334. Please 👍 to approve.

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2026
mcollina mentioned this pull request Jun 19, 2026
Closed
minusinus mentioned this pull request Jun 19, 2026
Closed
2 tasks
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2026

nodejs-github-bot commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

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

MikeMcC399 mentioned this pull request Jun 19, 2026
Closed
mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026

nodejs-github-bot commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

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

http: avoid stream listeners on idle agent sockets
0d62f61 
Signed-off-by: Matteo Collina <hello@matteocollina.com>
mcollina force-pushed the fix-http-agent-free-socket-onread branch from 8d8919c to 0d62f61 Compare June 20, 2026 08:30
mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2026

nodejs-github-bot commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

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

BYK mentioned this pull request Jun 24, 2026
Merged
joeluong-sfcc mentioned this pull request Jun 24, 2026
Merged
10 tasks
kaviththiranga mentioned this pull request Jun 25, 2026
Merged
lukaszjaskowski mentioned this pull request Jun 25, 2026
Merged
GaelS mentioned this pull request Jun 25, 2026
Merged
NumanAnees reviewed Jun 25, 2026
View reviewed changes

NumanAnees left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM!

jsumners-nr mentioned this pull request Jun 25, 2026
Merged
BYK mentioned this pull request Jun 25, 2026
Merged
iskounen mentioned this pull request Jun 25, 2026
Merged
ryium mentioned this pull request Jun 26, 2026
Closed
18 tasks
shojiiii mentioned this pull request Jun 26, 2026
Closed
cardox6 mentioned this pull request Jun 26, 2026
Merged
This was referenced Jun 26, 2026
Closed
Closed
BYK mentioned this pull request Jun 26, 2026
Merged
mattwiller mentioned this pull request Jun 26, 2026
Merged
rhdedgar mentioned this pull request Jun 26, 2026
Closed
2 tasks
kkohtaka mentioned this pull request Jun 27, 2026
Merged
kuu13580 mentioned this pull request Jun 27, 2026
Merged
6 tasks
maximilianoertel mentioned this pull request Jun 28, 2026
Merged
JohannsisBaires mentioned this pull request Jun 30, 2026
Closed
MaximusHaximus mentioned this pull request Jul 1, 2026
Merged
BigGillyStyle mentioned this pull request Jul 1, 2026
Merged
4 tasks
petebacondarwin mentioned this pull request Jul 2, 2026
Merged
5 tasks
hmgaudecker mentioned this pull request Jul 3, 2026
Merged
h-ikeda mentioned this pull request Jul 4, 2026
Merged
charlesBochet mentioned this pull request Jul 8, 2026
Merged
MikeMcC399 mentioned this pull request Jul 14, 2026
Open
jonathanawesome mentioned this pull request Jul 17, 2026
Merged
strawberryeleven mentioned this pull request Aug 18, 2026
Open
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
anonrig anonrig approved these changes
ronag ronag approved these changes
Renegade334 Renegade334 approved these changes
+2 more reviewers
NumanAnees NumanAnees left review comments
bjohansebas bjohansebas approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. backported-to-v22.x PRs backported to the v22.x-staging branch. backported-to-v24.x PRs backported to the v24.x-staging branch. fast-track PRs that do not need to wait for 48 hours to land. http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

http: node-fetch throws ERR_STREAM_PREMATURE_CLOSE on keep-alive socket closures after latest security releases
Back | FazBrowse Home | New Git URL