chore: further unify launching and connection (#2320)

docs/api.md

@@ -3976,6 +3976,7 @@ const { chromium } = require('playwright');  // Or 'firefox' or 'webkit'.
   - `wsEndpoint` <[string]> A browser websocket endpoint to connect to. **required**
   - `slowMo` <[number]> Slows down Playwright operations by the specified amount of milliseconds. Useful so that you can see what is going on. Defaults to 0.
   - `logger` <[Logger]> Logger sink for Playwright logging.
+  - `timeout` <[number]> Maximum time in milliseconds to wait for the connection to be established. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
 - returns: <[Promise]<[Browser]>>
 
 This methods attaches Playwright to an existing browser instance.

src/helper.ts

@@ -21,6 +21,7 @@ import * as fs from 'fs';
 import * as util from 'util';
 import { TimeoutError } from './errors';
 import * as types from './types';
+import { ChildProcess, execSync } from 'child_process';
 
 // NOTE: update this to point to playwright/lib when moving this file.
 const PLAYWRIGHT_LIB_PATH = __dirname;
@@ -332,6 +333,19 @@ class Helper {
   static optionsWithUpdatedTimeout<T extends types.TimeoutOptions>(options: T | undefined, deadline: number): T {
     return { ...(options || {}) as T, timeout: this.timeUntilDeadline(deadline) };
   }
+
+  static killProcess(proc: ChildProcess) {
+    if (proc.pid && !proc.killed) {
+      try {
+        if (process.platform === 'win32')
+          execSync(`taskkill /pid ${proc.pid} /T /F`);
+        else
+          process.kill(-proc.pid, 'SIGKILL');
+      } catch (e) {
+        // the process might have already stopped
+      }
+    }
+  }
 }
 
 export function assert(value: any, message?: string): asserts value {

src/logger.ts

@@ -62,12 +62,16 @@ export class RootLogger implements InnerLogger {
     this._logger.add(`launch`, new RecordingLogger('browser'));
   }
 
-  stopLaunchRecording(): string {
+  launchRecording(): string {
-    const logger = this._logger.remove(`launch`) as RecordingLogger;
+    const logger = this._logger.get(`launch`) as RecordingLogger;
     if (logger)
       return logger.recording();
     return '';
   }
+
+  stopLaunchRecording() {
+    this._logger.remove(`launch`);
+  }
 }
 
 const colorMap = new Map<string, number>([
@@ -87,10 +91,12 @@ class MultiplexingLogger implements Logger {
     this._loggers.set(id, logger);
   }
 
-  remove(id: string): Logger | undefined {
+  get(id: string): Logger | undefined {
-    const logger = this._loggers.get(id);
+    return this._loggers.get(id);
+  }
+
+  remove(id: string) {
     this._loggers.delete(id);
-    return logger;
   }
 
   isEnabled(name: string, severity: LoggerSeverity): boolean {

src/server/browserServer.ts

@@ -14,12 +14,12 @@
  * limitations under the License.
  */
 
-import { ChildProcess, execSync } from 'child_process';
+import { ChildProcess } from 'child_process';
 import { EventEmitter } from 'events';
 import { helper } from '../helper';
 import { RootLogger } from '../logger';
-import { TimeoutSettings } from '../timeoutSettings';
+import { LaunchOptions, processBrowserArgOptions } from './browserType';
-import { LaunchOptions } from './browserType';
+import { ConnectionTransport } from '../transport';
 
 export class WebSocketWrapper {
   readonly wsEndpoint: string;
@@ -51,32 +51,30 @@ export class WebSocketWrapper {
 }
 
 export class BrowserServer extends EventEmitter {
-  private _process: ChildProcess | undefined;
+  private _process: ChildProcess;
-  private _gracefullyClose: (() => Promise<void>) | undefined;
+  private _gracefullyClose: (() => Promise<void>);
   private _webSocketWrapper: WebSocketWrapper | null = null;
   readonly _launchOptions: LaunchOptions;
   readonly _logger: RootLogger;
-  readonly _launchDeadline: number;
+  readonly _downloadsPath: string;
+  readonly _transport: ConnectionTransport;
+  readonly _headful: boolean;
 
-  constructor(options: LaunchOptions) {
+  constructor(options: LaunchOptions, process: ChildProcess, gracefullyClose: () => Promise<void>, transport: ConnectionTransport, downloadsPath: string, webSocketWrapper: WebSocketWrapper | null) {
     super();
     this._launchOptions = options;
+    this._headful = !processBrowserArgOptions(options).headless;
     this._logger = new RootLogger(options.logger);
-    this._launchDeadline = TimeoutSettings.computeDeadline(typeof options.timeout === 'number' ? options.timeout : 30000);
-  }
 
-  _initialize(process: ChildProcess, gracefullyClose: () => Promise<void>, webSocketWrapper: WebSocketWrapper | null) {
     this._process = process;
     this._gracefullyClose = gracefullyClose;
+    this._transport = transport;
+    this._downloadsPath = downloadsPath;
     this._webSocketWrapper = webSocketWrapper;
   }
 
-  _isInitialized(): boolean {
-    return !!this._process;
-  }
-
   process(): ChildProcess {
-    return this._process!;
+    return this._process;
   }
 
   wsEndpoint(): string {
@@ -84,20 +82,11 @@ export class BrowserServer extends EventEmitter {
   }
 
   kill() {
-    if (this._process!.pid && !this._process!.killed) {
+    helper.killProcess(this._process);
-      try {
-        if (process.platform === 'win32')
-          execSync(`taskkill /pid ${this._process!.pid} /T /F`);
-        else
-          process.kill(-this._process!.pid, 'SIGKILL');
-      } catch (e) {
-        // the process might have already stopped
-      }
-    }
   }
 
   async close(): Promise<void> {
-    await this._gracefullyClose!();
+    await this._gracefullyClose();
   }
 
   async _checkLeaks(): Promise<void> {
@@ -105,28 +94,9 @@ export class BrowserServer extends EventEmitter {
       await this._webSocketWrapper.checkLeaks();
   }
 
-  async _initializeOrClose<T>(init: () => Promise<T>): Promise<T> {
+  async _closeOrKill(deadline: number): Promise<void> {
     try {
-      let promise: Promise<T>;
+      await helper.waitWithDeadline(this.close(), '', deadline, ''); // The error message is ignored.
-      if ((this._launchOptions as any).__testHookBeforeCreateBrowser)
-        promise = (this._launchOptions as any).__testHookBeforeCreateBrowser().then(init);
-      else
-        promise = init();
-      const result = await helper.waitWithDeadline(promise, 'the browser to launch', this._launchDeadline, 'pw:browser*');
-      this._logger.stopLaunchRecording();
-      return result;
-    } catch (e) {
-      e.message += '\n=============== Process output during launch: ===============\n' +
-          this._logger.stopLaunchRecording() +
-          '\n=============================================================';
-      await this._closeOrKill();
-      throw e;
-    }
-  }
-
-  private async _closeOrKill(): Promise<void> {
-    try {
-      await helper.waitWithDeadline(this.close(), '', this._launchDeadline, ''); // The error message is ignored.
     } catch (ignored) {
       this.kill();
     }

src/server/browserType.ts

@@ -20,7 +20,8 @@ import * as browserPaths from '../install/browserPaths';
 import { Logger, RootLogger } from '../logger';
 import { ConnectionTransport, WebSocketTransport } from '../transport';
 import { BrowserBase, BrowserOptions, Browser } from '../browser';
-import { assert } from '../helper';
+import { assert, helper } from '../helper';
+import { TimeoutSettings } from '../timeoutSettings';
 
 export type BrowserArgOptions = {
   headless?: boolean,
@@ -48,6 +49,7 @@ export type ConnectOptions = {
   wsEndpoint: string,
   slowMo?: number,
   logger?: Logger,
+  timeout?: number,
 };
 export type LaunchType = 'local' | 'server' | 'persistent';
 export type LaunchOptions = LaunchOptionsBase & { slowMo?: number };
@@ -86,42 +88,88 @@ export abstract class BrowserTypeBase implements BrowserType {
 
   async launch(options: LaunchOptions = {}): Promise<Browser> {
     assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead');
-    const browserServer = new BrowserServer(options);
+    return this._innerLaunch('local', options);
-    const { transport, downloadsPath } = await this._launchServer(options, 'local', browserServer);
-    return await browserServer._initializeOrClose(async () => {
-      return this._connectToServer(browserServer, false, transport!, downloadsPath);
-    });
   }
 
   async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise<BrowserContext> {
-    const browserServer = new BrowserServer(options);
+    const browser = await this._innerLaunch('persistent', options, userDataDir);
-    const { transport, downloadsPath } = await this._launchServer(options, 'persistent', browserServer, userDataDir);
+    return browser._defaultContext!;
+  }
 
-    return await browserServer._initializeOrClose(async () => {
+  async _innerLaunch(launchType: LaunchType, options: LaunchOptions, userDataDir?: string): Promise<BrowserBase> {
-      const browser = await this._connectToServer(browserServer, true, transport!, downloadsPath);
+    const deadline = TimeoutSettings.computeDeadline(options.timeout, 30000);
+    const logger = new RootLogger(options.logger);
+    logger.startLaunchRecording();
+
+    let browserServer: BrowserServer | undefined;
+    try {
+      browserServer = await this._launchServer(options, launchType, logger, deadline, userDataDir);
+      const promise = this._innerLaunchPromise(browserServer, launchType, options);
+      const browser = await helper.waitWithDeadline(promise, 'the browser to launch', deadline, 'pw:browser*');
+      return browser;
+    } catch (e) {
+      e.message += '\n=============== Process output during launch: ===============\n' +
+          logger.launchRecording() +
+          '\n=============================================================';
+      if (browserServer)
+        await browserServer._closeOrKill(deadline);
+      throw e;
+    } finally {
+      logger.stopLaunchRecording();
+    }
+  }
+
+  async _innerLaunchPromise(browserServer: BrowserServer, launchType: LaunchType, options: LaunchOptions): Promise<BrowserBase> {
+    if ((options as any).__testHookBeforeCreateBrowser)
+      await (options as any).__testHookBeforeCreateBrowser();
+
+    const browser = await this._connectToServer(browserServer, launchType === 'persistent');
+    if (launchType === 'persistent' && (!options.ignoreDefaultArgs || Array.isArray(options.ignoreDefaultArgs))) {
       const context = browser._defaultContext!;
-      if (!options.ignoreDefaultArgs || Array.isArray(options.ignoreDefaultArgs))
+      await context._loadDefaultContext();
-        await context._loadDefaultContext();
+    }
-      return context;
+    return browser;
-    });
   }
 
   async launchServer(options: LaunchServerOptions = {}): Promise<BrowserServer> {
-    const browserServer = new BrowserServer(options);
+    const logger = new RootLogger(options.logger);
-    await this._launchServer(options, 'server', browserServer);
+    return this._launchServer(options, 'server', logger, TimeoutSettings.computeDeadline(options.timeout, 30000));
-    return browserServer;
   }
 
   async connect(options: ConnectOptions): Promise<Browser> {
+    const deadline = TimeoutSettings.computeDeadline(options.timeout, 30000);
     const logger = new RootLogger(options.logger);
-    return await WebSocketTransport.connect(options.wsEndpoint, async transport => {
+    logger.startLaunchRecording();
-      if ((options as any).__testHookBeforeCreateBrowser)
+
-        await (options as any).__testHookBeforeCreateBrowser();
+    let transport: ConnectionTransport | undefined;
-      return this._connectToTransport(transport, { slowMo: options.slowMo, logger, downloadsPath: '' });
+    try {
-    }, logger);
+      transport = await WebSocketTransport.connect(options.wsEndpoint, logger, deadline);
+      const promise = this._innerConnectPromise(transport, options, logger);
+      const browser = await helper.waitWithDeadline(promise, 'connect to browser', deadline, 'pw:browser*');
+      logger.stopLaunchRecording();
+      return browser;
+    } catch (e) {
+      e.message += '\n=============== Process output during connect: ===============\n' +
+          logger.launchRecording() +
+          '\n=============================================================';
+      try {
+        if (transport)
+          transport.close();
+      } catch (e) {
+      }
+      throw e;
+    } finally {
+      logger.stopLaunchRecording();
+    }
   }
 
-  abstract _launchServer(options: LaunchServerOptions, launchType: LaunchType, browserServer: BrowserServer, userDataDir?: string): Promise<{ transport?: ConnectionTransport, downloadsPath: string }>;
+  async _innerConnectPromise(transport: ConnectionTransport, options: ConnectOptions, logger: RootLogger): Promise<Browser> {
-  abstract _connectToServer(browserServer: BrowserServer, persistent: boolean, transport: ConnectionTransport, downloadsPath: string): Promise<BrowserBase>;
+    if ((options as any).__testHookBeforeCreateBrowser)
+      await (options as any).__testHookBeforeCreateBrowser();
+    return this._connectToTransport(transport, { slowMo: options.slowMo, logger, downloadsPath: '' });
+  }
+
+  abstract _launchServer(options: LaunchServerOptions, launchType: LaunchType, logger: RootLogger, deadline: number, userDataDir?: string): Promise<BrowserServer>;
+  abstract _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise<BrowserBase>;
   abstract _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<BrowserBase>;
 }

src/server/chromium.ts

@@ -19,7 +19,7 @@ import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
 import * as util from 'util';
-import { helper, assert, isDebugMode, debugAssert } from '../helper';
+import { helper, assert, isDebugMode } from '../helper';
 import { CRBrowser } from '../chromium/crBrowser';
 import * as ws from 'ws';
 import { launchProcess } from './processLauncher';
@@ -29,7 +29,7 @@ import { BrowserArgOptions, LaunchServerOptions, BrowserTypeBase, processBrowser
 import { BrowserServer, WebSocketWrapper } from './browserServer';
 import { Events } from '../events';
 import { ConnectionTransport, ProtocolRequest } from '../transport';
-import { InnerLogger, logError } from '../logger';
+import { InnerLogger, logError, RootLogger } from '../logger';
 import { BrowserDescriptor } from '../install/browserPaths';
 import { CRDevTools } from '../chromium/crDevTools';
 import { BrowserBase, BrowserOptions } from '../browser';
@@ -47,19 +47,19 @@ export class Chromium extends BrowserTypeBase {
     return new CRDevTools(path.join(this._browserPath, 'devtools-preferences.json'));
   }
 
-  async _connectToServer(browserServer: BrowserServer, persistent: boolean, transport: ConnectionTransport, downloadsPath: string): Promise<BrowserBase> {
+  async _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise<BrowserBase> {
     const options = browserServer._launchOptions;
     let devtools = this._devtools;
     if ((options as any).__testHookForDevTools) {
       devtools = this._createDevTools();
       await (options as any).__testHookForDevTools(devtools);
     }
-    return await CRBrowser.connect(transport, {
+    return await CRBrowser.connect(browserServer._transport, {
       slowMo: options.slowMo,
       persistent,
-      headful: !processBrowserArgOptions(options).headless,
+      headful: browserServer._headful,
       logger: browserServer._logger,
-      downloadsPath,
+      downloadsPath: browserServer._downloadsPath,
       ownedServer: browserServer,
     }, devtools);
   }
@@ -68,7 +68,7 @@ export class Chromium extends BrowserTypeBase {
     return CRBrowser.connect(transport, options);
   }
 
-  async _launchServer(options: LaunchServerOptions, launchType: LaunchType, browserServer: BrowserServer, userDataDir?: string): Promise<{ transport?: ConnectionTransport, downloadsPath: string }> {
+  async _launchServer(options: LaunchServerOptions, launchType: LaunchType, logger: RootLogger, deadline: number, userDataDir?: string): Promise<BrowserServer> {
     const {
       ignoreDefaultArgs = false,
       args = [],
@@ -80,7 +80,6 @@ export class Chromium extends BrowserTypeBase {
       port = 0,
     } = options;
     assert(!port || launchType === 'server', 'Cannot specify a port without launching as a server.');
-    const logger = browserServer._logger;
 
     let temporaryUserDataDir: string | null = null;
     if (!userDataDir) {
@@ -106,6 +105,7 @@ export class Chromium extends BrowserTypeBase {
     // Note: it is important to define these variables before launchProcess, so that we don't get
     // "Cannot access 'browserServer' before initialization" if something went wrong.
     let transport: PipeTransport | undefined = undefined;
+    let browserServer: BrowserServer | undefined = undefined;
     const { launchedProcess, gracefullyClose, downloadsPath } = await launchProcess({
       executablePath: chromeExecutable,
       args: chromeArguments,
@@ -119,7 +119,6 @@ export class Chromium extends BrowserTypeBase {
       attemptToGracefullyClose: async () => {
         if ((options as any).__testHookGracefullyClose)
           await (options as any).__testHookGracefullyClose();
-        debugAssert(browserServer._isInitialized());
         // We try to gracefully close to prevent crash reporting and core dumps.
         // Note that it's fine to reuse the pipe transport, since
         // our connection ignores kBrowserCloseMessageId.
@@ -128,14 +127,15 @@ export class Chromium extends BrowserTypeBase {
         t.send(message);
       },
       onkill: (exitCode, signal) => {
-        browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
+        if (browserServer)
+          browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
       },
     });
 
     const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
     transport = new PipeTransport(stdio[3], stdio[4], logger);
-    browserServer._initialize(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port) : null);
+    browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, transport, downloadsPath, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port) : null);
-    return { transport, downloadsPath };
+    return browserServer;
   }
 
   private _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string): string[] {

src/server/electron.ts

@@ -168,8 +168,7 @@ export class Electron  {
       handleSIGTERM = true,
       handleSIGHUP = true,
     } = options;
-    const browserServer = new BrowserServer(options);
+    const deadline = TimeoutSettings.computeDeadline(options.timeout, 30000);
-
     let app: ElectronApplication | undefined = undefined;
 
     const logger = new RootLogger(options.logger);
@@ -192,18 +191,14 @@ export class Electron  {
       },
     });
 
-    const deadline = browserServer._launchDeadline;
     const timeoutError = new TimeoutError(`Timed out while trying to connect to Electron!`);
     const nodeMatch = await waitForLine(launchedProcess, launchedProcess.stderr, /^Debugger listening on (ws:\/\/.*)$/, helper.timeUntilDeadline(deadline), timeoutError);
-    const nodeConnection = await WebSocketTransport.connect(nodeMatch[1], transport => {
+    const nodeTransport = await WebSocketTransport.connect(nodeMatch[1], logger, deadline);
-      return new CRConnection(transport, logger);
+    const nodeConnection = new CRConnection(nodeTransport, logger);
-    });
 
     const chromeMatch = await waitForLine(launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/, helper.timeUntilDeadline(deadline), timeoutError);
-    const chromeTransport = await WebSocketTransport.connect(chromeMatch[1], transport => {
+    const chromeTransport = await WebSocketTransport.connect(chromeMatch[1], logger, deadline);
-      return transport;
+    const browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, chromeTransport, '', null);
-    }, logger);
-    browserServer._initialize(launchedProcess, gracefullyClose, null);
     const browser = await CRBrowser.connect(chromeTransport, { headful: true, logger, persistent: true, viewport: null, ownedServer: browserServer, downloadsPath: '' });
     app = new ElectronApplication(logger, browser, nodeConnection);
     await app._init();

src/server/firefox.ts

@@ -24,12 +24,12 @@ import { TimeoutError } from '../errors';
 import { Events } from '../events';
 import { FFBrowser } from '../firefox/ffBrowser';
 import { kBrowserCloseMessageId } from '../firefox/ffConnection';
-import { helper, assert, debugAssert } from '../helper';
+import { helper, assert } from '../helper';
 import { BrowserServer, WebSocketWrapper } from './browserServer';
 import { BrowserArgOptions, LaunchServerOptions, BrowserTypeBase, processBrowserArgOptions, LaunchType } from './browserType';
 import { launchProcess, waitForLine } from './processLauncher';
 import { ConnectionTransport, SequenceNumberMixer, WebSocketTransport } from '../transport';
-import { InnerLogger, logError } from '../logger';
+import { InnerLogger, logError, RootLogger } from '../logger';
 import { BrowserDescriptor } from '../install/browserPaths';
 import { BrowserBase, BrowserOptions } from '../browser';
 
@@ -40,26 +40,22 @@ export class Firefox extends BrowserTypeBase {
     super(packagePath, browser);
   }
 
-  _connectToServer(browserServer: BrowserServer, persistent: boolean, transport: ConnectionTransport, downloadsPath: string): Promise<BrowserBase> {
+  _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise<BrowserBase> {
-    const options = browserServer._launchOptions;
+    return FFBrowser.connect(browserServer._transport, {
-    // TODO: connect to the underlying socket.
+      slowMo: browserServer._launchOptions.slowMo,
-    return WebSocketTransport.connect(browserServer.wsEndpoint()!, transport => {
+      logger: browserServer._logger,
-      return FFBrowser.connect(transport, {
+      persistent,
-        slowMo: options.slowMo,
+      downloadsPath: browserServer._downloadsPath,
-        logger: browserServer._logger,
+      headful: browserServer._headful,
-        persistent,
+      ownedServer: browserServer,
-        downloadsPath,
+    });
-        headful: !processBrowserArgOptions(options).headless,
-        ownedServer: browserServer,
-      });
-    }, browserServer._logger);
   }
 
   _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<FFBrowser> {
     return FFBrowser.connect(transport, options);
   }
 
-  async _launchServer(options: LaunchServerOptions, launchType: LaunchType, browserServer: BrowserServer, userDataDir?: string): Promise<{ downloadsPath: string }> {
+  async _launchServer(options: LaunchServerOptions, launchType: LaunchType, logger: RootLogger, deadline: number, userDataDir?: string): Promise<BrowserServer> {
     const {
       ignoreDefaultArgs = false,
       args = [],
@@ -72,7 +68,6 @@ export class Firefox extends BrowserTypeBase {
       port = 0,
     } = options;
     assert(!port || launchType === 'server', 'Cannot specify a port without launching as a server.');
-    const logger = browserServer._logger;
 
     let temporaryProfileDir = null;
     if (!userDataDir) {
@@ -93,8 +88,9 @@ export class Firefox extends BrowserTypeBase {
       throw new Error(`No executable path is specified. Pass "executablePath" option directly.`);
 
     // Note: it is important to define these variables before launchProcess, so that we don't get
-    // "Cannot access 'browserServer' before initialization" if something went wrong.
+    // "Cannot access 'transport' before initialization" if something went wrong.
-    let browserWSEndpoint: string | undefined = undefined;
+    let browserServer: BrowserServer | undefined = undefined;
+    let transport: ConnectionTransport | undefined = undefined;
     const { launchedProcess, gracefullyClose, downloadsPath } = await launchProcess({
       executablePath: firefoxExecutable,
       args: firefoxArguments,
@@ -112,14 +108,14 @@ export class Firefox extends BrowserTypeBase {
       attemptToGracefullyClose: async () => {
         if ((options as any).__testHookGracefullyClose)
           await (options as any).__testHookGracefullyClose();
-        debugAssert(browserServer._isInitialized());
+
         // We try to gracefully close to prevent crash reporting and core dumps.
-        const transport = await WebSocketTransport.connect(browserWSEndpoint!, async transport => transport);
         const message = { method: 'Browser.close', params: {}, id: kBrowserCloseMessageId };
-        transport.send(message);
+        transport!.send(message);
       },
       onkill: (exitCode, signal) => {
-        browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
+        if (browserServer)
+          browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
       },
     });
 
@@ -127,12 +123,17 @@ export class Firefox extends BrowserTypeBase {
     const match = await waitForLine(launchedProcess, launchedProcess.stdout, /^Juggler listening on (ws:\/\/.*)$/, timeout, timeoutError);
     const innerEndpoint = match[1];
 
-    const webSocketWrapper = launchType === 'server' ?
+    try {
-      (await WebSocketTransport.connect(innerEndpoint, t => wrapTransportWithWebSocket(t, logger, port), logger)) :
+      // If we can't communicate with Firefox on start, kill the process and exit.
-      new WebSocketWrapper(innerEndpoint, []);
+      transport = await WebSocketTransport.connect(innerEndpoint, logger, deadline);
-    browserWSEndpoint = webSocketWrapper.wsEndpoint;
+    } catch (e) {
-    browserServer._initialize(launchedProcess, gracefullyClose, webSocketWrapper);
+      helper.killProcess(launchedProcess);
-    return { downloadsPath };
+      throw e;
+    }
+
+    const webSocketWrapper = launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port) : null;
+    browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, transport, downloadsPath, webSocketWrapper);
+    return browserServer;
   }
 
   private _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string, port: number): string[] {

src/server/processLauncher.ts

@@ -73,7 +73,6 @@ type LaunchResult = {
 export async function launchProcess(options: LaunchProcessOptions): Promise<LaunchResult> {
   const logger = options.logger;
   const stdio: ('ignore' | 'pipe')[] = options.pipe ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['ignore', 'pipe', 'pipe'];
-  logger.startLaunchRecording();
   logger._log(browserLog, `<launching> ${options.executablePath} ${options.args.join(' ')}`);
   const spawnedProcess = childProcess.spawn(
       options.executablePath,

src/server/webkit.ts

@@ -38,14 +38,13 @@ export class WebKit extends BrowserTypeBase {
     super(packagePath, browser);
   }
 
-  _connectToServer(browserServer: BrowserServer, persistent: boolean, transport: ConnectionTransport, downloadsPath: string): Promise<BrowserBase> {
+  _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise<BrowserBase> {
-    const options = browserServer._launchOptions;
+    return WKBrowser.connect(browserServer._transport, {
-    return WKBrowser.connect(transport, {
+      slowMo: browserServer._launchOptions.slowMo,
-      slowMo: options.slowMo,
+      headful: browserServer._headful,
-      headful: !processBrowserArgOptions(options).headless,
       logger: browserServer._logger,
       persistent,
-      downloadsPath,
+      downloadsPath: browserServer._downloadsPath,
       ownedServer: browserServer
     });
   }
@@ -54,7 +53,7 @@ export class WebKit extends BrowserTypeBase {
     return WKBrowser.connect(transport, options);
   }
 
-  async _launchServer(options: LaunchServerOptions, launchType: LaunchType, browserServer: BrowserServer, userDataDir?: string): Promise<{ transport?: ConnectionTransport, downloadsPath: string, logger: RootLogger }> {
+  async _launchServer(options: LaunchServerOptions, launchType: LaunchType, logger: RootLogger, deadline: number, userDataDir?: string): Promise<BrowserServer> {
     const {
       ignoreDefaultArgs = false,
       args = [],
@@ -66,7 +65,6 @@ export class WebKit extends BrowserTypeBase {
       port = 0,
     } = options;
     assert(!port || launchType === 'server', 'Cannot specify a port without launching as a server.');
-    const logger = browserServer._logger;
 
     let temporaryUserDataDir: string | null = null;
     if (!userDataDir) {
@@ -89,6 +87,7 @@ export class WebKit extends BrowserTypeBase {
     // Note: it is important to define these variables before launchProcess, so that we don't get
     // "Cannot access 'browserServer' before initialization" if something went wrong.
     let transport: ConnectionTransport | undefined = undefined;
+    let browserServer: BrowserServer | undefined = undefined;
     const { launchedProcess, gracefullyClose, downloadsPath } = await launchProcess({
       executablePath: webkitExecutable,
       args: webkitArguments,
@@ -102,21 +101,21 @@ export class WebKit extends BrowserTypeBase {
       attemptToGracefullyClose: async () => {
         if ((options as any).__testHookGracefullyClose)
           await (options as any).__testHookGracefullyClose();
-        assert(transport);
         // We try to gracefully close to prevent crash reporting and core dumps.
         // Note that it's fine to reuse the pipe transport, since
         // our connection ignores kBrowserCloseMessageId.
-        transport.send({method: 'Playwright.close', params: {}, id: kBrowserCloseMessageId});
+        transport!.send({method: 'Playwright.close', params: {}, id: kBrowserCloseMessageId});
       },
       onkill: (exitCode, signal) => {
-        browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
+        if (browserServer)
+          browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
       },
     });
 
     const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
     transport = new PipeTransport(stdio[3], stdio[4], logger);
-    browserServer._initialize(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port || 0) : null);
+    browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, transport, downloadsPath, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port || 0) : null);
-    return { transport, downloadsPath, logger };
+    return browserServer;
   }
 
   _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string, port: number): string[] {

src/timeoutSettings.ts

@@ -60,10 +60,11 @@ export class TimeoutSettings {
     return TimeoutSettings.computeDeadline(typeof timeout === 'number' ? timeout : this._timeout());
   }
 
-  static computeDeadline(timeout: number): number {
+  static computeDeadline(timeout: number | undefined, defaultValue = 30000): number {
     if (timeout === 0)
       return Number.MAX_SAFE_INTEGER;
-    else
+    else if (typeof timeout === 'number')
       return helper.monotonicTime() + timeout;
+    return helper.monotonicTime() + defaultValue;
   }
 }

src/transport.ts

@@ -119,39 +119,33 @@ export class DeferWriteTransport implements ConnectionTransport {
 }
 
 export class WebSocketTransport implements ConnectionTransport {
-  _ws: WebSocket;
+  private _ws: WebSocket;
-  _logger?: InnerLogger;
+  private _logger: InnerLogger;
 
   onmessage?: (message: ProtocolResponse) => void;
   onclose?: () => void;
 
-  // 'onmessage' handler must be installed synchronously when 'onopen' callback is invoked to
+  static connect(url: string, logger: InnerLogger, deadline: number): Promise<ConnectionTransport> {
-  // avoid missing incoming messages.
+    logger._log({ name: 'browser' }, `<ws connecting> ${url}`);
-  static connect<T>(url: string, onopen: (transport: ConnectionTransport) => Promise<T> | T, logger?: InnerLogger): Promise<T> {
+    const transport = new WebSocketTransport(url, logger, deadline);
-    logger && logger._log({ name: 'browser' }, `<ws connecting> ${url}`);
+    return new Promise<ConnectionTransport>((fulfill, reject) => {
-    const transport = new WebSocketTransport(url, logger);
-    return new Promise<T>((fulfill, reject) => {
       transport._ws.addEventListener('open', async () => {
-        logger && logger._log({ name: 'browser' }, `<ws connected> ${url}`);
+        logger._log({ name: 'browser' }, `<ws connected> ${url}`);
-        try {
+        fulfill(transport);
-          fulfill(await onopen(transport));
-        } catch (e) {
-          logger && logger._log({ name: 'browser' }, `<ws disconnecting> ${url}`);
-          try { transport._ws.close(); } catch (ignored) {}
-          reject(e);
-        }
       });
       transport._ws.addEventListener('error', event => {
-        logger && logger._log({ name: 'browser' }, `<ws connect error> ${url} ${event.message}`);
+        logger._log({ name: 'browser' }, `<ws connect error> ${url} ${event.message}`);
         reject(new Error('WebSocket error: ' + event.message));
+        transport._ws.close();
       });
     });
   }
 
-  constructor(url: string, logger: InnerLogger | undefined) {
+  constructor(url: string, logger: InnerLogger, deadline: number) {
     this._ws = new WebSocket(url, [], {
       perMessageDeflate: false,
-      maxPayload: 256 * 1024 * 1024, // 256Mb
+      maxPayload: 256 * 1024 * 1024, // 256Mb,
+      handshakeTimeout: helper.timeUntilDeadline(deadline)
     });
     this._logger = logger;
     // The 'ws' module in node sometimes sends us multiple messages in a single task.