http: fix keylog listener setup on existing agent sockets by shani-singh1 · Pull Request #65066 · nodejs/node · GitHub

maybeEnableKeylog() runs as the agent's 'newListener' handler and attaches the agent's keylog handler to the sockets the agent already owns:

// Existing sockets will start listening on keylog now.
const sockets = ObjectValues(this.sockets);
for (let i = 0; i < sockets.length; i++) {
  sockets[i].on('keylog', this[kOnKeylog]);
}

agent.sockets maps a name to an array of sockets, so ObjectValues() yields arrays, not sockets, and .on() is called on an array. Agent.prototype.destroy() in the same file gets this right with a nested walk over both maps.

Two things follow. Adding a 'keylog' listener to an agent that already owns a socket throws, and because the throw happens inside the 'newListener' handler it propagates out of agent.on() before the listener is stored, so the caller gets an exception and no listener. Separately, agent.freeSockets is never visited, so idle keep-alive sockets never start listening even once the crash is out of the way.

Reproduction

const http = require('http');

const server = http.createServer((req, res) => setTimeout(() => res.end('ok'), 300));

server.listen(0, '127.0.0.1', () => {
  const agent = new http.Agent({ keepAlive: true });
  const req = http.get({ host: '127.0.0.1', port: server.address().port, agent }, (res) => res.resume());
  req.on('error', () => {});
  req.on('socket', () => setImmediate(() => {
    console.log('bucket is an Array :', Array.isArray(Object.values(agent.sockets)[0]));
    try {
      agent.on('keylog', () => {});
      console.log('agent.on("keylog") : OK');
    } catch (err) {
      console.log('agent.on("keylog") : THREW ' + err.constructor.name + ': ' + err.message);
    }
    console.log('keylog listeners   :', agent.listenerCount('keylog'), '(expected 1)');
    agent.destroy();
    server.close();
    process.exit(0);
  }));
});

On v24.11.1:

bucket is an Array : true
agent.on("keylog") : THREW TypeError: sockets[i].on is not a function
keylog listeners   : 0 (expected 1)

The documented 'keylog' event on https.Agent is therefore unusable on any agent that has already opened a socket, which is the normal case for a long-lived agent. The existing coverage in test/parallel/test-https-agent-keylog.js registers the listener on https.globalAgent before any request is made, so agent.sockets is empty and the loop body never runs.

Change

Walk both maps the same way Agent.prototype.destroy() does.

Verification

Running the old and new loop bodies against a real http.Agent holding one idle socket and one in-flight socket:

sockets in freeSockets : 1
sockets in sockets     : 1
total existing sockets : 2
OLD loop               : THREW TypeError: sockets[i].on is not a function
NEW loop               : no throw
sockets wired by NEW   : 2 / 2  (expected all)

The added test uses two servers so the sockets get different names and one stays parked in freeSockets rather than being reused. It fails on main with the TypeError above at the agent.on('keylog', ...) line, and passes with this change.

Review requested:

@nodejs/http
@nodejs/net

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (51c0d94) to head (9058cd0).
⚠️ Report is 180 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65066      +/-   ##
==========================================
- Coverage   90.30%   90.28%   -0.03%     
==========================================
  Files         759      759              
  Lines      247644   247761     +117     
  Branches    46687    46716      +29     
==========================================
+ Hits       223645   223693      +48     
- Misses      15474    15520      +46     
- Partials     8525     8548      +23

Files with missing lines	Coverage Δ
lib/_http_agent.js	96.59% <100.00%> (+0.46%)	⬆️

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

Nice work, thanks for fixing this @shani-singh1!

Commit linting is failing as there's no sign-off trailer, so you'll need to git commit --amend -s and then force push to update it.

`maybeEnableKeylog()` runs as the agent's `'newListener'` handler and attaches the agent's keylog handler to the sockets the agent already owns. `agent.sockets` maps a name to an array of sockets, but the loop treated those arrays as sockets and called `.on()` on them. Adding a `'keylog'` listener to an agent that already owned a socket therefore threw `TypeError: sockets[i].on is not a function` out of `agent.on('keylog', ...)`. Since the throw happened inside the `'newListener'` handler it propagated before the listener was stored, so the caller got an exception and no listener. Sockets parked in `agent.freeSockets` were never visited at all. Walk both maps the way `Agent.prototype.destroy()` does. Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>

Thanks for the review!

Amended with -s and force pushed, so the head is now 9058cd0 and carries the sign-off trailer. I applied the same to my three other open PRs (#65064, #65065, #65067) since they would have hit the same lint failure once CI ran on them.

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

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

Landed in 45d6d31

`maybeEnableKeylog()` runs as the agent's `'newListener'` handler and attaches the agent's keylog handler to the sockets the agent already owns. `agent.sockets` maps a name to an array of sockets, but the loop treated those arrays as sockets and called `.on()` on them. Adding a `'keylog'` listener to an agent that already owned a socket therefore threw `TypeError: sockets[i].on is not a function` out of `agent.on('keylog', ...)`. Since the throw happened inside the `'newListener'` handler it propagated before the listener was stored, so the caller got an exception and no listener. Sockets parked in `agent.freeSockets` were never visited at all. Walk both maps the way `Agent.prototype.destroy()` does. Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com> PR-URL: #65066 Reviewed-By: Tim Perry <pimterry@gmail.com>

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

pimterry approved these changes Aug 6, 2026

View reviewed changes

shani-singh1 force-pushed the http-agent-keylog-existing-sockets branch from 898e685 to 9058cd0 Compare August 6, 2026 11:18

pimterry approved these changes Aug 6, 2026

View reviewed changes

pimterry added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 6, 2026

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

pimterry added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 6, 2026

pimterry added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 17, 2026

nodejs-github-bot merged commit 45d6d31 into nodejs:main Aug 17, 2026
75 checks passed

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 17, 2026

github-actions Bot mentioned this pull request Aug 25, 2026

2026-08-26, Version 26.8.0 (Current) #65551

Open

http: fix keylog listener setup on existing agent sockets by shani-singh1 · Pull Request #65066 · nodejs/node · GitHub

Uh oh!

Uh oh!

http: fix keylog listener setup on existing agent sockets - #65066

http: fix keylog listener setup on existing agent sockets#65066
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
shani-singh1:http-agent-keylog-existing-sockets

shani-singh1 commented Aug 6, 2026

Uh oh!

nodejs-github-bot commented Aug 6, 2026

Uh oh!

codecov Bot commented Aug 6, 2026 •
edited
Loading

Uh oh!

Uh oh!

pimterry left a comment

Uh oh!

shani-singh1 commented Aug 6, 2026

Uh oh!

nodejs-github-bot commented Aug 6, 2026

Uh oh!

nodejs-github-bot commented Aug 6, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

http: fix keylog listener setup on existing agent sockets by shani-singh1 · Pull Request #65066 · nodejs/node · GitHub

Uh oh!

Uh oh!

http: fix keylog listener setup on existing agent sockets - #65066

http: fix keylog listener setup on existing agent sockets#65066nodejs-github-bot merged 1 commit intonodejs:mainnodejs/node:mainfrom shani-singh1:http-agent-keylog-existing-socketsshani-singh1/node:http-agent-keylog-existing-socketsCopy head branch name to clipboard

Conversation

shani-singh1 commented Aug 6, 2026

Reproduction

Change

Verification

Uh oh!

nodejs-github-bot commented Aug 6, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

pimterry left a comment

Choose a reason for hiding this comment

Uh oh!

shani-singh1 commented Aug 6, 2026

Uh oh!

nodejs-github-bot commented Aug 6, 2026

Uh oh!

nodejs-github-bot commented Aug 6, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

http: fix keylog listener setup on existing agent sockets#65066
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
shani-singh1:http-agent-keylog-existing-sockets

codecov Bot commented Aug 6, 2026 •
edited
Loading