chore: replace process.exit with graceful closure (#24242)

Everywhere we call `process.exit()`, we might actually need to
gracefully close all browsers.
This commit is contained in:
Dmitry Gozman 2023-07-24 08:29:29 -07:00 committed by GitHub
parent b93b2a7155
commit b4c412eb1f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 78 additions and 84 deletions

View File

@ -11,6 +11,11 @@ module.exports = {
*/
"rules": {
"no-console": 2,
"no-debugger": 2
"no-debugger": 2,
"no-restricted-properties": [2, {
"object": "process",
"property": "exit",
"message": "Please use gracefullyProcessExitDoNotHang function to exit the process.",
}],
}
};

View File

@ -18,7 +18,7 @@
/* eslint-disable no-console */
import { getPackageManager } from '../utils';
import { getPackageManager, gracefullyProcessExitDoNotHang } from '../utils';
import program from './program';
function printPlaywrightTestError(command: string) {
@ -53,7 +53,7 @@ function printPlaywrightTestError(command: string) {
command.description('Run tests with Playwright Test. Available in @playwright/test package.');
command.action(async () => {
printPlaywrightTestError('test');
process.exit(1);
gracefullyProcessExitDoNotHang(1);
});
}
@ -62,7 +62,7 @@ function printPlaywrightTestError(command: string) {
command.description('Show Playwright Test HTML report. Available in @playwright/test package.');
command.action(async () => {
printPlaywrightTestError('show-report');
process.exit(1);
gracefullyProcessExitDoNotHang(1);
});
}

View File

@ -23,7 +23,7 @@ import type { LaunchServerOptions } from '../client/types';
import { createPlaywright, DispatcherConnection, RootDispatcher, PlaywrightDispatcher } from '../server';
import { PipeTransport } from '../protocol/transport';
import { PlaywrightServer } from '../remote/playwrightServer';
import { gracefullyCloseAll } from '../utils/processLauncher';
import { gracefullyProcessExitDoNotHang } from '../utils/processLauncher';
export function printApiJson() {
// Note: this file is generated by build-playwright-driver.sh
@ -42,7 +42,7 @@ export function runDriver() {
transport.onclose = () => {
// Drop any messages during shutdown on the floor.
dispatcherConnection.onmessage = () => {};
selfDestruct();
gracefullyProcessExitDoNotHang(0);
};
// Ignore the SIGINT signal in the driver process so the parent can gracefully close the connection.
// We still will destruct everything (close browsers and exit) when the transport pipe closes.
@ -71,7 +71,7 @@ export async function runServer(options: RunServerOptions) {
const wsEndpoint = await server.listen(port);
process.on('exit', () => server.close().catch(console.error));
console.log('Listening on ' + wsEndpoint); // eslint-disable-line no-console
process.stdin.on('close', () => selfDestruct());
process.stdin.on('close', () => gracefullyProcessExitDoNotHang(0));
}
export async function launchBrowserServer(browserName: string, configFile?: string) {
@ -82,12 +82,3 @@ export async function launchBrowserServer(browserName: string, configFile?: stri
const server = await browserType.launchServer(options);
console.log(server.wsEndpoint());
}
function selfDestruct() {
// Force exit after 30 seconds.
setTimeout(() => process.exit(0), 30000);
// Meanwhile, try to gracefully close all browsers.
gracefullyCloseAll().then(() => {
process.exit(0);
});
}

View File

@ -31,7 +31,7 @@ import type { Page } from '../client/page';
import type { BrowserType } from '../client/browserType';
import type { BrowserContextOptions, LaunchOptions } from '../client/types';
import { spawn } from 'child_process';
import { wrapInASCIIBox, isLikelyNpxGlobal, assert } from '../utils';
import { wrapInASCIIBox, isLikelyNpxGlobal, assert, gracefullyProcessExitDoNotHang } from '../utils';
import type { Executable } from '../server';
import { registry, writeDockerVersion } from '../server';
@ -104,10 +104,8 @@ function checkBrowsersToInstall(args: string[]): Executable[] {
else
executables.push(executable);
}
if (faultyArguments.length) {
console.log(`Invalid installation targets: ${faultyArguments.map(name => `'${name}'`).join(', ')}. Expecting one of: ${suggestedBrowsersToInstall()}`);
process.exit(1);
}
if (faultyArguments.length)
throw new Error(`Invalid installation targets: ${faultyArguments.map(name => `'${name}'`).join(', ')}. Expecting one of: ${suggestedBrowsersToInstall()}`);
return executables;
}
@ -163,7 +161,7 @@ program
}
} catch (e) {
console.log(`Failed to install browsers\n${e}`);
process.exit(1);
gracefullyProcessExitDoNotHang(1);
}
}).addHelpText('afterAll', `
@ -199,7 +197,7 @@ program
await registry.installDeps(checkBrowsersToInstall(args), !!options.dryRun);
} catch (e) {
console.log(`Failed to install browser dependencies\n${e}`);
process.exit(1);
gracefullyProcessExitDoNotHang(1);
}
}).addHelpText('afterAll', `
Examples:
@ -308,7 +306,7 @@ program
openTraceInBrowser(traces, openOptions).catch(logErrorAndExit);
} else {
openTraceViewerApp(traces, options.browser, openOptions).then(page => {
page.on('close', () => process.exit(0));
page.on('close', () => gracefullyProcessExitDoNotHang(0));
}).catch(logErrorAndExit);
}
}).addHelpText('afterAll', `
@ -406,7 +404,7 @@ async function launchContext(options: Options, headless: boolean, executablePath
const hasCrashLine = logs.some(line => line.includes('process did exit:') && !line.includes('process did exit: exitCode=0, signal=null'));
if (hasCrashLine) {
process.stderr.write('Detected browser crash.\n');
process.exit(1);
gracefullyProcessExitDoNotHang(1);
}
});
}
@ -417,8 +415,7 @@ async function launchContext(options: Options, headless: boolean, executablePath
const [width, height] = options.viewportSize.split(',').map(n => parseInt(n, 10));
contextOptions.viewport = { width, height };
} catch (e) {
console.log('Invalid window size format: use "width, height", for example --window-size=800,600');
process.exit(0);
throw new Error('Invalid viewport size format: use "width, height", for example --viewport-size=800,600');
}
}
@ -432,8 +429,7 @@ async function launchContext(options: Options, headless: boolean, executablePath
longitude
};
} catch (e) {
console.log('Invalid geolocation format: user lat, long, for example --geolocation="37.819722,-122.478611"');
process.exit(0);
throw new Error('Invalid geolocation format, should be "lat,long". For example --geolocation="37.819722,-122.478611"');
}
contextOptions.permissions = ['geolocation'];
}
@ -507,7 +503,7 @@ async function launchContext(options: Options, headless: boolean, executablePath
});
process.on('SIGINT', async () => {
await closeBrowser();
process.exit(130);
gracefullyProcessExitDoNotHang(130);
});
const timeout = options.timeout ? parseInt(options.timeout, 10) : 0;
@ -596,10 +592,8 @@ async function screenshot(options: Options, captureOptions: CaptureOptions, url:
}
async function pdf(options: Options, captureOptions: CaptureOptions, url: string, path: string) {
if (options.browser !== 'chromium') {
console.error('PDF creation is only working with Chromium');
process.exit(1);
}
if (options.browser !== 'chromium')
throw new Error('PDF creation is only working with Chromium');
const { context } = await launchContext({ ...options, browser: 'chromium' }, true);
console.log('Navigating to ' + url);
const page = await openPage(context, url);
@ -632,20 +626,21 @@ function lookupBrowserType(options: Options): BrowserType {
function validateOptions(options: Options) {
if (options.device && !(options.device in playwright.devices)) {
console.log(`Device descriptor not found: '${options.device}', available devices are:`);
const lines = [`Device descriptor not found: '${options.device}', available devices are:`];
for (const name in playwright.devices)
console.log(` "${name}"`);
process.exit(0);
}
if (options.colorScheme && !['light', 'dark'].includes(options.colorScheme)) {
console.log('Invalid color scheme, should be one of "light", "dark"');
process.exit(0);
lines.push(` "${name}"`);
throw new Error(lines.join('\n'));
}
if (options.colorScheme && !['light', 'dark'].includes(options.colorScheme))
throw new Error('Invalid color scheme, should be one of "light", "dark"');
}
function logErrorAndExit(e: Error) {
console.error(e);
process.exit(1);
if (process.env.PWDEBUGIMPL)
console.error(e);
else
console.error(e.name + ': ' + e.message);
gracefullyProcessExitDoNotHang(1);
}
function codegenId(): string {

View File

@ -15,7 +15,7 @@
*/
import type { Mode, Source } from '@recorder/recorderTypes';
import { gracefullyCloseAll } from '../utils/processLauncher';
import { gracefullyProcessExitDoNotHang } from '../utils/processLauncher';
import type { Browser } from './browser';
import type { BrowserContext } from './browserContext';
import { createInstrumentation, SdkObject, serverSideCallMetadata } from './instrumentation';
@ -138,7 +138,7 @@ export class DebugController extends SdkObject {
return;
const heartBeat = () => {
if (!this._playwright.allPages().length)
selfDestruct();
gracefullyProcessExitDoNotHang(0);
else
this._autoCloseTimer = setTimeout(heartBeat, 5000);
};
@ -168,7 +168,7 @@ export class DebugController extends SdkObject {
}
async kill() {
selfDestruct();
gracefullyProcessExitDoNotHang(0);
}
async closeAllBrowsers() {
@ -218,15 +218,6 @@ export class DebugController extends SdkObject {
}
}
function selfDestruct() {
// Force exit after 30 seconds.
setTimeout(() => process.exit(0), 30000);
// Meanwhile, try to gracefully close all browsers.
gracefullyCloseAll().then(() => {
process.exit(0);
});
}
class InspectingRecorderApp extends EmptyRecorderApp {
private _debugController: DebugController;

View File

@ -166,5 +166,6 @@ async function main() {
main().catch(error => {
// eslint-disable-next-line no-console
console.error(error);
// eslint-disable-next-line no-restricted-properties
process.exit(1);
});

View File

@ -18,7 +18,7 @@ import path from 'path';
import fs from 'fs';
import { HttpServer } from '../../../utils/httpServer';
import { findChromiumChannel } from '../../registry';
import { createGuid, gracefullyCloseAll, isUnderTest } from '../../../utils';
import { createGuid, gracefullyProcessExitDoNotHang, isUnderTest } from '../../../utils';
import { installAppIcon, syncLocalStorageWithSettings } from '../../chromium/crApp';
import { serverSideCallMetadata } from '../../instrumentation';
import { createPlaywright } from '../../playwright';
@ -54,7 +54,7 @@ async function startTraceViewerServer(traceUrls: string[], options?: OpenTraceVi
if (!traceUrl.startsWith('http://') && !traceUrl.startsWith('https://') && !fs.existsSync(traceFile) && !fs.existsSync(traceFile + '.trace')) {
// eslint-disable-next-line no-console
console.error(`Trace file ${traceUrl} does not exist!`);
process.exit(1);
gracefullyProcessExitDoNotHang(1);
}
}
@ -192,7 +192,7 @@ class StdinServer implements Transport {
else
this._loadTrace(url);
});
process.stdin.on('close', () => this._selfDestruct());
process.stdin.on('close', () => gracefullyProcessExitDoNotHang(0));
}
async dispatch(method: string, params: any) {
@ -203,7 +203,7 @@ class StdinServer implements Transport {
}
onclose() {
this._selfDestruct();
gracefullyProcessExitDoNotHang(0);
}
sendEvent?: (method: string, params: any) => void;
@ -221,15 +221,6 @@ class StdinServer implements Transport {
this._pollLoadTrace(url);
}, 500);
}
private _selfDestruct() {
// Force exit after 30 seconds.
setTimeout(() => process.exit(0), 30000);
// Meanwhile, try to gracefully close all browsers.
gracefullyCloseAll().then(() => {
process.exit(0);
});
}
}
function traceDescriptor(traceName: string) {

View File

@ -56,6 +56,17 @@ export async function gracefullyCloseAll() {
await Promise.all(Array.from(gracefullyCloseSet).map(gracefullyClose => gracefullyClose().catch(e => {})));
}
export function gracefullyProcessExitDoNotHang(code: number) {
// Force exit after 30 seconds.
// eslint-disable-next-line no-restricted-properties
setTimeout(() => process.exit(code), 30000);
// Meanwhile, try to gracefully close all browsers.
gracefullyCloseAll().then(() => {
// eslint-disable-next-line no-restricted-properties
process.exit(code);
});
}
function exitHandler() {
for (const kill of killSet)
kill();
@ -65,10 +76,13 @@ let sigintHandlerCalled = false;
function sigintHandler() {
const exitWithCode130 = () => {
// Give tests a chance to see that launched process did exit and dispatch any async calls.
if (isUnderTest())
if (isUnderTest()) {
// eslint-disable-next-line no-restricted-properties
setTimeout(() => process.exit(130), 1000);
else
} else {
// eslint-disable-next-line no-restricted-properties
process.exit(130);
}
};
if (sigintHandlerCalled) {

View File

@ -20,7 +20,7 @@ import type { Command } from 'playwright-core/lib/utilsBundle';
import fs from 'fs';
import path from 'path';
import { Runner } from './runner/runner';
import { stopProfiling, startProfiling } from 'playwright-core/lib/utils';
import { stopProfiling, startProfiling, gracefullyProcessExitDoNotHang } from 'playwright-core/lib/utils';
import { experimentalLoaderOption, fileIsModule, serializeError } from './util';
import { showHTMLReport } from './reporters/html';
import { createMergedReport } from './reporters/merge';
@ -44,7 +44,7 @@ function addTestCommand(program: Command) {
await runTests(args, opts);
} catch (e) {
console.error(e);
process.exit(1);
gracefullyProcessExitDoNotHang(1);
}
});
command.addHelpText('afterAll', `
@ -68,7 +68,7 @@ function addListFilesCommand(program: Command) {
await listTestFiles(opts);
} catch (e) {
console.error(e);
process.exit(1);
gracefullyProcessExitDoNotHang(1);
}
});
}
@ -96,7 +96,7 @@ function addMergeReportsCommand(program: Command) {
await mergeReports(dir, options);
} catch (e) {
console.error(e);
process.exit(1);
gracefullyProcessExitDoNotHang(1);
}
});
command.option('-c, --config <file>', `Configuration file. Can be used to specify additional configuration for the output report.`);
@ -143,9 +143,8 @@ async function runTests(args: string[], opts: { [key: string]: any }) {
else
status = await runner.runAllTests();
await stopProfiling('runner');
if (status === 'interrupted')
process.exit(130);
process.exit(status === 'passed' ? 0 : 1);
const exitCode = status === 'interrupted' ? 130 : (status === 'passed' ? 0 : 1);
gracefullyProcessExitDoNotHang(exitCode);
}
async function listTestFiles(opts: { [key: string]: any }) {
@ -164,13 +163,13 @@ async function listTestFiles(opts: { [key: string]: any }) {
const runner = new Runner(config);
const report = await runner.listTestFiles(opts.project);
stdoutWrite(JSON.stringify(report), () => {
process.exit(0);
gracefullyProcessExitDoNotHang(0);
});
} catch (e) {
const error: TestError = serializeError(e);
error.location = prepareErrorStack(e.stack).location;
stdoutWrite(JSON.stringify({ error }), () => {
process.exit(0);
gracefullyProcessExitDoNotHang(0);
});
}
}
@ -288,7 +287,7 @@ function restartWithExperimentalTsEsm(configFile: string | null): boolean {
innerProcess.on('close', (code: number | null) => {
if (code !== 0 && code !== null)
process.exit(code);
gracefullyProcessExitDoNotHang(code);
});
return true;
}

View File

@ -91,10 +91,12 @@ async function gracefullyCloseAndExit() {
return;
closed = true;
// Force exit after 30 seconds.
// eslint-disable-next-line no-restricted-properties
setTimeout(() => process.exit(0), 30000);
// Meanwhile, try to gracefully shutdown.
await processRunner.gracefullyClose().catch(() => {});
await stopProfiling(processName).catch(() => {});
// eslint-disable-next-line no-restricted-properties
process.exit(0);
}

View File

@ -99,7 +99,9 @@ export class WebServerPlugin implements TestRunnerPlugin {
cwd: this._options.cwd,
stdio: 'stdin',
shell: true,
attemptToGracefullyClose: async () => {},
// Reject to indicate that we cannot close the web server gracefully
// and should fallback to non-graceful shutdown.
attemptToGracefullyClose: () => Promise.reject(),
log: () => {},
onExit: code => processExitedReject(new Error(code ? `Process from config.webServer was not able to start. Exit code: ${code}` : 'Process from config.webServer exited early.')),
tempDirectories: [],

View File

@ -20,7 +20,7 @@ import path from 'path';
import type { TransformCallback } from 'stream';
import { Transform } from 'stream';
import type { FullConfig, Suite } from '../../types/testReporter';
import { HttpServer, assert, calculateSha1, copyFileAndMakeWritable, removeFolders } from 'playwright-core/lib/utils';
import { HttpServer, assert, calculateSha1, copyFileAndMakeWritable, gracefullyProcessExitDoNotHang, removeFolders } from 'playwright-core/lib/utils';
import type { JsonAttachment, JsonReport, JsonSuite, JsonTestCase, JsonTestResult, JsonTestStep } from './raw';
import RawReporter from './raw';
import { stripAnsiEscapes } from './base';
@ -161,7 +161,7 @@ export async function showHTMLReport(reportFolder: string | undefined, host: str
assert(fs.statSync(folder).isDirectory());
} catch (e) {
console.log(colors.red(`No report found at "${folder}"`));
process.exit(1);
gracefullyProcessExitDoNotHang(1);
return;
}
const server = startHtmlReportServer(folder);

View File

@ -23,7 +23,7 @@ import { ConfigLoader } from '../common/configLoader';
import type { Suite, TestCase } from '../common/test';
import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config';
import { FixtureRunner } from './fixtureRunner';
import { ManualPromise, captureLibraryStackTrace } from 'playwright-core/lib/utils';
import { ManualPromise, captureLibraryStackTrace, gracefullyCloseAll } from 'playwright-core/lib/utils';
import { TestInfoImpl } from './testInfo';
import { TimeoutManager, type TimeSlot } from './timeoutManager';
import { ProcessRunner } from '../common/process';
@ -109,6 +109,9 @@ export class WorkerMain extends ProcessRunner {
// We have to load the project to get the right deadline below.
await this._loadIfNeeded();
await this._teardownScopes();
// Close any other browsers launched in this process. This includes anything launched
// manually in the test/hooks and internal browsers like Playwright Inspector.
await gracefullyCloseAll();
} catch (e) {
this._fatalErrors.push(serializeError(e));
}