diff --git a/src/logger.ts b/src/logger.ts index 75d4bf0a6d..925283ea75 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -41,24 +41,32 @@ export function logError(logger: InnerLogger): (error: Error) => void { } export class RootLogger implements InnerLogger { - private _userSink: Logger | undefined; - private _debugSink: DebugLoggerSink; + private _logger = new MultiplexingLogger(); constructor(userSink: Logger | undefined) { - this._userSink = userSink; - this._debugSink = new DebugLoggerSink(); + if (userSink) + this._logger.add('user', userSink); + this._logger.add('debug', new DebugLogger()); } _isLogEnabled(log: Log): boolean { - return (this._userSink && this._userSink.isEnabled(log.name, log.severity || 'info')) || - this._debugSink.isEnabled(log.name, log.severity || 'info'); + return this._logger.isEnabled(log.name, log.severity || 'info'); } _log(log: Log, message: string | Error, ...args: any[]) { - if (this._userSink && this._userSink.isEnabled(log.name, log.severity || 'info')) - this._userSink.log(log.name, log.severity || 'info', message, args, log.color ? { color: log.color } : {}); - if (this._debugSink.isEnabled(log.name, log.severity || 'info')) - this._debugSink.log(log.name, log.severity || 'info', message, args, log.color ? { color: log.color } : {}); + if (this._logger.isEnabled(log.name, log.severity || 'info')) + this._logger.log(log.name, log.severity || 'info', message, args, log.color ? { color: log.color } : {}); + } + + startLaunchRecording() { + this._logger.add(`launch`, new RecordingLogger('browser')); + } + + stopLaunchRecording(): string { + const logger = this._logger.remove(`launch`) as RecordingLogger; + if (logger) + return logger.recording(); + return ''; } } @@ -72,7 +80,55 @@ const colorMap = new Map([ ['reset', 0], ]); -class DebugLoggerSink implements Logger { +class MultiplexingLogger implements Logger { + private _loggers = new Map(); + + add(id: string, logger: Logger) { + this._loggers.set(id, logger); + } + + remove(id: string): Logger | undefined { + const logger = this._loggers.get(id); + this._loggers.delete(id); + return logger; + } + + isEnabled(name: string, severity: LoggerSeverity): boolean { + for (const logger of this._loggers.values()) { + if (logger.isEnabled(name, severity)) + return true; + } + return false; + } + + log(name: string, severity: LoggerSeverity, message: string | Error, args: any[], hints: { color?: string }) { + for (const logger of this._loggers.values()) + logger.log(name, severity, message, args, hints); + } +} + +export class RecordingLogger implements Logger { + private _prefix: string; + private _recording: string[] = []; + + constructor(prefix: string) { + this._prefix = prefix; + } + + isEnabled(name: string, severity: LoggerSeverity): boolean { + return name.startsWith(this._prefix); + } + + log(name: string, severity: LoggerSeverity, message: string | Error, args: any[], hints: { color?: string }) { + this._recording.push(String(message)); + } + + recording(): string { + return this._recording.join('\n'); + } +} + +class DebugLogger implements Logger { private _debuggers = new Map(); isEnabled(name: string, severity: LoggerSeverity): boolean { diff --git a/src/server/browserServer.ts b/src/server/browserServer.ts index 71ca39c6df..1c3d4023f4 100644 --- a/src/server/browserServer.ts +++ b/src/server/browserServer.ts @@ -17,6 +17,9 @@ import { ChildProcess, execSync } from 'child_process'; import { EventEmitter } from 'events'; import { helper } from '../helper'; +import { RootLogger } from '../logger'; +import { TimeoutSettings } from '../timeoutSettings'; +import { LaunchOptionsBase } from './browserType'; export class WebSocketWrapper { readonly wsEndpoint: string; @@ -48,19 +51,32 @@ export class WebSocketWrapper { } export class BrowserServer extends EventEmitter { - private _process: ChildProcess; - private _gracefullyClose: () => Promise; - private _webSocketWrapper: WebSocketWrapper | null; + private _process: ChildProcess | undefined; + private _gracefullyClose: (() => Promise) | undefined; + private _webSocketWrapper: WebSocketWrapper | null = null; + private _launchOptions: LaunchOptionsBase; + readonly _logger: RootLogger; + readonly _launchDeadline: number; - constructor(process: ChildProcess, gracefullyClose: () => Promise, webSocketWrapper: WebSocketWrapper | null) { + constructor(options: LaunchOptionsBase) { super(); + this._launchOptions = options; + 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._webSocketWrapper = webSocketWrapper; } + _isInitialized(): boolean { + return !!this._process; + } + process(): ChildProcess { - return this._process; + return this._process!; } wsEndpoint(): string { @@ -68,12 +84,12 @@ export class BrowserServer extends EventEmitter { } kill() { - if (this._process.pid && !this._process.killed) { + if (this._process!.pid && !this._process!.killed) { try { if (process.platform === 'win32') - execSync(`taskkill /pid ${this._process.pid} /T /F`); + execSync(`taskkill /pid ${this._process!.pid} /T /F`); else - process.kill(-this._process.pid, 'SIGKILL'); + process.kill(-this._process!.pid, 'SIGKILL'); } catch (e) { // the process might have already stopped } @@ -81,7 +97,7 @@ export class BrowserServer extends EventEmitter { } async close(): Promise { - await this._gracefullyClose(); + await this._gracefullyClose!(); } async _checkLeaks(): Promise { @@ -89,19 +105,28 @@ export class BrowserServer extends EventEmitter { await this._webSocketWrapper.checkLeaks(); } - async _initializeOrClose(deadline: number, init: () => Promise): Promise { + async _initializeOrClose(init: () => Promise): Promise { try { - const result = await helper.waitWithDeadline(init(), 'the browser to launch', deadline, 'pw:browser*'); + 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) { - await this._closeOrKill(deadline); + e.message += '\n=============== Process output during launch: ===============\n' + + this._logger.stopLaunchRecording() + + '\n============================================================='; + await this._closeOrKill(); throw e; } } - async _closeOrKill(deadline: number): Promise { + private async _closeOrKill(): Promise { try { - await helper.waitWithDeadline(this.close(), '', deadline, ''); // The error message is ignored. + await helper.waitWithDeadline(this.close(), '', this._launchDeadline, ''); // The error message is ignored. } catch (ignored) { this.kill(); } diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 56f8323956..d33992252a 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -25,7 +25,7 @@ export type BrowserArgOptions = { devtools?: boolean, }; -type LaunchOptionsBase = BrowserArgOptions & { +export type LaunchOptionsBase = BrowserArgOptions & { executablePath?: string, ignoreDefaultArgs?: boolean | string[], handleSIGINT?: boolean, diff --git a/src/server/chromium.ts b/src/server/chromium.ts index a11a6d8cc9..00ba661c1f 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 } from '../helper'; +import { helper, assert, isDebugMode, debugAssert } from '../helper'; import { CRBrowser } from '../chromium/crBrowser'; import * as ws from 'ws'; import { launchProcess } from './processLauncher'; @@ -33,7 +33,6 @@ import { ConnectionTransport, ProtocolRequest, WebSocketTransport } from '../tra import { BrowserContext } from '../browserContext'; import { InnerLogger, logError, RootLogger } from '../logger'; import { BrowserDescriptor } from '../install/browserPaths'; -import { TimeoutSettings } from '../timeoutSettings'; import { CRDevTools } from '../chromium/crDevTools'; export class Chromium extends AbstractBrowserType { @@ -51,12 +50,9 @@ export class Chromium extends AbstractBrowserType { async launch(options: LaunchOptions = {}): Promise { assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); - const { timeout = 30000 } = options; - const deadline = TimeoutSettings.computeDeadline(timeout); - const { browserServer, transport, downloadsPath, logger } = await this._launchServer(options, 'local'); - return await browserServer._initializeOrClose(deadline, async () => { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); + const browserServer = new BrowserServer(options); + const { transport, downloadsPath } = await this._launchServer(options, 'local', browserServer); + return await browserServer._initializeOrClose(async () => { let devtools = this._devtools; if ((options as any).__testHookForDevTools) { devtools = this._createDevTools(); @@ -65,7 +61,7 @@ export class Chromium extends AbstractBrowserType { return await CRBrowser.connect(transport!, { slowMo: options.slowMo, headful: !processBrowserArgOptions(options).headless, - logger, + logger: browserServer._logger, downloadsPath, ownedServer: browserServer, }, devtools); @@ -73,20 +69,19 @@ export class Chromium extends AbstractBrowserType { } async launchServer(options: LaunchServerOptions = {}): Promise { - return (await this._launchServer(options, 'server')).browserServer; + const browserServer = new BrowserServer(options); + await this._launchServer(options, 'server', browserServer); + return browserServer; } async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise { - const { timeout = 30000 } = options; - const deadline = TimeoutSettings.computeDeadline(timeout); - const { transport, browserServer, downloadsPath, logger } = await this._launchServer(options, 'persistent', userDataDir); - return await browserServer._initializeOrClose(deadline, async () => { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); + const browserServer = new BrowserServer(options); + const { transport, downloadsPath } = await this._launchServer(options, 'persistent', browserServer, userDataDir); + return await browserServer._initializeOrClose(async () => { const browser = await CRBrowser.connect(transport!, { slowMo: options.slowMo, persistent: true, - logger, + logger: browserServer._logger, downloadsPath, headful: !processBrowserArgOptions(options).headless, ownedServer: browserServer @@ -98,7 +93,7 @@ export class Chromium extends AbstractBrowserType { }); } - private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, userDataDir?: string): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport, downloadsPath: string, logger: InnerLogger }> { + private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, browserServer: BrowserServer, userDataDir?: string): Promise<{ transport?: ConnectionTransport, downloadsPath: string }> { const { ignoreDefaultArgs = false, args = [], @@ -110,7 +105,7 @@ export class Chromium extends AbstractBrowserType { port = 0, } = options; assert(!port || launchType === 'server', 'Cannot specify a port without launching as a server.'); - const logger = new RootLogger(options.logger); + const logger = browserServer._logger; let temporaryUserDataDir: string | null = null; if (!userDataDir) { @@ -136,7 +131,6 @@ export class Chromium extends AbstractBrowserType { // 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, @@ -148,7 +142,7 @@ export class Chromium extends AbstractBrowserType { pipe: true, tempDir: temporaryUserDataDir || undefined, attemptToGracefullyClose: async () => { - assert(browserServer); + 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. @@ -157,15 +151,14 @@ export class Chromium extends AbstractBrowserType { t.send(message); }, onkill: (exitCode, signal) => { - if (browserServer) - browserServer.emit(Events.BrowserServer.Close, exitCode, signal); + 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 = new BrowserServer(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port) : null); - return { browserServer, transport, downloadsPath, logger }; + browserServer._initialize(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port) : null); + return { transport, downloadsPath }; } async connect(options: ConnectOptions): Promise { diff --git a/src/server/electron.ts b/src/server/electron.ts index 2695624a3c..9f077cc054 100644 --- a/src/server/electron.ts +++ b/src/server/electron.ts @@ -167,9 +167,8 @@ export class Electron { handleSIGINT = true, handleSIGTERM = true, handleSIGHUP = true, - timeout = 30000, } = options; - const deadline = TimeoutSettings.computeDeadline(timeout); + const browserServer = new BrowserServer(options); let app: ElectronApplication | undefined = undefined; @@ -193,6 +192,7 @@ 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 => { @@ -203,7 +203,7 @@ export class Electron { const chromeTransport = await WebSocketTransport.connect(chromeMatch[1], transport => { return transport; }, logger); - const browserServer = new BrowserServer(launchedProcess, gracefullyClose, null); + 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(); diff --git a/src/server/firefox.ts b/src/server/firefox.ts index d5a56e06f9..8ce46cf0e8 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -26,14 +26,13 @@ import { TimeoutError } from '../errors'; import { Events } from '../events'; import { FFBrowser } from '../firefox/ffBrowser'; import { kBrowserCloseMessageId } from '../firefox/ffConnection'; -import { helper, assert } from '../helper'; +import { helper, assert, debugAssert } from '../helper'; import { BrowserServer, WebSocketWrapper } from './browserServer'; import { BrowserArgOptions, LaunchOptions, LaunchServerOptions, ConnectOptions, AbstractBrowserType, processBrowserArgOptions } from './browserType'; import { launchProcess, waitForLine } from './processLauncher'; import { ConnectionTransport, SequenceNumberMixer, WebSocketTransport } from '../transport'; import { RootLogger, InnerLogger, logError } from '../logger'; import { BrowserDescriptor } from '../install/browserPaths'; -import { TimeoutSettings } from '../timeoutSettings'; const mkdtempAsync = util.promisify(fs.mkdtemp); @@ -44,46 +43,42 @@ export class Firefox extends AbstractBrowserType { async launch(options: LaunchOptions = {}): Promise { assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); - const { timeout = 30000 } = options; - const deadline = TimeoutSettings.computeDeadline(timeout); - const { browserServer, downloadsPath, logger } = await this._launchServer(options, 'local'); - return await browserServer._initializeOrClose(deadline, async () => { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); + const browserServer = new BrowserServer(options); + const { downloadsPath } = await this._launchServer(options, 'local', browserServer); + return await browserServer._initializeOrClose(async () => { const browser = await WebSocketTransport.connect(browserServer.wsEndpoint()!, transport => { return FFBrowser.connect(transport, { slowMo: options.slowMo, - logger, + logger: browserServer._logger, downloadsPath, headful: !processBrowserArgOptions(options).headless, ownedServer: browserServer, }); - }, logger); + }, browserServer._logger); return browser; }); } async launchServer(options: LaunchServerOptions = {}): Promise { - return (await this._launchServer(options, 'server')).browserServer; + const browserServer = new BrowserServer(options); + await this._launchServer(options, 'server', browserServer); + return browserServer; } async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise { - const { timeout = 30000 } = options; - const deadline = TimeoutSettings.computeDeadline(timeout); - const { browserServer, downloadsPath, logger } = await this._launchServer(options, 'persistent', userDataDir); - return await browserServer._initializeOrClose(deadline, async () => { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); + const browserServer = new BrowserServer(options); + const { downloadsPath } = await this._launchServer(options, 'persistent', browserServer, userDataDir); + return await browserServer._initializeOrClose(async () => { const browser = await WebSocketTransport.connect(browserServer.wsEndpoint()!, transport => { return FFBrowser.connect(transport, { slowMo: options.slowMo, - logger, + logger: browserServer._logger, persistent: true, downloadsPath, ownedServer: browserServer, headful: !processBrowserArgOptions(options).headless, }); - }, logger); + }, browserServer._logger); const context = browser._defaultContext!; if (!options.ignoreDefaultArgs || Array.isArray(options.ignoreDefaultArgs)) await context._loadDefaultContext(); @@ -91,7 +86,7 @@ export class Firefox extends AbstractBrowserType { }); } - private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, userDataDir?: string): Promise<{ browserServer: BrowserServer, downloadsPath: string, logger: InnerLogger }> { + private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, browserServer: BrowserServer, userDataDir?: string): Promise<{ downloadsPath: string }> { const { ignoreDefaultArgs = false, args = [], @@ -104,7 +99,7 @@ export class Firefox extends AbstractBrowserType { port = 0, } = options; assert(!port || launchType === 'server', 'Cannot specify a port without launching as a server.'); - const logger = new RootLogger(options.logger); + const logger = browserServer._logger; let temporaryProfileDir = null; if (!userDataDir) { @@ -126,7 +121,6 @@ export class Firefox extends AbstractBrowserType { // 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 browserServer: BrowserServer | undefined = undefined; let browserWSEndpoint: string | undefined = undefined; const { launchedProcess, gracefullyClose, downloadsPath } = await launchProcess({ executablePath: firefoxExecutable, @@ -143,15 +137,14 @@ export class Firefox extends AbstractBrowserType { pipe: false, tempDir: temporaryProfileDir || undefined, attemptToGracefullyClose: async () => { - assert(browserServer); + 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 }; await transport.send(message); }, onkill: (exitCode, signal) => { - if (browserServer) - browserServer.emit(Events.BrowserServer.Close, exitCode, signal); + browserServer.emit(Events.BrowserServer.Close, exitCode, signal); }, }); @@ -163,8 +156,8 @@ export class Firefox extends AbstractBrowserType { (await WebSocketTransport.connect(innerEndpoint, t => wrapTransportWithWebSocket(t, logger, port), logger)) : new WebSocketWrapper(innerEndpoint, []); browserWSEndpoint = webSocketWrapper.wsEndpoint; - browserServer = new BrowserServer(launchedProcess, gracefullyClose, webSocketWrapper); - return { browserServer, downloadsPath, logger }; + browserServer._initialize(launchedProcess, gracefullyClose, webSocketWrapper); + return { downloadsPath }; } async connect(options: ConnectOptions): Promise { diff --git a/src/server/processLauncher.ts b/src/server/processLauncher.ts index 6bb64fec84..1dc5d8450d 100644 --- a/src/server/processLauncher.ts +++ b/src/server/processLauncher.ts @@ -16,7 +16,7 @@ */ import * as childProcess from 'child_process'; -import { Log, InnerLogger } from '../logger'; +import { Log, RootLogger } from '../logger'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -44,7 +44,6 @@ const browserStdErrLog: Log = { severity: 'warning' }; - export type LaunchProcessOptions = { executablePath: string, args: string[], @@ -62,7 +61,7 @@ export type LaunchProcessOptions = { // Note: attemptToGracefullyClose should reject if it does not close the browser. attemptToGracefullyClose: () => Promise, onkill: (exitCode: number | null, signal: string | null) => void, - logger: InnerLogger, + logger: RootLogger, }; type LaunchResult = { @@ -74,6 +73,7 @@ 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 60ff0d76ae..36af3f9dc3 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -33,7 +33,6 @@ import { Events } from '../events'; import { BrowserContext } from '../browserContext'; import { InnerLogger, logError, RootLogger } from '../logger'; import { BrowserDescriptor } from '../install/browserPaths'; -import { TimeoutSettings } from '../timeoutSettings'; export class WebKit extends AbstractBrowserType { constructor(packagePath: string, browser: BrowserDescriptor) { @@ -42,16 +41,13 @@ export class WebKit extends AbstractBrowserType { async launch(options: LaunchOptions = {}): Promise { assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead'); - const { timeout = 30000 } = options; - const deadline = TimeoutSettings.computeDeadline(timeout); - const { browserServer, transport, downloadsPath, logger } = await this._launchServer(options, 'local'); - return await browserServer._initializeOrClose(deadline, async () => { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); + const browserServer = new BrowserServer(options); + const { transport, downloadsPath } = await this._launchServer(options, 'local', browserServer); + return await browserServer._initializeOrClose(async () => { return await WKBrowser.connect(transport!, { slowMo: options.slowMo, headful: !processBrowserArgOptions(options).headless, - logger, + logger: browserServer._logger, downloadsPath, ownedServer: browserServer }); @@ -59,20 +55,19 @@ export class WebKit extends AbstractBrowserType { } async launchServer(options: LaunchServerOptions = {}): Promise { - return (await this._launchServer(options, 'server')).browserServer; + const browserServer = new BrowserServer(options); + await this._launchServer(options, 'server', browserServer); + return browserServer; } async launchPersistentContext(userDataDir: string, options: LaunchOptions = {}): Promise { - const { timeout = 30000 } = options; - const deadline = TimeoutSettings.computeDeadline(timeout); - const { transport, browserServer, logger, downloadsPath } = await this._launchServer(options, 'persistent', userDataDir); - return await browserServer._initializeOrClose(deadline, async () => { - if ((options as any).__testHookBeforeCreateBrowser) - await (options as any).__testHookBeforeCreateBrowser(); + const browserServer = new BrowserServer(options); + const { transport, downloadsPath } = await this._launchServer(options, 'persistent', browserServer, userDataDir); + return await browserServer._initializeOrClose(async () => { const browser = await WKBrowser.connect(transport!, { slowMo: options.slowMo, headful: !processBrowserArgOptions(options).headless, - logger, + logger: browserServer._logger, persistent: true, downloadsPath, ownedServer: browserServer @@ -84,7 +79,7 @@ export class WebKit extends AbstractBrowserType { }); } - private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, userDataDir?: string): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport, downloadsPath: string, logger: InnerLogger }> { + private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, browserServer: BrowserServer, userDataDir?: string): Promise<{ transport?: ConnectionTransport, downloadsPath: string, logger: RootLogger }> { const { ignoreDefaultArgs = false, args = [], @@ -96,7 +91,7 @@ export class WebKit extends AbstractBrowserType { port = 0, } = options; assert(!port || launchType === 'server', 'Cannot specify a port without launching as a server.'); - const logger = new RootLogger(options.logger); + const logger = browserServer._logger; let temporaryUserDataDir: string | null = null; if (!userDataDir) { @@ -119,7 +114,6 @@ export class WebKit extends AbstractBrowserType { // 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, @@ -135,18 +129,17 @@ export class WebKit extends AbstractBrowserType { // 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. - await transport.send({method: 'Playwright.close', params: {}, id: kBrowserCloseMessageId}); + transport.send({method: 'Playwright.close', params: {}, id: kBrowserCloseMessageId}); }, onkill: (exitCode, signal) => { - if (browserServer) - browserServer.emit(Events.BrowserServer.Close, exitCode, signal); + 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 = new BrowserServer(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port || 0) : null); - return { browserServer, transport, downloadsPath, logger }; + browserServer._initialize(launchedProcess, gracefullyClose, launchType === 'server' ? wrapTransportWithWebSocket(transport, logger, port || 0) : null); + return { transport, downloadsPath, logger }; } async connect(options: ConnectOptions): Promise { diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index ac8e274956..d6603196b5 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -39,6 +39,7 @@ export class WKBrowser extends BrowserBase { static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise { const browser = new WKBrowser(SlowMoTransport.wrap(transport, options.slowMo), options); + // TODO: add Playwright.enable to test connection. return browser; } diff --git a/test/launcher.spec.js b/test/launcher.spec.js index c18521c51e..90c021cf9d 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -54,7 +54,7 @@ describe('Playwright', function() { it('should handle timeout', async({browserType, defaultBrowserOptions}) => { const options = { ...defaultBrowserOptions, timeout: 5000, __testHookBeforeCreateBrowser: () => new Promise(f => setTimeout(f, 6000)) }; const error = await browserType.launch(options).catch(e => e); - expect(error.message).toBe('Waiting for the browser to launch failed: timeout exceeded. Re-run with the DEBUG=pw:browser* env variable to see the debug log.'); + expect(error.message).toContain('Waiting for the browser to launch failed: timeout exceeded. Re-run with the DEBUG=pw:browser* env variable to see the debug log.'); }); it('should handle exception', async({browserType, defaultBrowserOptions}) => { const e = new Error('Dummy'); @@ -62,6 +62,12 @@ describe('Playwright', function() { const error = await browserType.launch(options).catch(e => e); expect(error).toBe(e); }); + it('should report launch log', async({browserType, defaultBrowserOptions}) => { + const e = new Error('Dummy'); + const options = { ...defaultBrowserOptions, __testHookBeforeCreateBrowser: () => { throw e; }, timeout: 9000 }; + const error = await browserType.launch(options).catch(e => e); + expect(error.message).toContain(''); + }); }); describe('browserType.launchPersistentContext', function() { @@ -102,7 +108,7 @@ describe('Playwright', function() { const userDataDir = await makeUserDataDir(); const options = { ...defaultBrowserOptions, timeout: 5000, __testHookBeforeCreateBrowser: () => new Promise(f => setTimeout(f, 6000)) }; const error = await browserType.launchPersistentContext(userDataDir, options).catch(e => e); - expect(error.message).toBe('Waiting for the browser to launch failed: timeout exceeded. Re-run with the DEBUG=pw:browser* env variable to see the debug log.'); + expect(error.message).toContain('Waiting for the browser to launch failed: timeout exceeded. Re-run with the DEBUG=pw:browser* env variable to see the debug log.'); await removeUserDataDir(userDataDir); }); it('should handle exception', async({browserType, defaultBrowserOptions}) => { diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index 364da83bd6..9e71740d1e 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -213,7 +213,7 @@ function compareDocumentations(actual, expected) { let expectedName = expected.name; for (const replacer of tsReplacers) expectedName = expectedName.replace(...replacer); - if (expectedName !== actualName) + if (normalizeType(expectedName) !== normalizeType(actualName)) errors.push(`${source} ${actualName} != ${expectedName}`); if (actual.name === 'boolean' || actual.name === 'string') return; @@ -304,3 +304,23 @@ function diff(actual, expected) { } } +function normalizeType(type) { + let nesting = 0; + const result = []; + let word = ''; + for (const c of type) { + if (c === '<') { + ++nesting; + } else if (c === '>') { + --nesting; + } + if (c === '|' && !nesting) { + result.push(word); + word = ''; + } else { + word += c; + } + } + result.sort(); + return result.join('|'); +}