Skip to content

Commit

Permalink
fix(browsers): various fixes and improvements (#9966)
Browse files Browse the repository at this point in the history
  • Loading branch information
OrKoN committed Apr 4, 2023
1 parent fe934ad commit f1211cb
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 15 deletions.
2 changes: 1 addition & 1 deletion packages/browsers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 8 additions & 6 deletions packages/browsers/src/CLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -69,7 +70,7 @@ export class CLI {
this.#rl = rl;
}

#defineBrowserParameter(yargs: yargs.Argv<unknown>): void {
#defineBrowserParameter(yargs: Yargs.Argv<unknown>): void {
yargs.positional('browser', {
description:
'Which browser to install <browser>[@<buildId|latest>]. `latest` will try to find the latest available build. `buildId` is a browser-specific identifier such as a version or a revision.',
Expand All @@ -83,7 +84,7 @@ export class CLI {
});
}

#definePlatformParameter(yargs: yargs.Argv<unknown>): void {
#definePlatformParameter(yargs: Yargs.Argv<unknown>): void {
yargs.option('platform', {
type: 'string',
desc: 'Platform that the binary needs to be compatible with.',
Expand All @@ -92,7 +93,7 @@ export class CLI {
});
}

#definePathParameter(yargs: yargs.Argv<unknown>, required = false): void {
#definePathParameter(yargs: Yargs.Argv<unknown>, 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.',
Expand All @@ -105,7 +106,8 @@ export class CLI {
}

async run(argv: string[]): Promise<void> {
await yargs(hideBin(argv))
const yargsInstance = yargs(hideBin(argv));
await yargsInstance
.scriptName('@puppeteer/browsers')
.command(
'install <browser>',
Expand Down Expand Up @@ -254,7 +256,7 @@ export class CLI {
)
.demandCommand(1)
.help()
.wrap(Math.min(120, yargs.terminalWidth()))
.wrap(Math.min(120, yargsInstance.terminalWidth()))
.parse();
}

Expand Down
62 changes: 54 additions & 8 deletions packages/browsers/src/launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,12 @@ type LaunchOptions = {
pipe?: boolean;
dumpio?: boolean;
args?: string[];
env?: Record<string, string>;
env?: Record<string, string | undefined>;
handleSIGINT?: boolean;
handleSIGTERM?: boolean;
handleSIGHUP?: boolean;
detached?: boolean;
onExit?: () => Promise<void>;
};

export function launch(opts: LaunchOptions): Process {
Expand All @@ -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<void>;

constructor(opts: LaunchOptions) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -236,19 +263,24 @@ class Process {
process.exit(130);
case 'SIGTERM':
case 'SIGHUP':
this.kill();
this.close();
break;
}
};

close(): Promise<void> {
async close(): Promise<void> {
await this.#runHooks();
if (this.#exited) {
return Promise.resolve();
return this.#browserProcessExiting;
}
this.kill();
return this.#browserProcessExiting;
}

hasClosed(): Promise<void> {
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
Expand Down Expand Up @@ -329,6 +361,9 @@ class Process {
error ? ' ' + error.message : ''
}`,
stderr,
'',
'TROUBLESHOOTING: https://pptr.dev/troubleshooting',
'',
].join('\n')
)
);
Expand All @@ -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!`
)
);
Expand Down Expand Up @@ -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);
}
}
2 changes: 2 additions & 0 deletions packages/browsers/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export {
launch,
computeExecutablePath,
computeSystemExecutablePath,
TimeoutError,
CDP_WEBSOCKET_ENDPOINT_REGEX,
WEBDRIVER_BIDI_WEBSOCKET_ENDPOINT_REGEX,
} from './launcher.js';
Expand All @@ -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';

0 comments on commit f1211cb

Please sign in to comment.