Launcher: add timeout to chrome launching (#434)

This patch:
- adds a 'timeout' launcher option that constrains the time for chromium to launch.
- adds a 'handleSIGINT' launcher option that is `true` by default and that closes chrome instance

Fixes #363.

docs/api.md

@@ -158,6 +158,8 @@ This methods attaches Puppeteer to an existing Chromium instance.
   - `executablePath` <[string]> Path to a Chromium executable to run instead of bundled Chromium. If `executablePath` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
   - `slowMo` <[number]> Slows down Puppeteer operations by the specified amount of milliseconds. Useful so that you can see what is going on.
   - `args` <[Array]<[string]>> Additional arguments to pass to the Chromium instance. List of Chromium flags can be found [here](http://peter.sh/experiments/chromium-command-line-switches/).
+  - `handleSIGINT` <[boolean]> Close chrome process on Ctrl-C. Defaults to `true`.
+  - `timeout` <[number]> Maximum time in milliseconds to wait for the Chrome instance to start. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
   - `dumpio` <[boolean]> Whether to pipe browser process stdout and stderr into `process.stdout` and `process.stderr`. Defaults to `false`.
 - returns: <[Promise]<[Browser]>> Promise which resolves to browser instance.
 

lib/Launcher.js

@@ -14,13 +14,15 @@
  * limitations under the License.
  */
 const path = require('path');
-const removeRecursive = require('rimraf').sync;
+const removeSync = require('rimraf').sync;
 const childProcess = require('child_process');
 const Downloader = require('../utils/ChromiumDownloader');
 const Connection = require('./Connection');
 const Browser = require('./Browser');
 const readline = require('readline');
 const crypto = require('crypto');
+const helper = require('./helper');
+const ChromiumRevision = require('../package.json').puppeteer.chromium_revision;
 
 const CHROME_PROFILE_PATH = path.resolve(__dirname, '..', '.dev_profile');
 let browserId = 0;
@@ -70,49 +72,41 @@ class Launcher {
     }
     let chromeExecutable = options.executablePath;
     if (typeof chromeExecutable !== 'string') {
-      let chromiumRevision = require('../package.json').puppeteer.chromium_revision;
+      let revisionInfo = Downloader.revisionInfo(Downloader.currentPlatform(), ChromiumRevision);
-      let revisionInfo = Downloader.revisionInfo(Downloader.currentPlatform(), chromiumRevision);
       console.assert(revisionInfo.downloaded, `Chromium revision is not downloaded. Run "npm install"`);
       chromeExecutable = revisionInfo.executablePath;
     }
     if (Array.isArray(options.args))
       chromeArguments.push(...options.args);
+
     let chromeProcess = childProcess.spawn(chromeExecutable, chromeArguments, {});
     if (options.dumpio) {
       chromeProcess.stdout.pipe(process.stdout);
       chromeProcess.stderr.pipe(process.stderr);
     }
-    let stderr = '';
+
-    chromeProcess.stderr.on('data', data => stderr += data.toString('utf8'));
     // Cleanup as processes exit.
-    const onProcessExit = () => chromeProcess.kill();
+    let listeners = [
-    process.on('exit', onProcessExit);
+      helper.addEventListener(process, 'exit', killChromeAndCleanup),
-    let terminated = false;
+      helper.addEventListener(chromeProcess, 'exit', killChromeAndCleanup),
-    chromeProcess.on('exit', () => {
+    ];
-      terminated = true;
+    if (options.handleSIGINT !== false)
-      process.removeListener('exit', onProcessExit);
+      listeners.push(helper.addEventListener(process, 'SIGINT', killChromeAndCleanup));
-      removeRecursive(userDataDir);
+    try {
-    });
-
-    let browserWSEndpoint = await waitForWSEndpoint(chromeProcess);
-    if (terminated) {
-      throw new Error([
-        'Failed to launch chrome!',
-        stderr,
-        '',
-        'TROUBLESHOOTING: https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md',
-        '',
-      ].join('\n'));
-    }
-    // Failed to connect to browser.
-    if (!browserWSEndpoint) {
-      chromeProcess.kill();
-      throw new Error('Failed to connect to chrome!');
-    }
-
       let connectionDelay = options.slowMo || 0;
+      let browserWSEndpoint = await waitForWSEndpoint(chromeProcess, options.timeout || 30 * 1000);
       let connection = await Connection.create(browserWSEndpoint, connectionDelay);
-    return new Browser(connection, !!options.ignoreHTTPSErrors, () => chromeProcess.kill());
+      return new Browser(connection, !!options.ignoreHTTPSErrors, killChromeAndCleanup);
+    } catch (e) {
+      killChromeAndCleanup();
+      throw e;
+    }
+
+    function killChromeAndCleanup() {
+      helper.removeEventListeners(listeners);
+      chromeProcess.kill('SIGKILL');
+      removeSync(userDataDir);
+    }
   }
 
   /**
@@ -127,23 +121,52 @@ class Launcher {
 
 /**
  * @param {!ChildProcess} chromeProcess
+ * @param {number} timeout
  * @return {!Promise<string>}
  */
-function waitForWSEndpoint(chromeProcess) {
+function waitForWSEndpoint(chromeProcess, timeout) {
-  return new Promise(fulfill => {
+  return new Promise((resolve, reject) => {
     const rl = readline.createInterface({ input: chromeProcess.stderr });
-    rl.on('line', onLine);
+    let stderr = '';
-    rl.once('close', () => fulfill(''));
+    let listeners = [
+      helper.addEventListener(rl, 'line', onLine),
+      helper.addEventListener(rl, 'close', onClose),
+      helper.addEventListener(chromeProcess, 'exit', onClose)
+    ];
+    let timeoutId = timeout ? setTimeout(onTimeout, timeout) : 0;
+
+    function onClose() {
+      cleanup();
+      reject(new Error([
+        'Failed to launch chrome!',
+        stderr,
+        '',
+        'TROUBLESHOOTING: https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md',
+        '',
+      ].join('\n')));
+    }
+
+    function onTimeout() {
+      cleanup();
+      reject(new Error(`Timed out after ${timeout} ms while trying to connect to Chrome! The only Chrome revision guaranteed to work is r${ChromiumRevision}`));
+    }
 
     /**
      * @param {string} line
      */
     function onLine(line) {
+      stderr += line + '\n';
       const match = line.match(/^DevTools listening on (ws:\/\/.*)$/);
       if (!match)
         return;
-      rl.removeListener('line', onLine);
+      cleanup();
-      fulfill(match[1]);
+      resolve(match[1]);
+    }
+
+    function cleanup() {
+      if (timeoutId)
+        clearTimeout(timeoutId);
+      helper.removeEventListeners(listeners);
     }
   });
 }