Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete puppeteer tmp dir after browser closes #1367

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
89 changes: 19 additions & 70 deletions packages/renderer/src/browser/BrowserRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@

import * as childProcess from 'child_process';
import * as fs from 'fs';
import * as path from 'path';
import * as readline from 'readline';
import {promisify} from 'util';
import {deleteDirectory} from '../delete-directory';
import {assert} from './assert';
import {Connection} from './Connection';
import {TimeoutError} from './Errors';
import type {LaunchOptions} from './LaunchOptions';
import {NodeWebSocketTransport} from './NodeWebSocketTransport';
import type {Product} from './Product';
import type {PuppeteerEventListener} from './util';
import {
addEventListener,
Expand All @@ -34,20 +31,15 @@ import {
removeEventListeners,
} from './util';

const renameAsync = promisify(fs.rename);
const unlinkAsync = promisify(fs.unlink);

const PROCESS_ERROR_EXPLANATION = `Puppeteer was unable to kill the process which ran the browser binary.
This means that, on future Puppeteer launches, Puppeteer might not be able to launch the browser.
Please check your open processes and ensure that the browser processes that Puppeteer launched have been killed.
If you think this is a bug, please report it on the Puppeteer issue tracker.`;

export class BrowserRunner {
#product: Product;
#executablePath: string;
#processArguments: string[];
#userDataDir: string;
#isTempUserDataDir?: boolean;
#closed = true;
#listeners: PuppeteerEventListener[] = [];
#processClosing!: Promise<void>;
Expand All @@ -56,28 +48,21 @@ export class BrowserRunner {
connection?: Connection;

constructor({
product,
executablePath,
processArguments,
userDataDir,
isTempUserDataDir,
}: {
product: Product;
executablePath: string;
processArguments: string[];
userDataDir: string;
isTempUserDataDir?: boolean;
}) {
this.#product = product;
this.#executablePath = executablePath;
this.#processArguments = processArguments;
this.#userDataDir = userDataDir;
this.#isTempUserDataDir = isTempUserDataDir;
}

start(options: LaunchOptions): void {
const {handleSIGINT, handleSIGTERM, handleSIGHUP, dumpio, env, pipe} =
options;
const {dumpio, env, pipe} = options;
let stdio: Array<'ignore' | 'pipe'>;
if (pipe) {
if (dumpio) {
Expand Down Expand Up @@ -115,74 +100,40 @@ export class BrowserRunner {
(this.proc as childProcess.ChildProcess).once('exit', async () => {
this.#closed = true;
// Cleanup as processes exit.
if (this.#isTempUserDataDir) {
try {
try {
if (fs.existsSync(this.#userDataDir)) {
await deleteDirectory(this.#userDataDir);
fulfill();
} catch (error) {
reject(error);
}
} else {
if (this.#product === 'firefox') {
try {
// When an existing user profile has been used remove the user
// preferences file and restore possibly backuped preferences.
await unlinkAsync(path.join(this.#userDataDir, 'user.js'));

const prefsBackupPath = path.join(
this.#userDataDir,
'prefs.js.puppeteer'
);
if (fs.existsSync(prefsBackupPath)) {
const prefsPath = path.join(this.#userDataDir, 'prefs.js');
await unlinkAsync(prefsPath);
await renameAsync(prefsBackupPath, prefsPath);
}
} catch (error) {
reject(error);
}
}

fulfill();
} catch (error) {
reject(error);
}
});
});
this.#listeners = [addEventListener(process, 'exit', this.kill.bind(this))];
if (handleSIGINT) {
this.#listeners.push(
addEventListener(process, 'SIGINT', () => {
this.kill();
process.exit(130);
})
);
}
this.#listeners.push(
addEventListener(process, 'SIGINT', () => {
this.kill();
process.exit(130);
})
);

if (handleSIGTERM) {
this.#listeners.push(
addEventListener(process, 'SIGTERM', this.close.bind(this))
);
}
this.#listeners.push(
addEventListener(process, 'SIGTERM', this.close.bind(this))
);

if (handleSIGHUP) {
this.#listeners.push(
addEventListener(process, 'SIGHUP', this.close.bind(this))
);
}
this.#listeners.push(
addEventListener(process, 'SIGHUP', this.close.bind(this))
);
}

close(): Promise<void> {
if (this.#closed) {
return Promise.resolve();
}

if (this.#isTempUserDataDir) {
this.kill();
} else if (this.connection) {
// Attempt to close the browser gracefully
this.connection.send('Browser.close').catch(() => {
this.kill();
});
}
this.kill();

// Cleanup this listener last, as that makes sure the full callback runs. If we
// perform this earlier, then the previous function calls would not happen.
Expand Down Expand Up @@ -231,9 +182,7 @@ export class BrowserRunner {

// Attempt to remove temporary profile directory to avoid littering.
try {
if (this.#isTempUserDataDir) {
fs.rmSync(this.#userDataDir, {recursive: true, force: true});
}
fs.rmSync(this.#userDataDir, {recursive: true, force: true});
} catch (error) {}

// Cleanup this listener last, as that makes sure the full callback runs. If we
Expand Down
4 changes: 0 additions & 4 deletions packages/renderer/src/browser/LaunchOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,11 @@ export interface BrowserLaunchArgumentOptions {

export interface LaunchOptions {
executablePath?: string;
handleSIGINT?: boolean;
handleSIGTERM?: boolean;
handleSIGHUP?: boolean;
timeout?: number;
dumpio?: boolean;
env?: Record<string, string | undefined>;
pipe?: boolean;
product?: Product;
extraPrefsFirefox?: Record<string, unknown>;
}

export type PuppeteerNodeLaunchOptions = BrowserLaunchArgumentOptions &
Expand Down