chore: createHttpServer that destroys sockets upon close (#23294)

This avoids the server hanging on close.
This commit is contained in:
Dmitry Gozman 2023-05-26 07:03:41 -07:00 committed by GitHub
parent 0ef8832f56
commit fa86f2aee0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 56 additions and 31 deletions

View File

@ -16,7 +16,7 @@
import { wsServer } from '../utilsBundle';
import type { WebSocketServer } from '../utilsBundle';
import http from 'http';
import type http from 'http';
import type { Browser } from '../server/browser';
import type { Playwright } from '../server/playwright';
import { createPlaywright } from '../server/playwright';
@ -27,6 +27,7 @@ import { ManualPromise } from '../utils/manualPromise';
import type { AndroidDevice } from '../server/android/android';
import { type SocksProxy } from '../common/socksProxy';
import { debugLogger } from '../common/debugLogger';
import { createHttpServer } from '../utils';
let lastConnectionId = 0;
const kConnectionSymbol = Symbol('kConnection');
@ -56,7 +57,7 @@ export class PlaywrightServer {
async listen(port: number = 0): Promise<string> {
debugLogger.log('server', `Server started at ${new Date()}`);
const server = http.createServer((request: http.IncomingMessage, response: http.ServerResponse) => {
const server = createHttpServer((request: http.IncomingMessage, response: http.ServerResponse) => {
if (request.method === 'GET' && request.url === '/json') {
response.setHeader('Content-Type', 'application/json');
response.end(JSON.stringify({

View File

@ -14,12 +14,13 @@
* limitations under the License.
*/
import * as http from 'http';
import type http from 'http';
import fs from 'fs';
import path from 'path';
import { mime, wsServer } from '../utilsBundle';
import type { WebSocketServer } from '../utilsBundle';
import { assert } from './';
import { assert } from './debug';
import { createHttpServer } from './network';
import { ManualPromise } from './manualPromise';
export type ServerRouteHandler = (request: http.IncomingMessage, response: http.ServerResponse) => boolean;
@ -30,10 +31,10 @@ export class HttpServer {
private _port: number = 0;
private _started = false;
private _routes: { prefix?: string, exact?: string, handler: ServerRouteHandler }[] = [];
private _activeSockets = new Set<import('net').Socket>();
constructor(address: string = '') {
this._urlPrefix = address;
this._server = http.createServer(this._onRequest.bind(this));
this._server = createHttpServer(this._onRequest.bind(this));
}
createWebSocketServer(): WebSocketServer {
@ -71,10 +72,6 @@ export class HttpServer {
async start(options: { port?: number, preferredPort?: number, host?: string } = {}): Promise<string> {
assert(!this._started, 'server already started');
this._started = true;
this._server.on('connection', socket => {
this._activeSockets.add(socket);
socket.once('close', () => this._activeSockets.delete(socket));
});
const host = options.host || 'localhost';
if (options.preferredPort) {
@ -103,8 +100,6 @@ export class HttpServer {
}
async stop() {
for (const socket of this._activeSockets)
socket.destroy();
await new Promise(cb => this._server!.close(cb));
}

View File

@ -17,6 +17,7 @@
import http from 'http';
import https from 'https';
import type net from 'net';
import { getProxyForUrl } from '../utilsBundle';
import { HttpsProxyAgent } from '../utilsBundle';
import * as URL from 'url';
@ -151,3 +152,35 @@ export function constructURLBasedOnBaseURL(baseURL: string | undefined, givenURL
return givenURL;
}
}
export function createHttpServer(requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): http.Server;
export function createHttpServer(options: http.ServerOptions, requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): http.Server;
export function createHttpServer(...args: any[]): http.Server {
const server = http.createServer(...args);
decorateServer(server);
return server;
}
export function createHttpsServer(requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): https.Server;
export function createHttpsServer(options: https.ServerOptions, requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): https.Server;
export function createHttpsServer(...args: any[]): https.Server {
const server = https.createServer(...args);
decorateServer(server);
return server;
}
function decorateServer(server: http.Server | http.Server) {
const sockets = new Set<net.Socket>();
server.on('connection', socket => {
sockets.add(socket);
socket.once('close', () => sockets.delete(socket));
});
const close = server.close;
server.close = (callback?: (err?: Error) => void) => {
for (const socket of sockets)
socket.destroy();
sockets.clear();
return close.call(server, callback);
};
}

View File

@ -16,8 +16,7 @@
*/
import fs from 'fs';
import http from 'http';
import https from 'https';
import type http from 'http';
import mime from 'mime';
import type net from 'net';
import path from 'path';
@ -25,6 +24,7 @@ import url from 'url';
import util from 'util';
import ws from 'ws';
import zlib, { gzip } from 'zlib';
import { createHttpServer, createHttpsServer } from '../../../packages/playwright-core/lib/utils/network';
const fulfillSymbol = Symbol('fullfil callback');
const rejectSymbol = Symbol('reject callback');
@ -38,7 +38,6 @@ export class TestServer {
readonly debugServer: any;
private _startTime: Date;
private _cachedPathPrefix: string | null;
private _sockets = new Set<net.Socket>();
private _routes = new Map<string, (arg0: http.IncomingMessage, arg1: http.ServerResponse) => any>();
private _auths = new Map<string, { username: string; password: string; }>();
private _csp = new Map<string, string>();
@ -68,9 +67,9 @@ export class TestServer {
constructor(dirPath: string, port: number, loopback?: string, sslOptions?: object) {
if (sslOptions)
this._server = https.createServer(sslOptions, this._onRequest.bind(this));
this._server = createHttpsServer(sslOptions, this._onRequest.bind(this));
else
this._server = http.createServer(this._onRequest.bind(this));
this._server = createHttpServer(this._onRequest.bind(this));
this._server.on('connection', socket => this._onSocket(socket));
this._wsServer = new ws.WebSocketServer({ noServer: true });
this._server.on('upgrade', async (request, socket, head) => {
@ -109,14 +108,12 @@ export class TestServer {
}
_onSocket(socket: net.Socket) {
this._sockets.add(socket);
// ECONNRESET and HPE_INVALID_EOF_STATE are legit errors given
// that tab closing aborts outgoing connections to the server.
socket.on('error', error => {
if ((error as any).code !== 'ECONNRESET' && (error as any).code !== 'HPE_INVALID_EOF_STATE')
throw error;
});
socket.once('close', () => this._sockets.delete(socket));
}
enableHTTPCache(pathPrefix: string) {
@ -142,9 +139,6 @@ export class TestServer {
async stop() {
this.reset();
for (const socket of this._sockets)
socket.destroy();
this._sockets.clear();
await new Promise(x => this._server.close(x));
}

View File

@ -17,7 +17,7 @@
import fs from 'fs';
import os from 'os';
import http from 'http';
import type http from 'http';
import type net from 'net';
import * as path from 'path';
import { getUserAgent } from '../../packages/playwright-core/lib/utils/userAgent';
@ -26,6 +26,7 @@ import { expect, playwrightTest } from '../config/browserTest';
import { parseTrace, suppressCertificateWarning } from '../config/utils';
import formidable from 'formidable';
import type { Browser, ConnectOptions } from 'playwright-core';
import { createHttpServer } from '../../packages/playwright-core/lib/utils/network';
type ExtraFixtures = {
connect: (wsEndpoint: string, options?: ConnectOptions, redirectPortForTest?: number) => Promise<Browser>,
@ -48,7 +49,7 @@ const test = playwrightTest.extend<ExtraFixtures>({
},
dummyServerPort: async ({}, use) => {
const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
const server = createHttpServer((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end('<html><body>from-dummy-server</body></html>');
});
await new Promise<void>(resolve => server.listen(0, resolve));
@ -58,7 +59,7 @@ const test = playwrightTest.extend<ExtraFixtures>({
ipV6ServerPort: async ({}, use) => {
test.skip(!!process.env.INSIDE_DOCKER, 'docker does not support IPv6 by default');
const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
const server = createHttpServer((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end('<html><body>from-ipv6-server</body></html>');
});
await new Promise<void>(resolve => server.listen(0, '::1', resolve));

View File

@ -14,9 +14,10 @@
* limitations under the License.
*/
import http from 'http';
import type http from 'http';
import path from 'path';
import { test, expect } from './playwright-test-fixtures';
import { createHttpServer } from '../../packages/playwright-core/lib/utils/network';
const SIMPLE_SERVER_PATH = path.join(__dirname, 'assets', 'simple-server.js');
@ -251,7 +252,7 @@ test('should time out waiting for a server with url', async ({ runInlineTest },
test('should be able to specify the baseURL without the server', async ({ runInlineTest }, { workerIndex }) => {
const port = workerIndex * 2 + 10500;
const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
const server = createHttpServer((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end('<html><body>hello</body></html>');
});
await new Promise<void>(resolve => server.listen(port, resolve));
@ -281,7 +282,7 @@ test('should be able to specify the baseURL without the server', async ({ runInl
test('should be able to specify a custom baseURL with the server', async ({ runInlineTest }, { workerIndex }) => {
const customWebServerPort = workerIndex * 2 + 10500;
const webServerPort = customWebServerPort + 1;
const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
const server = createHttpServer((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end('<html><body>hello</body></html>');
});
await new Promise<void>(resolve => server.listen(customWebServerPort, resolve));
@ -314,7 +315,7 @@ test('should be able to specify a custom baseURL with the server', async ({ runI
test('should be able to use an existing server when reuseExistingServer:true', async ({ runInlineTest }, { workerIndex }) => {
const port = workerIndex * 2 + 10500;
const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
const server = createHttpServer((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end('<html><body>hello</body></html>');
});
await new Promise<void>(resolve => server.listen(port, resolve));
@ -347,7 +348,7 @@ test('should be able to use an existing server when reuseExistingServer:true', a
test('should throw when a server is already running on the given port and strict is true', async ({ runInlineTest }, { workerIndex }) => {
const port = workerIndex * 2 + 10500;
const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
const server = createHttpServer((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end('<html><body>hello</body></html>');
});
await new Promise<void>(resolve => server.listen(port, resolve));
@ -379,7 +380,7 @@ test('should throw when a server is already running on the given port and strict
for (const host of ['localhost', '127.0.0.1', '0.0.0.0']) {
test(`should detect the server if a web-server is already running on ${host}`, async ({ runInlineTest }, { workerIndex }) => {
const port = workerIndex * 2 + 10500;
const server = http.createServer((req: http.IncomingMessage, res: http.ServerResponse) => {
const server = createHttpServer((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end('<html><body>hello</body></html>');
});
await new Promise<void>(resolve => server.listen(port, host, resolve));