From f1211cbec091ec669de019aeb7fb4f011a81c1d7 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Tue, 4 Apr 2023 13:58:32 +0200 Subject: [PATCH] fix(browsers): various fixes and improvements (#9966) --- packages/browsers/package.json | 2 +- packages/browsers/src/CLI.ts | 14 ++++--- packages/browsers/src/launcher.ts | 62 +++++++++++++++++++++++++++---- packages/browsers/src/main.ts | 2 + 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/packages/browsers/package.json b/packages/browsers/package.json index f1274444e0d..ad3ab613688 100644 --- a/packages/browsers/package.json +++ b/packages/browsers/package.json @@ -20,7 +20,7 @@ }, "wireit": { "build": { - "command": "tsc -b && chmod a+x lib/cjs/main-cli.js lib/esm/main-cli.js", + "command": "tsc -b && chmod a+x lib/cjs/main-cli.js lib/esm/main-cli.js && echo '{\"type\":\"module\"}' > lib/esm/package.json", "files": [ "src/**/*.ts", "tsconfig.json" diff --git a/packages/browsers/src/CLI.ts b/packages/browsers/src/CLI.ts index 1d694073ac5..0fb9b5428ca 100644 --- a/packages/browsers/src/CLI.ts +++ b/packages/browsers/src/CLI.ts @@ -18,8 +18,9 @@ import {stdin as input, stdout as output} from 'process'; import * as readline from 'readline'; import ProgressBar from 'progress'; -import yargs from 'yargs'; +import type * as Yargs from 'yargs'; import {hideBin} from 'yargs/helpers'; +import yargs from 'yargs/yargs'; import { resolveBuildId, @@ -69,7 +70,7 @@ export class CLI { this.#rl = rl; } - #defineBrowserParameter(yargs: yargs.Argv): void { + #defineBrowserParameter(yargs: Yargs.Argv): void { yargs.positional('browser', { description: 'Which browser to install [@]. `latest` will try to find the latest available build. `buildId` is a browser-specific identifier such as a version or a revision.', @@ -83,7 +84,7 @@ export class CLI { }); } - #definePlatformParameter(yargs: yargs.Argv): void { + #definePlatformParameter(yargs: Yargs.Argv): void { yargs.option('platform', { type: 'string', desc: 'Platform that the binary needs to be compatible with.', @@ -92,7 +93,7 @@ export class CLI { }); } - #definePathParameter(yargs: yargs.Argv, required = false): void { + #definePathParameter(yargs: Yargs.Argv, required = false): void { yargs.option('path', { type: 'string', desc: 'Path to the root folder for the browser downloads and installation. The installation folder structure is compatible with the cache structure used by Puppeteer.', @@ -105,7 +106,8 @@ export class CLI { } async run(argv: string[]): Promise { - await yargs(hideBin(argv)) + const yargsInstance = yargs(hideBin(argv)); + await yargsInstance .scriptName('@puppeteer/browsers') .command( 'install ', @@ -254,7 +256,7 @@ export class CLI { ) .demandCommand(1) .help() - .wrap(Math.min(120, yargs.terminalWidth())) + .wrap(Math.min(120, yargsInstance.terminalWidth())) .parse(); } diff --git a/packages/browsers/src/launcher.ts b/packages/browsers/src/launcher.ts index 57cdbc456bf..888a95c4b8e 100644 --- a/packages/browsers/src/launcher.ts +++ b/packages/browsers/src/launcher.ts @@ -122,11 +122,12 @@ type LaunchOptions = { pipe?: boolean; dumpio?: boolean; args?: string[]; - env?: Record; + env?: Record; handleSIGINT?: boolean; handleSIGTERM?: boolean; handleSIGHUP?: boolean; detached?: boolean; + onExit?: () => Promise; }; export function launch(opts: LaunchOptions): Process { @@ -143,6 +144,11 @@ class Process { #args: string[]; #browserProcess: childProcess.ChildProcess; #exited = false; + // The browser process can be closed externally or from the driver process. We + // need to invoke the hooks only once though but we don't know how many times + // we will be invoked. + #hooksRan = false; + #onExitHook = async () => {}; #browserProcessExiting: Promise; constructor(opts: LaunchOptions) { @@ -190,15 +196,36 @@ class Process { if (opts.handleSIGHUP) { process.on('SIGHUP', this.#onDriverProcessSignal); } - this.#browserProcessExiting = new Promise(resolve => { - this.#browserProcess.once('exit', () => { - this.#exited = true; + if (opts.onExit) { + this.#onExitHook = opts.onExit; + } + this.#browserProcessExiting = new Promise((resolve, reject) => { + this.#browserProcess.once('exit', async () => { this.#clearListeners(); + this.#exited = true; + try { + await this.#runHooks(); + } catch (err) { + reject(err); + return; + } resolve(); }); }); } + async #runHooks() { + if (this.#hooksRan) { + return; + } + this.#hooksRan = true; + await this.#onExitHook(); + } + + get nodeProcess(): childProcess.ChildProcess { + return this.#browserProcess; + } + #configureStdio(opts: { pipe: boolean; dumpio: boolean; @@ -236,19 +263,24 @@ class Process { process.exit(130); case 'SIGTERM': case 'SIGHUP': - this.kill(); + this.close(); break; } }; - close(): Promise { + async close(): Promise { + await this.#runHooks(); if (this.#exited) { - return Promise.resolve(); + return this.#browserProcessExiting; } this.kill(); return this.#browserProcessExiting; } + hasClosed(): Promise { + return this.#browserProcessExiting; + } + kill(): void { // If the process failed to launch (for example if the browser executable path // is invalid), then the process does not get a pid assigned. A call to @@ -329,6 +361,9 @@ class Process { error ? ' ' + error.message : '' }`, stderr, + '', + 'TROUBLESHOOTING: https://pptr.dev/troubleshooting', + '', ].join('\n') ) ); @@ -337,7 +372,7 @@ class Process { function onTimeout(): void { cleanup(); reject( - new Error( + new TimeoutError( `Timed out after ${timeout} ms while waiting for the WS endpoint URL to appear in stdout!` ) ); @@ -403,3 +438,14 @@ export function isErrnoException(obj: unknown): obj is NodeJS.ErrnoException { ('errno' in obj || 'code' in obj || 'path' in obj || 'syscall' in obj) ); } + +export class TimeoutError extends Error { + /** + * @internal + */ + constructor(message?: string) { + super(message); + this.name = this.constructor.name; + Error.captureStackTrace(this, this.constructor); + } +} diff --git a/packages/browsers/src/main.ts b/packages/browsers/src/main.ts index 157e7c11a43..0c3deca60be 100644 --- a/packages/browsers/src/main.ts +++ b/packages/browsers/src/main.ts @@ -18,6 +18,7 @@ export { launch, computeExecutablePath, computeSystemExecutablePath, + TimeoutError, CDP_WEBSOCKET_ENDPOINT_REGEX, WEBDRIVER_BIDI_WEBSOCKET_ENDPOINT_REGEX, } from './launcher.js'; @@ -28,6 +29,7 @@ export { Browser, BrowserPlatform, ChromeReleaseChannel, + createProfile, } from './browser-data/browser-data.js'; export {CLI, makeProgressCallback} from './CLI.js'; export {Cache} from './Cache.js';