kyfx
/
playwright
mirror of https://github.com/microsoft/playwright.git
4
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(fetch): listener leaks on Socket (#32956) 

Closes https://github.com/microsoft/playwright/issues/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.
This commit is contained in:
Simon Knott 2024-10-07 18:43:25 +02:00 committed by GitHub
parent d3fbf1aaeb
commit 9a6f03eb87
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 28 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
packages/playwright-core/src/server/fetch.ts
View File

@ -24,7 +24,7 @@ import zlib from 'zlib';
import type { HTTPCredentials } from '../../types/types'; import type { HTTPCredentials } from '../../types/types';
import { TimeoutSettings } from '../common/timeoutSettings'; import { TimeoutSettings } from '../common/timeoutSettings';
import { getUserAgent } from '../utils/userAgent'; import { getUserAgent } from '../utils/userAgent';
import { assert, constructURLBasedOnBaseURL, createGuid, monotonicTime } from '../utils'; import { assert, constructURLBasedOnBaseURL, createGuid, eventsHelper, monotonicTime, type RegisteredListener } from '../utils';
import { HttpsProxyAgent, SocksProxyAgent } from '../utilsBundle'; import { HttpsProxyAgent, SocksProxyAgent } from '../utilsBundle';
import { BrowserContext, verifyClientCertificates } from './browserContext'; import { BrowserContext, verifyClientCertificates } from './browserContext';
import { CookieStore, domainMatches, parseRawCookie } from './cookieStore'; import { CookieStore, domainMatches, parseRawCookie } from './cookieStore';
@ -311,8 +311,11 @@ export abstract class APIRequestContext extends SdkObject {
let securityDetails: har.SecurityDetails | undefined; let securityDetails: har.SecurityDetails | undefined;
const listeners: RegisteredListener[] = [];
const request = requestConstructor(url, requestOptions as any, async response => { const request = requestConstructor(url, requestOptions as any, async response => {
const responseAt = monotonicTime(); const responseAt = monotonicTime();
const notifyRequestFinished = (body?: Buffer) => { const notifyRequestFinished = (body?: Buffer) => {
const endAt = monotonicTime(); const endAt = monotonicTime();
// spec: http://www.softwareishard.com/blog/har-12-spec/#timings // spec: http://www.softwareishard.com/blog/har-12-spec/#timings
@ -477,12 +480,13 @@ export abstract class APIRequestContext extends SdkObject {
}); });
request.on('error', reject); request.on('error', reject);
const disposeListener = () => { listeners.push(
reject(new Error('Request context disposed.')); eventsHelper.addEventListener(this, APIRequestContext.Events.Dispose, () => {
request.destroy(); reject(new Error('Request context disposed.'));
}; request.destroy();
this.on(APIRequestContext.Events.Dispose, disposeListener); })
request.on('close', () => this.off(APIRequestContext.Events.Dispose, disposeListener)); );
request.on('close', () => eventsHelper.removeEventListeners(listeners));
request.on('socket', socket => { request.on('socket', socket => {
// happy eyeballs don't emit lookup and connect events, so we use our custom ones // happy eyeballs don't emit lookup and connect events, so we use our custom ones
@ -491,22 +495,24 @@ export abstract class APIRequestContext extends SdkObject {
tcpConnectionAt ??= happyEyeBallsTimings.tcpConnectionAt; tcpConnectionAt ??= happyEyeBallsTimings.tcpConnectionAt;
// non-happy-eyeballs sockets // non-happy-eyeballs sockets
socket.on('lookup', () => { dnsLookupAt = monotonicTime(); }); listeners.push(
socket.on('connect', () => { tcpConnectionAt ??= monotonicTime(); }); eventsHelper.addEventListener(socket, 'lookup', () => { dnsLookupAt = monotonicTime(); }),
socket.on('secureConnect', () => { eventsHelper.addEventListener(socket, 'connect', () => { tcpConnectionAt ??= monotonicTime(); }),
tlsHandshakeAt = monotonicTime(); eventsHelper.addEventListener(socket, 'secureConnect', () => {
tlsHandshakeAt = monotonicTime();
if (socket instanceof TLSSocket) { if (socket instanceof TLSSocket) {
const peerCertificate = socket.getPeerCertificate(); const peerCertificate = socket.getPeerCertificate();
securityDetails = { securityDetails = {
protocol: socket.getProtocol() ?? undefined, protocol: socket.getProtocol() ?? undefined,
subjectName: peerCertificate.subject.CN, subjectName: peerCertificate.subject.CN,
validFrom: new Date(peerCertificate.valid_from).getTime() / 1000, validFrom: new Date(peerCertificate.valid_from).getTime() / 1000,
validTo: new Date(peerCertificate.valid_to).getTime() / 1000, validTo: new Date(peerCertificate.valid_to).getTime() / 1000,
issuer: peerCertificate.issuer.CN issuer: peerCertificate.issuer.CN
}; };
} }
}); }),
);
// when using socks proxy, having the socket means the connection got established // when using socks proxy, having the socket means the connection got established
if (agent instanceof SocksProxyAgent) if (agent instanceof SocksProxyAgent)