diff --git a/docs/api.md b/docs/api.md index 5330ea12d0..44b1353ed3 100644 --- a/docs/api.md +++ b/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. diff --git a/src/helper.ts b/src/helper.ts index 3a117c96f0..9ef343cb82 100644 --- a/src/helper.ts +++ b/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(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 { diff --git a/src/logger.ts b/src/logger.ts index 925283ea75..6df3cd47a7 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -62,12 +62,16 @@ export class RootLogger implements InnerLogger { this._logger.add(`launch`, new RecordingLogger('browser')); } - stopLaunchRecording(): string { - const logger = this._logger.remove(`launch`) as RecordingLogger; + launchRecording(): string { + const logger = this._logger.get(`launch`) as RecordingLogger; if (logger) return logger.recording(); return ''; } + + stopLaunchRecording() { + this._logger.remove(`launch`); + } } const colorMap = new Map([ @@ -87,10 +91,12 @@ class MultiplexingLogger implements Logger { this._loggers.set(id, logger); } - remove(id: string): Logger | undefined { - const logger = this._loggers.get(id); + get(id: string): Logger | undefined { + return this._loggers.get(id); + } + + remove(id: string) { this._loggers.delete(id); - return logger; } isEnabled(name: string, severity: LoggerSeverity): boolean { diff --git a/src/server/browserServer.ts b/src/server/browserServer.ts index 66d5128af5..fcba491aec 100644 --- a/src/server/browserServer.ts +++ b/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 } from './browserType'; +import { LaunchOptions, processBrowserArgOptions } 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 _gracefullyClose: (() => Promise) | undefined; + private _process: ChildProcess; + private _gracefullyClose: (() => Promise); 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, 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, 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) { - 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 - } - } + helper.killProcess(this._process); } async close(): Promise { - await this._gracefullyClose!(); + await this._gracefullyClose(); } async _checkLeaks(): Promise { @@ -105,28 +94,9 @@ export class BrowserServer extends EventEmitter { await this._webSocketWrapper.checkLeaks(); } - async _initializeOrClose(init: () => Promise): Promise { + async _closeOrKill(deadline: number): Promise { try { - let promise: Promise; - 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 { - try { - await helper.waitWithDeadline(this.close(), '', this._launchDeadline, ''); // The error message is ignored. + await helper.waitWithDeadline(this.close(), '', deadline, ''); // The error message is ignored. } catch (ignored) { this.kill(); } diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 3604c157ec..31c80c8b33 100644 --- a/src/server/browserType.ts +++ b/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 { assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); - const browserServer = new BrowserServer(options); - const { transport, downloadsPath } = await this._launchServer(options, 'local', browserServer); - return await browserServer._initializeOrClose(async () => { - return this._connectToServer(browserServer, false, transport!, downloadsPath); - }); + return this._innerLaunch('local', options); } async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise { - const browserServer = new BrowserServer(options); - const { transport, downloadsPath } = await this._launchServer(options, 'persistent', browserServer, userDataDir); + const browser = await this._innerLaunch('persistent', options, userDataDir); + return browser._defaultContext!; + } - return await browserServer._initializeOrClose(async () => { - const browser = await this._connectToServer(browserServer, true, transport!, downloadsPath); + async _innerLaunch(launchType: LaunchType, options: LaunchOptions, userDataDir?: string): Promise { + 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 { + 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(); - return context; - }); + await context._loadDefaultContext(); + } + return browser; } async launchServer(options: LaunchServerOptions = {}): Promise { - const browserServer = new BrowserServer(options); - await this._launchServer(options, 'server', browserServer); - return browserServer; + const logger = new RootLogger(options.logger); + return this._launchServer(options, 'server', logger, TimeoutSettings.computeDeadline(options.timeout, 30000)); } async connect(options: ConnectOptions): Promise { + const deadline = TimeoutSettings.computeDeadline(options.timeout, 30000); const logger = new RootLogger(options.logger); - return await WebSocketTransport.connect(options.wsEndpoint, async transport => { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); - return this._connectToTransport(transport, { slowMo: options.slowMo, logger, downloadsPath: '' }); - }, logger); + logger.startLaunchRecording(); + + let transport: ConnectionTransport | undefined; + try { + 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 }>; - abstract _connectToServer(browserServer: BrowserServer, persistent: boolean, transport: ConnectionTransport, downloadsPath: string): Promise; + async _innerConnectPromise(transport: ConnectionTransport, options: ConnectOptions, logger: RootLogger): Promise { + 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; + abstract _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise; abstract _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise; } diff --git a/src/server/chromium.ts b/src/server/chromium.ts index aee204d483..863bfd06a3 100644 --- a/src/server/chromium.ts +++ b/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 { + async _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise { 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 { 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); - return { transport, downloadsPath }; + browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, transport, downloadsPath, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port) : null); + return browserServer; } private _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string): string[] { diff --git a/src/server/electron.ts b/src/server/electron.ts index 511e335632..173fe08457 100644 --- a/src/server/electron.ts +++ b/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 => { - return new CRConnection(transport, logger); - }); + const nodeTransport = await WebSocketTransport.connect(nodeMatch[1], logger, deadline); + 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 => { - return transport; - }, logger); - browserServer._initialize(launchedProcess, gracefullyClose, null); + const chromeTransport = await WebSocketTransport.connect(chromeMatch[1], logger, deadline); + const browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, chromeTransport, '', 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(); diff --git a/src/server/firefox.ts b/src/server/firefox.ts index 8538f7a1dc..92edd2845a 100644 --- a/src/server/firefox.ts +++ b/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 { - const options = browserServer._launchOptions; - // TODO: connect to the underlying socket. - return WebSocketTransport.connect(browserServer.wsEndpoint()!, transport => { - return FFBrowser.connect(transport, { - slowMo: options.slowMo, - logger: browserServer._logger, - persistent, - downloadsPath, - headful: !processBrowserArgOptions(options).headless, - ownedServer: browserServer, - }); - }, browserServer._logger); + _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise { + return FFBrowser.connect(browserServer._transport, { + slowMo: browserServer._launchOptions.slowMo, + logger: browserServer._logger, + persistent, + downloadsPath: browserServer._downloadsPath, + headful: browserServer._headful, + ownedServer: browserServer, + }); } _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise { 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 { 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. - let browserWSEndpoint: string | undefined = undefined; + // "Cannot access 'transport' before initialization" if something went wrong. + 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' ? - (await WebSocketTransport.connect(innerEndpoint, t => wrapTransportWithWebSocket(t, logger, port), logger)) : - new WebSocketWrapper(innerEndpoint, []); - browserWSEndpoint = webSocketWrapper.wsEndpoint; - browserServer._initialize(launchedProcess, gracefullyClose, webSocketWrapper); - return { downloadsPath }; + try { + // If we can't communicate with Firefox on start, kill the process and exit. + transport = await WebSocketTransport.connect(innerEndpoint, logger, deadline); + } catch (e) { + helper.killProcess(launchedProcess); + 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[] { diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index 9c046212ae..cfb06c860d 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -73,7 +73,6 @@ type LaunchResult = { export async function launchProcess(options: LaunchProcessOptions): Promise { const logger = options.logger; const stdio: ('ignore' | 'pipe')[] = options.pipe ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['ignore', 'pipe', 'pipe']; - logger.startLaunchRecording(); logger._log(browserLog, ` ${options.executablePath} ${options.args.join(' ')}`); const spawnedProcess = childProcess.spawn( options.executablePath, diff --git a/src/server/webkit.ts b/src/server/webkit.ts index 76ed0c248f..fcfc469f08 100644 --- a/src/server/webkit.ts +++ b/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 { - const options = browserServer._launchOptions; - return WKBrowser.connect(transport, { - slowMo: options.slowMo, - headful: !processBrowserArgOptions(options).headless, + _connectToServer(browserServer: BrowserServer, persistent: boolean): Promise { + return WKBrowser.connect(browserServer._transport, { + slowMo: browserServer._launchOptions.slowMo, + headful: browserServer._headful, 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 { 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); - return { transport, downloadsPath, logger }; + browserServer = new BrowserServer(options, launchedProcess, gracefullyClose, transport, downloadsPath, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port || 0) : null); + return browserServer; } _defaultArgs(options: BrowserArgOptions = {}, launchType: LaunchType, userDataDir: string, port: number): string[] { diff --git a/src/timeoutSettings.ts b/src/timeoutSettings.ts index 04840f5b72..c62634dd65 100644 --- a/src/timeoutSettings.ts +++ b/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; } } diff --git a/src/transport.ts b/src/transport.ts index 810eab1efa..92c6e153f3 100644 --- a/src/transport.ts +++ b/src/transport.ts @@ -119,39 +119,33 @@ export class DeferWriteTransport implements ConnectionTransport { } export class WebSocketTransport implements ConnectionTransport { - _ws: WebSocket; - _logger?: InnerLogger; + private _ws: WebSocket; + private _logger: InnerLogger; onmessage?: (message: ProtocolResponse) => void; onclose?: () => void; - // 'onmessage' handler must be installed synchronously when 'onopen' callback is invoked to - // avoid missing incoming messages. - static connect(url: string, onopen: (transport: ConnectionTransport) => Promise | T, logger?: InnerLogger): Promise { - logger && logger._log({ name: 'browser' }, ` ${url}`); - const transport = new WebSocketTransport(url, logger); - return new Promise((fulfill, reject) => { + static connect(url: string, logger: InnerLogger, deadline: number): Promise { + logger._log({ name: 'browser' }, ` ${url}`); + const transport = new WebSocketTransport(url, logger, deadline); + return new Promise((fulfill, reject) => { transport._ws.addEventListener('open', async () => { - logger && logger._log({ name: 'browser' }, ` ${url}`); - try { - fulfill(await onopen(transport)); - } catch (e) { - logger && logger._log({ name: 'browser' }, ` ${url}`); - try { transport._ws.close(); } catch (ignored) {} - reject(e); - } + logger._log({ name: 'browser' }, ` ${url}`); + fulfill(transport); }); transport._ws.addEventListener('error', event => { - logger && logger._log({ name: 'browser' }, ` ${url} ${event.message}`); + logger._log({ name: 'browser' }, ` ${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.