Skip to content

Commit

Permalink
cherry-pick(#32956): fix(fetch): listener leaks on Socket
Browse files Browse the repository at this point in the history
Closes #32951

`node:http` reuses TCP Sockets under the hood. We weren't cleaning up
our listeners, leading to the `MaxListenersExceededWarning`.

This PR adds cleanup logic. It also raises the warning threshhold, so
that it doesn't trigger until there's 100 concurrent requests over the
same socket.
  • Loading branch information
Skn0tt committed Oct 8, 2024
1 parent 0c17732 commit 97aaa12
Showing 1 changed file with 29 additions and 23 deletions.
52 changes: 29 additions & 23 deletions packages/playwright-core/src/server/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import zlib from 'zlib';
import type { HTTPCredentials } from '../../types/types';
import { TimeoutSettings } from '../common/timeoutSettings';
import { getUserAgent } from '../utils/userAgent';
import { assert, createGuid, monotonicTime } from '../utils';
import { assert, createGuid, eventsHelper, monotonicTime, type RegisteredListener } from '../utils';
import { HttpsProxyAgent, SocksProxyAgent } from '../utilsBundle';
import { BrowserContext, verifyClientCertificates } from './browserContext';
import { CookieStore, domainMatches, parseRawCookie } from './cookieStore';
Expand Down Expand Up @@ -312,8 +312,11 @@ export abstract class APIRequestContext extends SdkObject {

let securityDetails: har.SecurityDetails | undefined;

const listeners: RegisteredListener[] = [];

const request = requestConstructor(url, requestOptions as any, async response => {
const responseAt = monotonicTime();

const notifyRequestFinished = (body?: Buffer) => {
const endAt = monotonicTime();
// spec: http://www.softwareishard.com/blog/har-12-spec/#timings
Expand Down Expand Up @@ -478,12 +481,13 @@ export abstract class APIRequestContext extends SdkObject {
});
request.on('error', reject);

const disposeListener = () => {
reject(new Error('Request context disposed.'));
request.destroy();
};
this.on(APIRequestContext.Events.Dispose, disposeListener);
request.on('close', () => this.off(APIRequestContext.Events.Dispose, disposeListener));
listeners.push(
eventsHelper.addEventListener(this, APIRequestContext.Events.Dispose, () => {
reject(new Error('Request context disposed.'));
request.destroy();
})
);
request.on('close', () => eventsHelper.removeEventListeners(listeners));

request.on('socket', socket => {
// happy eyeballs don't emit lookup and connect events, so we use our custom ones
Expand All @@ -492,22 +496,24 @@ export abstract class APIRequestContext extends SdkObject {
tcpConnectionAt = happyEyeBallsTimings.tcpConnectionAt;

// non-happy-eyeballs sockets
socket.on('lookup', () => { dnsLookupAt = monotonicTime(); });
socket.on('connect', () => { tcpConnectionAt = monotonicTime(); });
socket.on('secureConnect', () => {
tlsHandshakeAt = monotonicTime();

if (socket instanceof TLSSocket) {
const peerCertificate = socket.getPeerCertificate();
securityDetails = {
protocol: socket.getProtocol() ?? undefined,
subjectName: peerCertificate.subject.CN,
validFrom: new Date(peerCertificate.valid_from).getTime() / 1000,
validTo: new Date(peerCertificate.valid_to).getTime() / 1000,
issuer: peerCertificate.issuer.CN
};
}
});
listeners.push(
eventsHelper.addEventListener(socket, 'lookup', () => { dnsLookupAt = monotonicTime(); }),
eventsHelper.addEventListener(socket, 'connect', () => { tcpConnectionAt = monotonicTime(); }),
eventsHelper.addEventListener(socket, 'secureConnect', () => {
tlsHandshakeAt = monotonicTime();

if (socket instanceof TLSSocket) {
const peerCertificate = socket.getPeerCertificate();
securityDetails = {
protocol: socket.getProtocol() ?? undefined,
subjectName: peerCertificate.subject.CN,
validFrom: new Date(peerCertificate.valid_from).getTime() / 1000,
validTo: new Date(peerCertificate.valid_to).getTime() / 1000,
issuer: peerCertificate.issuer.CN
};
}
}),
);

serverIPAddress = socket.remoteAddress;
serverPort = socket.remotePort;
Expand Down

0 comments on commit 97aaa12

Please sign in to comment.