Skip to content

http: avoid stream listeners on idle agent sockets#64004

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-http-agent-free-socket-onread
Open

http: avoid stream listeners on idle agent sockets#64004
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-http-agent-free-socket-onread

Conversation

@mcollina

@mcollina mcollina commented Jun 19, 2026

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.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@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 Jun 19, 2026
@mcollina

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 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

Copy link
Copy Markdown
Member Author

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

@Renegade334 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.

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 Renegade334 added the fast-track PRs that do not need to wait for 72 hours to land. label Jun 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

@github-actions 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

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fast-track PRs that do not need to wait for 72 hours to land. http Issues or PRs related to the http subsystem. 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 needs-ci PRs that need a full CI run.

Projects

None yet

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

5 participants