From b0c73b72f1aaed14eeeccdd5e82fb7cac44868e9 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 16 Oct 2023 13:13:00 -0700 Subject: [PATCH] chore: push protocol error conversion to dispatcher (#27608) --- packages/playwright-core/src/common/errors.ts | 1 + .../playwright-core/src/server/browserType.ts | 13 ++++-- .../src/server/chromium/chromium.ts | 17 ++++---- .../src/server/chromium/crConnection.ts | 41 ++++++------------- .../src/server/dispatchers/dispatcher.ts | 11 ++++- .../src/server/firefox/ffConnection.ts | 41 ++++++------------- .../src/server/firefox/firefox.ts | 10 +++-- .../src/server/protocolError.ts | 32 +++++++++++---- .../src/server/webkit/webkit.ts | 10 +++-- .../src/server/webkit/wkBrowser.ts | 2 +- .../src/server/webkit/wkConnection.ts | 40 +++++++----------- .../src/server/webkit/wkPage.ts | 4 +- .../src/server/webkit/wkWorkers.ts | 2 +- 13 files changed, 109 insertions(+), 115 deletions(-) diff --git a/packages/playwright-core/src/common/errors.ts b/packages/playwright-core/src/common/errors.ts index 78d7abdec9..12ea1cf6ee 100644 --- a/packages/playwright-core/src/common/errors.ts +++ b/packages/playwright-core/src/common/errors.ts @@ -26,6 +26,7 @@ class CustomError extends Error { export class TimeoutError extends CustomError {} export const kTargetClosedErrorMessage = 'Target page, context or browser has been closed'; +export const kTargetCrashedErrorMessage = 'Target crashed'; export class TargetClosedError extends Error { constructor() { diff --git a/packages/playwright-core/src/server/browserType.ts b/packages/playwright-core/src/server/browserType.ts index 781f6a1d5f..0a7600cd6b 100644 --- a/packages/playwright-core/src/server/browserType.ts +++ b/packages/playwright-core/src/server/browserType.ts @@ -39,6 +39,7 @@ import { RecentLogsCollector } from '../common/debugLogger'; import type { CallMetadata } from './instrumentation'; import { SdkObject } from './instrumentation'; import { ManualPromise } from '../utils/manualPromise'; +import { type ProtocolError, isProtocolError } from './protocolError'; export const kNoXServerRunningError = 'Looks like you launched a headed browser without having a XServer running.\n' + 'Set either \'headless: true\' or use \'xvfb-run \' before running Playwright.\n\n<3 Playwright Team'; @@ -68,7 +69,7 @@ export abstract class BrowserType extends SdkObject { const seleniumHubUrl = (options as any).__testHookSeleniumRemoteURL || process.env.SELENIUM_REMOTE_URL; if (seleniumHubUrl) return this._launchWithSeleniumHub(progress, seleniumHubUrl, options); - return this._innerLaunchWithRetries(progress, options, undefined, helper.debugProtocolLogger(protocolLogger)).catch(e => { throw this._rewriteStartupError(e); }); + return this._innerLaunchWithRetries(progress, options, undefined, helper.debugProtocolLogger(protocolLogger)).catch(e => { throw this._rewriteStartupLog(e); }); }, TimeoutSettings.launchTimeout(options)); return browser; } @@ -79,7 +80,7 @@ export abstract class BrowserType extends SdkObject { const persistent: channels.BrowserNewContextParams = options; controller.setLogName('browser'); const browser = await controller.run(progress => { - return this._innerLaunchWithRetries(progress, options, persistent, helper.debugProtocolLogger(), userDataDir).catch(e => { throw this._rewriteStartupError(e); }); + return this._innerLaunchWithRetries(progress, options, persistent, helper.debugProtocolLogger(), userDataDir).catch(e => { throw this._rewriteStartupLog(e); }); }, TimeoutSettings.launchTimeout(options)); return browser._defaultContext!; } @@ -294,10 +295,16 @@ export abstract class BrowserType extends SdkObject { } } + _rewriteStartupLog(error: Error): Error { + if (!isProtocolError(error)) + return error; + return this._doRewriteStartupLog(error); + } + abstract _defaultArgs(options: types.LaunchOptions, isPersistent: boolean, userDataDir: string): string[]; abstract _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise; abstract _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env; - abstract _rewriteStartupError(error: Error): Error; + abstract _doRewriteStartupLog(error: ProtocolError): ProtocolError; abstract _attemptToGracefullyCloseBrowser(transport: ConnectionTransport): void; } diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index b0206bbe38..f0bf0b7376 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -23,7 +23,6 @@ import { CRBrowser } from './crBrowser'; import type { Env } from '../../utils/processLauncher'; import { gracefullyCloseSet } from '../../utils/processLauncher'; import { kBrowserCloseMessageId } from './crConnection'; -import { rewriteErrorMessage } from '../../utils/stackTrace'; import { BrowserType, kNoXServerRunningError } from '../browserType'; import type { ConnectionTransport, ProtocolRequest } from '../transport'; import { WebSocketTransport } from '../transport'; @@ -49,6 +48,7 @@ import { registry } from '../registry'; import { ManualPromise } from '../../utils/manualPromise'; import { validateBrowserContextOptions } from '../browserContext'; import { chromiumSwitches } from './chromiumSwitches'; +import type { ProtocolError } from '../protocolError'; const ARTIFACTS_FOLDER = path.join(os.tmpdir(), 'playwright-artifacts-'); @@ -139,14 +139,16 @@ export class Chromium extends BrowserType { return CRBrowser.connect(this.attribution.playwright, transport, options, devtools); } - _rewriteStartupError(error: Error): Error { - if (error.message.includes('Missing X server')) - return rewriteErrorMessage(error, '\n' + wrapInASCIIBox(kNoXServerRunningError, 1)); + _doRewriteStartupLog(error: ProtocolError): ProtocolError { + if (!error.logs) + return error; + if (error.logs.includes('Missing X server')) + error.logs = '\n' + wrapInASCIIBox(kNoXServerRunningError, 1); // These error messages are taken from Chromium source code as of July, 2020: // https://github.com/chromium/chromium/blob/70565f67e79f79e17663ad1337dc6e63ee207ce9/content/browser/zygote_host/zygote_host_impl_linux.cc - if (!error.message.includes('crbug.com/357670') && !error.message.includes('No usable sandbox!') && !error.message.includes('crbug.com/638180')) + if (!error.logs.includes('crbug.com/357670') && !error.logs.includes('No usable sandbox!') && !error.logs.includes('crbug.com/638180')) return error; - return rewriteErrorMessage(error, [ + error.logs = [ `Chromium sandboxing failed!`, `================================`, `To workaround sandboxing issues, do either of the following:`, @@ -154,7 +156,8 @@ export class Chromium extends BrowserType { ` - (alternative): Launch Chromium without sandbox using 'chromiumSandbox: false' option`, `================================`, ``, - ].join('\n')); + ].join('\n'); + return error; } _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env { diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index 368970a6db..0f36e280c9 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -19,13 +19,11 @@ import { type RegisteredListener, assert, eventsHelper } from '../../utils'; import type { ConnectionTransport, ProtocolRequest, ProtocolResponse } from '../transport'; import type { Protocol } from './protocol'; import { EventEmitter } from 'events'; -import { rewriteErrorMessage } from '../../utils/stackTrace'; import type { RecentLogsCollector } from '../../common/debugLogger'; import { debugLogger } from '../../common/debugLogger'; import type { ProtocolLogger } from '../types'; import { helper } from '../helper'; import { ProtocolError } from '../protocolError'; -import { kTargetClosedErrorMessage } from '../../common/errors'; export const ConnectionEvents = { Disconnected: Symbol('ConnectionEvents.Disconnected') @@ -102,7 +100,7 @@ type SessionEventListener = (method: string, params?: Object) => void; export class CRSession extends EventEmitter { private readonly _connection: CRConnection; private _eventListener?: SessionEventListener; - private readonly _callbacks = new Map void, reject: (e: ProtocolError) => void, error: ProtocolError, method: string}>(); + private readonly _callbacks = new Map void, reject: (e: ProtocolError) => void, error: ProtocolError }>(); private readonly _sessionId: string; private readonly _parentSession: CRSession | null; private _crashed: boolean = false; @@ -138,25 +136,15 @@ export class CRSession extends EventEmitter { return session; } - private _closedErrorMessage() { - if (this._crashed) - return 'Target crashed'; - if (this._connection._browserDisconnectedLogs) - return kTargetClosedErrorMessage + '\nBrowser logs: ' + this._connection._browserDisconnectedLogs; - if (this._closed || this._connection._closed) - return kTargetClosedErrorMessage; - } - async send( method: T, params?: Protocol.CommandParameters[T] ): Promise { - const closedErrorMessage = this._closedErrorMessage(); - if (closedErrorMessage) - throw new ProtocolError(true, closedErrorMessage); + if (this._crashed || this._closed || this._connection._closed || this._connection._browserDisconnectedLogs) + throw new ProtocolError(this._crashed ? 'crashed' : 'closed', undefined, this._connection._browserDisconnectedLogs); const id = this._connection._rawSend(this._sessionId, method, params); return new Promise((resolve, reject) => { - this._callbacks.set(id, { resolve, reject, error: new ProtocolError(false), method }); + this._callbacks.set(id, { resolve, reject, error: new ProtocolError('error', method) }); }); } @@ -168,10 +156,12 @@ export class CRSession extends EventEmitter { if (object.id && this._callbacks.has(object.id)) { const callback = this._callbacks.get(object.id)!; this._callbacks.delete(object.id); - if (object.error) - callback.reject(createProtocolError(callback.error, callback.method, object.error)); - else + if (object.error) { + callback.error.setMessage(object.error.message); + callback.reject(callback.error); + } else { callback.resolve(object.result); + } } else if (object.id && object.error?.code === -32001) { // Message to a closed session, just ignore it. } else { @@ -199,10 +189,10 @@ export class CRSession extends EventEmitter { dispose() { this._closed = true; this._connection._sessions.delete(this._sessionId); - const errorMessage = this._closedErrorMessage()!; for (const callback of this._callbacks.values()) { - callback.error.sessionClosed = true; - callback.reject(rewriteErrorMessage(callback.error, errorMessage)); + callback.error.type = this._crashed ? 'crashed' : 'closed'; + callback.error.logs = this._connection._browserDisconnectedLogs; + callback.reject(callback.error); } this._callbacks.clear(); } @@ -247,10 +237,3 @@ export class CDPSession extends EventEmitter { this.emit(CDPSession.Events.Closed); } } - -function createProtocolError(error: ProtocolError, method: string, protocolError: { message: string; data: any; }): ProtocolError { - let message = `Protocol error (${method}): ${protocolError.message}`; - if ('data' in protocolError) - message += ` ${protocolError.data}`; - return rewriteErrorMessage(error, message); -} diff --git a/packages/playwright-core/src/server/dispatchers/dispatcher.ts b/packages/playwright-core/src/server/dispatchers/dispatcher.ts index 790e16d328..ab70a6a145 100644 --- a/packages/playwright-core/src/server/dispatchers/dispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/dispatcher.ts @@ -18,14 +18,15 @@ import { EventEmitter } from 'events'; import type * as channels from '@protocol/channels'; import { serializeError } from '../../protocol/serializers'; import { findValidator, ValidationError, createMetadataValidator, type ValidatorContext } from '../../protocol/validator'; -import { assert, isUnderTest, monotonicTime } from '../../utils'; -import { TargetClosedError } from '../../common/errors'; +import { assert, isUnderTest, monotonicTime, rewriteErrorMessage } from '../../utils'; +import { TargetClosedError, kTargetClosedErrorMessage, kTargetCrashedErrorMessage } from '../../common/errors'; import type { CallMetadata } from '../instrumentation'; import { SdkObject } from '../instrumentation'; import type { PlaywrightDispatcher } from './playwrightDispatcher'; import { eventsHelper } from '../..//utils/eventsHelper'; import type { RegisteredListener } from '../..//utils/eventsHelper'; import type * as trace from '@trace/trace'; +import { isProtocolError } from '../protocolError'; export const dispatcherSymbol = Symbol('dispatcher'); const metadataValidator = createMetadataValidator(); @@ -329,6 +330,12 @@ export class DispatcherConnection { const validator = findValidator(dispatcher._type, method, 'Result'); callMetadata.result = validator(result, '', { tChannelImpl: this._tChannelImplToWire.bind(this), binary: this._isLocal ? 'buffer' : 'toBase64' }); } catch (e) { + if (isProtocolError(e)) { + if (e.type === 'closed') + rewriteErrorMessage(e, kTargetClosedErrorMessage + e.browserLogMessage()); + if (e.type === 'crashed') + rewriteErrorMessage(e, kTargetCrashedErrorMessage + e.browserLogMessage()); + } callMetadata.error = serializeError(e); } finally { callMetadata.endTime = monotonicTime(); diff --git a/packages/playwright-core/src/server/firefox/ffConnection.ts b/packages/playwright-core/src/server/firefox/ffConnection.ts index e60a3f1aa3..3d5070440b 100644 --- a/packages/playwright-core/src/server/firefox/ffConnection.ts +++ b/packages/playwright-core/src/server/firefox/ffConnection.ts @@ -18,13 +18,11 @@ import { EventEmitter } from 'events'; import type { ConnectionTransport, ProtocolRequest, ProtocolResponse } from '../transport'; import type { Protocol } from './protocol'; -import { rewriteErrorMessage } from '../../utils/stackTrace'; import type { RecentLogsCollector } from '../../common/debugLogger'; import { debugLogger } from '../../common/debugLogger'; import type { ProtocolLogger } from '../types'; import { helper } from '../helper'; import { ProtocolError } from '../protocolError'; -import { kTargetClosedErrorMessage } from '../../common/errors'; export const ConnectionEvents = { Disconnected: Symbol('Disconnected'), @@ -103,7 +101,7 @@ export class FFConnection extends EventEmitter { export class FFSession extends EventEmitter { _connection: FFConnection; _disposed = false; - private _callbacks: Map; + private _callbacks: Map; private _sessionId: string; private _rawSend: (message: any) => void; private _crashed: boolean = false; @@ -132,26 +130,16 @@ export class FFSession extends EventEmitter { this._crashed = true; } - private _closedErrorMessage() { - if (this._crashed) - return 'Target crashed'; - if (this._connection._browserDisconnectedLogs) - return kTargetClosedErrorMessage + '\nBrowser logs: ' + this._connection._browserDisconnectedLogs; - if (this._disposed || this._connection._closed) - return kTargetClosedErrorMessage; - } - async send( method: T, params?: Protocol.CommandParameters[T] ): Promise { - const closedErrorMessage = this._closedErrorMessage(); - if (closedErrorMessage) - throw new ProtocolError(true, closedErrorMessage); + if (this._crashed || this._disposed || this._connection._closed || this._connection._browserDisconnectedLogs) + throw new ProtocolError(this._crashed ? 'crashed' : 'closed', undefined, this._connection._browserDisconnectedLogs); const id = this._connection.nextMessageId(); this._rawSend({ method, params, id }); return new Promise((resolve, reject) => { - this._callbacks.set(id, { resolve, reject, error: new ProtocolError(false), method }); + this._callbacks.set(id, { resolve, reject, error: new ProtocolError('error', method) }); }); } @@ -165,10 +153,12 @@ export class FFSession extends EventEmitter { // Callbacks could be all rejected if someone has called `.dispose()`. if (callback) { this._callbacks.delete(object.id); - if (object.error) - callback.reject(createProtocolError(callback.error, callback.method, object.error)); - else + if (object.error) { + callback.error.setMessage(object.error.message); + callback.reject(callback.error); + } else { callback.resolve(object.result); + } } } else { Promise.resolve().then(() => this.emit(object.method!, object.params)); @@ -178,18 +168,11 @@ export class FFSession extends EventEmitter { dispose() { this._disposed = true; this._connection._sessions.delete(this._sessionId); - const errorMessage = this._closedErrorMessage()!; for (const callback of this._callbacks.values()) { - callback.error.sessionClosed = true; - callback.reject(rewriteErrorMessage(callback.error, errorMessage)); + callback.error.type = this._crashed ? 'crashed' : 'closed'; + callback.error.logs = this._connection._browserDisconnectedLogs; + callback.reject(callback.error); } this._callbacks.clear(); } } - -function createProtocolError(error: ProtocolError, method: string, protocolError: { message: string; data: any; }): ProtocolError { - let message = `Protocol error (${method}): ${protocolError.message}`; - if ('data' in protocolError) - message += ` ${protocolError.data}`; - return rewriteErrorMessage(error, message); -} diff --git a/packages/playwright-core/src/server/firefox/firefox.ts b/packages/playwright-core/src/server/firefox/firefox.ts index 21f80ca488..90a03b7bf9 100644 --- a/packages/playwright-core/src/server/firefox/firefox.ts +++ b/packages/playwright-core/src/server/firefox/firefox.ts @@ -24,9 +24,9 @@ import type { Env } from '../../utils/processLauncher'; import type { ConnectionTransport } from '../transport'; import type { BrowserOptions } from '../browser'; import type * as types from '../types'; -import { rewriteErrorMessage } from '../../utils/stackTrace'; import { wrapInASCIIBox } from '../../utils'; import type { SdkObject } from '../instrumentation'; +import type { ProtocolError } from '../protocolError'; export class Firefox extends BrowserType { constructor(parent: SdkObject) { @@ -37,9 +37,11 @@ export class Firefox extends BrowserType { return FFBrowser.connect(this.attribution.playwright, transport, options); } - _rewriteStartupError(error: Error): Error { - if (error.message.includes('no DISPLAY environment variable specified')) - return rewriteErrorMessage(error, '\n' + wrapInASCIIBox(kNoXServerRunningError, 1)); + _doRewriteStartupLog(error: ProtocolError): ProtocolError { + if (!error.logs) + return error; + if (error.logs.includes('no DISPLAY environment variable specified')) + error.logs = '\n' + wrapInASCIIBox(kNoXServerRunningError, 1); return error; } diff --git a/packages/playwright-core/src/server/protocolError.ts b/packages/playwright-core/src/server/protocolError.ts index 5d38429d00..1f999b7c64 100644 --- a/packages/playwright-core/src/server/protocolError.ts +++ b/packages/playwright-core/src/server/protocolError.ts @@ -14,15 +14,33 @@ * limitations under the License. */ -export class ProtocolError extends Error { - sessionClosed: boolean; +import { rewriteErrorMessage } from '../utils/stackTrace'; - constructor(sessionClosed: boolean, message?: string) { - super(message); - this.sessionClosed = sessionClosed || false; +export class ProtocolError extends Error { + type: 'error' | 'closed' | 'crashed'; + method: string | undefined; + logs: string | undefined; + + constructor(type: 'error' | 'closed' | 'crashed', method?: string, logs?: string) { + super(); + this.type = type; + this.method = method; + this.logs = logs; + } + + setMessage(message: string) { + rewriteErrorMessage(this, `Protocol error (${this.method}): ${message}`); + } + + browserLogMessage() { + return this.logs ? '\nBrowser logs:\n' + this.logs : ''; } } -export function isSessionClosedError(e: Error): boolean { - return e instanceof ProtocolError && e.sessionClosed; +export function isProtocolError(e: Error): e is ProtocolError { + return e instanceof ProtocolError; +} + +export function isSessionClosedError(e: Error): e is ProtocolError { + return e instanceof ProtocolError && (e.type === 'closed' || e.type === 'crashed'); } diff --git a/packages/playwright-core/src/server/webkit/webkit.ts b/packages/playwright-core/src/server/webkit/webkit.ts index 9f99f2a583..33f4a68c20 100644 --- a/packages/playwright-core/src/server/webkit/webkit.ts +++ b/packages/playwright-core/src/server/webkit/webkit.ts @@ -23,9 +23,9 @@ import { BrowserType, kNoXServerRunningError } from '../browserType'; import type { ConnectionTransport } from '../transport'; import type { BrowserOptions } from '../browser'; import type * as types from '../types'; -import { rewriteErrorMessage } from '../../utils/stackTrace'; import { wrapInASCIIBox } from '../../utils'; import type { SdkObject } from '../instrumentation'; +import type { ProtocolError } from '../protocolError'; export class WebKit extends BrowserType { constructor(parent: SdkObject) { @@ -40,9 +40,11 @@ export class WebKit extends BrowserType { return { ...env, CURL_COOKIE_JAR_PATH: path.join(userDataDir, 'cookiejar.db') }; } - _rewriteStartupError(error: Error): Error { - if (error.message.includes('cannot open display')) - return rewriteErrorMessage(error, '\n' + wrapInASCIIBox(kNoXServerRunningError, 1)); + _doRewriteStartupLog(error: ProtocolError): ProtocolError { + if (!error.logs) + return error; + if (error.logs.includes('cannot open display')) + error.logs = '\n' + wrapInASCIIBox(kNoXServerRunningError, 1); return error; } diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts index 490e7d5aff..a9664e6a82 100644 --- a/packages/playwright-core/src/server/webkit/wkBrowser.ts +++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts @@ -165,7 +165,7 @@ export class WKBrowser extends Browser { context = this._defaultContext as WKBrowserContext; if (!context) return; - const pageProxySession = new WKSession(this._connection, pageProxyId, kTargetClosedErrorMessage, (message: any) => { + const pageProxySession = new WKSession(this._connection, pageProxyId, (message: any) => { this._connection.rawSend({ ...message, pageProxyId }); }); const opener = event.openerId ? this._wkPages.get(event.openerId) : undefined; diff --git a/packages/playwright-core/src/server/webkit/wkConnection.ts b/packages/playwright-core/src/server/webkit/wkConnection.ts index 71cb8435ae..ff92cf5770 100644 --- a/packages/playwright-core/src/server/webkit/wkConnection.ts +++ b/packages/playwright-core/src/server/webkit/wkConnection.ts @@ -19,12 +19,10 @@ import { EventEmitter } from 'events'; import { assert } from '../../utils'; import type { ConnectionTransport, ProtocolRequest, ProtocolResponse } from '../transport'; import type { Protocol } from './protocol'; -import { rewriteErrorMessage } from '../../utils/stackTrace'; import type { RecentLogsCollector } from '../../common/debugLogger'; import { debugLogger } from '../../common/debugLogger'; import type { ProtocolLogger } from '../types'; import { helper } from '../helper'; -import { kTargetClosedErrorMessage } from '../../common/errors'; import { ProtocolError } from '../protocolError'; // WKPlaywright uses this special id to issue Browser.close command which we @@ -51,7 +49,7 @@ export class WKConnection { this._onDisconnect = onDisconnect; this._protocolLogger = protocolLogger; this._browserLogsCollector = browserLogsCollector; - this.browserSession = new WKSession(this, '', kTargetClosedErrorMessage, (message: any) => { + this.browserSession = new WKSession(this, '', (message: any) => { this.rawSend(message); }); this._transport.onmessage = this._dispatchMessage.bind(this); @@ -101,12 +99,11 @@ export class WKConnection { export class WKSession extends EventEmitter { connection: WKConnection; - errorText: string; readonly sessionId: string; private _disposed = false; private readonly _rawSend: (message: any) => void; - private readonly _callbacks = new Map void, reject: (e: ProtocolError) => void, error: ProtocolError, method: string}>(); + private readonly _callbacks = new Map void, reject: (e: ProtocolError) => void, error: ProtocolError }>(); private _crashed: boolean = false; override on: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; @@ -115,13 +112,12 @@ export class WKSession extends EventEmitter { override removeListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; override once: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; - constructor(connection: WKConnection, sessionId: string, errorText: string, rawSend: (message: any) => void) { + constructor(connection: WKConnection, sessionId: string, rawSend: (message: any) => void) { super(); this.setMaxListeners(0); this.connection = connection; this.sessionId = sessionId; this._rawSend = rawSend; - this.errorText = errorText; this.on = super.on; this.off = super.removeListener; @@ -134,15 +130,13 @@ export class WKSession extends EventEmitter { method: T, params?: Protocol.CommandParameters[T] ): Promise { - if (this._crashed) - throw new ProtocolError(true, 'Target crashed'); - if (this._disposed) - throw new ProtocolError(true, kTargetClosedErrorMessage); + if (this._crashed || this._disposed || this.connection._browserDisconnectedLogs) + throw new ProtocolError(this._crashed ? 'crashed' : 'closed', undefined, this.connection._browserDisconnectedLogs); const id = this.connection.nextMessageId(); const messageObj = { id, method, params }; this._rawSend(messageObj); return new Promise((resolve, reject) => { - this._callbacks.set(id, { resolve, reject, error: new ProtocolError(false), method }); + this._callbacks.set(id, { resolve, reject, error: new ProtocolError('error', method) }); }); } @@ -159,11 +153,10 @@ export class WKSession extends EventEmitter { } dispose() { - if (this.connection._browserDisconnectedLogs) - this.errorText = kTargetClosedErrorMessage + '\nBrowser logs: ' + this.connection._browserDisconnectedLogs; for (const callback of this._callbacks.values()) { - callback.error.sessionClosed = true; - callback.reject(rewriteErrorMessage(callback.error, this.errorText)); + callback.error.type = this._crashed ? 'crashed' : 'closed'; + callback.error.logs = this.connection._browserDisconnectedLogs; + callback.reject(callback.error); } this._callbacks.clear(); this._disposed = true; @@ -173,10 +166,12 @@ export class WKSession extends EventEmitter { if (object.id && this._callbacks.has(object.id)) { const callback = this._callbacks.get(object.id)!; this._callbacks.delete(object.id); - if (object.error) - callback.reject(createProtocolError(callback.error, callback.method, object.error)); - else + if (object.error) { + callback.error.setMessage(object.error.message); + callback.reject(callback.error); + } else { callback.resolve(object.result); + } } else if (object.id && !object.error) { // Response might come after session has been disposed and rejected all callbacks. assert(this.isDisposed()); @@ -185,10 +180,3 @@ export class WKSession extends EventEmitter { } } } - -export function createProtocolError(error: ProtocolError, method: string, protocolError: { message: string; data: any; }): ProtocolError { - let message = `Protocol error (${method}): ${protocolError.message}`; - if ('data' in protocolError) - message += ` ${JSON.stringify(protocolError.data)}`; - return rewriteErrorMessage(error, message); -} diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index e93763ee84..d1197d7caf 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -45,7 +45,7 @@ import { WKWorkers } from './wkWorkers'; import { debugLogger } from '../../common/debugLogger'; import { ManualPromise } from '../../utils/manualPromise'; import { BrowserContext } from '../browserContext'; -import { TargetClosedError, kTargetClosedErrorMessage } from '../../common/errors'; +import { TargetClosedError } from '../../common/errors'; const UTILITY_WORLD_NAME = '__playwright_utility_world__'; @@ -304,7 +304,7 @@ export class WKPage implements PageDelegate { private async _onTargetCreated(event: Protocol.Target.targetCreatedPayload) { const { targetInfo } = event; - const session = new WKSession(this._pageProxySession.connection, targetInfo.targetId, kTargetClosedErrorMessage, (message: any) => { + const session = new WKSession(this._pageProxySession.connection, targetInfo.targetId, (message: any) => { this._pageProxySession.send('Target.sendMessageToTarget', { message: JSON.stringify(message), targetId: targetInfo.targetId }).catch(e => { diff --git a/packages/playwright-core/src/server/webkit/wkWorkers.ts b/packages/playwright-core/src/server/webkit/wkWorkers.ts index cb02c8b32a..ae69b72f8a 100644 --- a/packages/playwright-core/src/server/webkit/wkWorkers.ts +++ b/packages/playwright-core/src/server/webkit/wkWorkers.ts @@ -38,7 +38,7 @@ export class WKWorkers { this._sessionListeners = [ eventsHelper.addEventListener(session, 'Worker.workerCreated', (event: Protocol.Worker.workerCreatedPayload) => { const worker = new Worker(this._page, event.url); - const workerSession = new WKSession(session.connection, event.workerId, 'Most likely the worker has been closed.', (message: any) => { + const workerSession = new WKSession(session.connection, event.workerId, (message: any) => { session.send('Worker.sendMessageToWorker', { workerId: event.workerId, message: JSON.stringify(message)