fix(launcher): ensure that local browser launch waits for process exit (#489)

src/server/crPlaywright.ts

@@ -67,7 +67,10 @@ export class CRBrowserServer {
   }
 
   async connect(): Promise<CRBrowser> {
-    return CRBrowser.connect(this._connectOptions);
+    const browser = await CRBrowser.connect(this._connectOptions);
+    // Hack: for typical launch scenario, ensure that close waits for actual process termination.
+    browser.close = this._gracefullyClose;
+    return browser;
   }
 
   process(): ChildProcess {

src/server/ffPlaywright.ts

@@ -64,7 +64,10 @@ export class FFBrowserServer {
   }
 
   async connect(): Promise<FFBrowser> {
-    return FFBrowser.connect(this._connectOptions);
+    const browser = await FFBrowser.connect(this._connectOptions);
+    // Hack: for typical launch scenario, ensure that close waits for actual process termination.
+    browser.close = this._gracefullyClose;
+    return browser;
   }
 
   process(): ChildProcess {

src/server/processLauncher.ts

@@ -23,6 +23,7 @@ import * as readline from 'readline';
 import { TimeoutError } from '../errors';
 import * as platform from '../platform';
 
+const debugLauncher = platform.debug('pw:launcher');
 const removeFolderAsync = platform.promisify(removeFolder);
 
 export type LaunchProcessOptions = {
@@ -43,7 +44,10 @@ export type LaunchProcessOptions = {
 
 type LaunchResult = { launchedProcess: childProcess.ChildProcess, gracefullyClose: () => Promise<void> };
 
+let lastLaunchedId = 0;
+
 export async function launchProcess(options: LaunchProcessOptions): Promise<LaunchResult> {
+  const id = ++lastLaunchedId;
   let stdio: ('ignore' | 'pipe')[] = ['pipe', 'pipe', 'pipe'];
   if (options.pipe) {
     if (options.dumpio)
@@ -63,6 +67,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
         stdio
       }
   );
+  debugLauncher(`[${id}] <launching> ${options.executablePath} ${options.args.join(' ')}`);
 
   if (!spawnedProcess.pid) {
     let reject: (e: Error) => void;
@@ -81,13 +86,14 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
   let processClosed = false;
   const waitForProcessToClose = new Promise((fulfill, reject) => {
     spawnedProcess.once('exit', () => {
+      debugLauncher(`[${id}] <process did exit>`);
       processClosed = true;
       helper.removeEventListeners(listeners);
       // Cleanup as processes exit.
       if (options.tempDir) {
         removeFolderAsync(options.tempDir)
-            .then(() => fulfill())
+            .catch((err: Error) => console.error(err))
-            .catch((err: Error) => console.error(err));
+            .then(fulfill);
       } else {
         fulfill();
       }
@@ -111,14 +117,17 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
     if (gracefullyClosing)
       return;
     gracefullyClosing = true;
+    debugLauncher(`[${id}] <gracefully close start>`);
     options.attemptToGracefullyClose().catch(() => killProcess());
     // TODO: forcefully kill the process after some timeout.
     await waitForProcessToClose;
+    debugLauncher(`[${id}] <gracefully close end>`);
     helper.removeEventListeners(listeners);
   }
 
   // This method has to be sync to be used as 'exit' event handler.
   function killProcess() {
+    debugLauncher(`[${id}] <kill>`);
     helper.removeEventListeners(listeners);
     if (spawnedProcess.pid && !spawnedProcess.killed && !processClosed) {
       // Force kill chrome.

src/server/wkPlaywright.ts

@@ -57,7 +57,10 @@ export class WKBrowserServer {
   }
 
   async connect(): Promise<WKBrowser> {
-    return WKBrowser.connect(this._connectOptions);
+    const browser = await WKBrowser.connect(this._connectOptions);
+    // Hack: for typical launch scenario, ensure that close waits for actual process termination.
+    browser.close = this._gracefullyClose;
+    return browser;
   }
 
   process(): ChildProcess {