thunderstrike/puppeteer
thunderstrike/puppeteer
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(Launcher): consume protocol errors when initiating browser.close() (#2332)

Browse Source 
Handle errors properly while sending Browser.close()

Fixes #1429.
This commit is contained in:
Andrey Lushnikov 2018-04-09 14:49:02 -07:00 committed by GitHub
parent a052b9e774
commit c86c12e605
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
lib/Launcher.js
View File

@ -22,7 +22,7 @@ const {Connection} = require('./Connection');
const Browser = require('./Browser'); const Browser = require('./Browser');
const readline = require('readline'); const readline = require('readline');
const fs = require('fs'); const fs = require('fs');
const {helper} = require('./helper'); const {helper, debugError} = require('./helper');
const ChromiumRevision = require(path.join(helper.projectRoot(), 'package.json')).puppeteer.chromium_revision; const ChromiumRevision = require(path.join(helper.projectRoot(), 'package.json')).puppeteer.chromium_revision;
const mkdtempAsync = helper.promisify(fs.mkdtemp); const mkdtempAsync = helper.promisify(fs.mkdtemp);
@ -140,13 +140,13 @@ class Launcher {
}); });
}); });
const listeners = [ helper.addEventListener(process, 'exit', forceKillChrome) ]; const listeners = [ helper.addEventListener(process, 'exit', killChrome) ];
if (options.handleSIGINT !== false) if (options.handleSIGINT !== false)
listeners.push(helper.addEventListener(process, 'SIGINT', forceKillChrome)); listeners.push(helper.addEventListener(process, 'SIGINT', killChrome));
if (options.handleSIGTERM !== false) if (options.handleSIGTERM !== false)
listeners.push(helper.addEventListener(process, 'SIGTERM', killChrome)); listeners.push(helper.addEventListener(process, 'SIGTERM', gracefullyCloseChrome));
if (options.handleSIGHUP !== false) if (options.handleSIGHUP !== false)
listeners.push(helper.addEventListener(process, 'SIGHUP', killChrome)); listeners.push(helper.addEventListener(process, 'SIGHUP', gracefullyCloseChrome));
/** @type {?Connection} */ /** @type {?Connection} */
let connection = null; let connection = null;
try { try {
@ -158,27 +158,31 @@ class Launcher {
} else { } else {
connection = Connection.createForPipe(/** @type {!NodeJS.WritableStream} */(chromeProcess.stdio[3]), /** @type {!NodeJS.ReadableStream} */ (chromeProcess.stdio[4]), connectionDelay); connection = Connection.createForPipe(/** @type {!NodeJS.WritableStream} */(chromeProcess.stdio[3]), /** @type {!NodeJS.ReadableStream} */ (chromeProcess.stdio[4]), connectionDelay);
} }
return Browser.create(connection, options, chromeProcess, killChrome); return Browser.create(connection, options, chromeProcess, gracefullyCloseChrome);
} catch (e) { } catch (e) {
forceKillChrome(); killChrome();
throw e; throw e;
} }
/** /**
* @return {Promise} * @return {Promise}
*/ */
function killChrome() { function gracefullyCloseChrome() {
helper.removeEventListeners(listeners); helper.removeEventListeners(listeners);
if (temporaryUserDataDir) { if (temporaryUserDataDir) {
forceKillChrome(); killChrome();
} else if (connection) { } else if (connection) {
// Attempt to close chrome gracefully // Attempt to close chrome gracefully
connection.send('Browser.close'); connection.send('Browser.close').catch(error => {
debugError(error);
killChrome();
});
} }
return waitForChromeToClose; return waitForChromeToClose;
} }
function forceKillChrome() { // This method has to be sync to be used as 'exit' event handler.
function killChrome() {
helper.removeEventListeners(listeners); helper.removeEventListeners(listeners);
if (chromeProcess.pid && !chromeProcess.killed && !chromeClosed) { if (chromeProcess.pid && !chromeProcess.killed && !chromeClosed) {
// Force kill chrome. // Force kill chrome.
@ -221,7 +225,7 @@ class Launcher {
static async connect(options = {}) { static async connect(options = {}) {
const connectionDelay = options.slowMo || 0; const connectionDelay = options.slowMo || 0;
const connection = await Connection.createForWebSocket(options.browserWSEndpoint, connectionDelay); const connection = await Connection.createForWebSocket(options.browserWSEndpoint, connectionDelay);
return Browser.create(connection, options, null, () => connection.send('Browser.close')); return Browser.create(connection, options, null, () => connection.send('Browser.close').catch(debugError));
} }
} }

17
test/browser.spec.js
View File

@ -14,6 +14,8 @@
* limitations under the License. * limitations under the License.
*/ */
const utils = require('./utils.js');
module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, puppeteer}) { module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, puppeteer}) {
const {describe, xdescribe, fdescribe} = testRunner; const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit} = testRunner; const {it, fit, xit} = testRunner;
@ -61,15 +63,24 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p
remoteBrowser1.on('disconnected', () => ++disconnectedRemote1); remoteBrowser1.on('disconnected', () => ++disconnectedRemote1);
remoteBrowser2.on('disconnected', () => ++disconnectedRemote2); remoteBrowser2.on('disconnected', () => ++disconnectedRemote2);
await remoteBrowser2.disconnect(); await Promise.all([
utils.waitForEvents(remoteBrowser2, 'disconnected'),
remoteBrowser2.disconnect(),
]);
expect(disconnectedOriginal).toBe(0); expect(disconnectedOriginal).toBe(0);
expect(disconnectedRemote1).toBe(0); expect(disconnectedRemote1).toBe(0);
expect(disconnectedRemote2).toBe(1); expect(disconnectedRemote2).toBe(1);
await originalBrowser.close(); await Promise.all([
utils.waitForEvents(remoteBrowser1, 'disconnected'),
utils.waitForEvents(originalBrowser, 'disconnected'),
originalBrowser.close(),
]);
expect(disconnectedOriginal).toBe(1); expect(disconnectedOriginal).toBe(1);
expect(disconnectedRemote1).toBe(1); expect(disconnectedRemote1).toBe(1);
expect(disconnectedRemote2).toBe(1); expect(disconnectedRemote2).toBe(1);
}); });
}); });
}; };