From d99c3aa182f338a74ca273be03bc335dac8df481 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 15 May 2026 16:12:27 +0100 Subject: [PATCH 1/3] chore(expect): throw typed errors on expect failure Frame.expect and Page.expectScreenshot now throw on mismatch with strongly-typed errorDetails on the wire instead of returning a result object. Frame.expect no longer carries a protocol return value; failure details (received, timedOut, customErrorMessage) travel in a FrameExpectErrorDetails payload next to the response error, and the client rebuilds the matcher's ExpectResult from the rejected PlaywrightError. --- packages/injected/src/injectedScript.ts | 1 - .../playwright-core/src/client/connection.ts | 11 ++- packages/playwright-core/src/client/errors.ts | 34 +++---- packages/playwright-core/src/client/frame.ts | 30 ++++--- packages/playwright-core/src/client/page.ts | 22 +++-- .../playwright-core/src/protocol/validator.ts | 11 +-- .../src/protocol/validatorPrimitives.ts | 4 +- .../src/server/dispatchers/dispatcher.ts | 5 +- .../src/server/dispatchers/frameDispatcher.ts | 31 +++---- packages/playwright-core/src/server/frames.ts | 89 ++++++++++++------- packages/playwright-core/src/server/page.ts | 24 ++--- .../playwright-core/src/server/recorder.ts | 9 +- packages/protocol/spec/frame.yml | 8 +- packages/protocol/spec/page.yml | 3 +- packages/protocol/src/channels.d.ts | 11 +-- tests/library/inspector/pause.spec.ts | 21 +++++ tests/library/trace-viewer.spec.ts | 2 - tests/page/to-match-aria-snapshot.spec.ts | 51 ++++++++--- utils/generate_channels.js | 9 ++ 19 files changed, 238 insertions(+), 138 deletions(-) diff --git a/packages/injected/src/injectedScript.ts b/packages/injected/src/injectedScript.ts index 0b3e9f4e58fc5..ee7a461596193 100644 --- a/packages/injected/src/injectedScript.ts +++ b/packages/injected/src/injectedScript.ts @@ -48,7 +48,6 @@ import type { Builtins } from './utilityScript'; export type FrameExpectParams = Omit & { expectedValue?: any; - noAutoWaiting?: boolean; }; export type ElementState = 'visible' | 'hidden' | 'enabled' | 'disabled' | 'editable' | 'checked' | 'unchecked' | 'indeterminate' | 'stable'; diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index 4e72253fb1f0d..cfe8ff831426c 100644 --- a/packages/playwright-core/src/client/connection.ts +++ b/packages/playwright-core/src/client/connection.ts @@ -15,6 +15,7 @@ */ import { rewriteErrorMessage } from '@isomorphic/stackTrace'; + import { EventEmitter } from './eventEmitter'; import { Android, AndroidDevice, AndroidSocket } from './android'; import { Artifact } from './artifact'; @@ -42,7 +43,7 @@ import { Stream } from './stream'; import { Tracing } from './tracing'; import { Worker } from './worker'; import { WritableStream } from './writableStream'; -import { ValidationError, findValidator } from '../protocol/validator'; +import { ValidationError, findValidator, maybeFindValidator } from '../protocol/validator'; import type { ClientInstrumentation } from './clientInstrumentation'; import type { HeadersArray } from './types'; import type { ValidatorContext } from '../protocol/validator'; @@ -214,7 +215,7 @@ export class Connection extends EventEmitter { if (this._closedError) return; - const { id, guid, method, params, result, error, log } = message as any; + const { id, guid, method, params, result, error, errorDetails, log } = message as any; if (id) { if (this._platform.isLogEnabled('channel')) this._platform.log('channel', 'ErrorDetails` schema. +export class PlaywrightError extends Error { + log: string[] = []; + details?: any; +} + +export class TimeoutError extends PlaywrightError { constructor(message: string) { super(message); this.name = 'TimeoutError'; } } -export class TargetClosedError extends Error { +export class TargetClosedError extends PlaywrightError { constructor(cause?: string) { super(cause || 'Target page, context or browser has been closed'); } @@ -42,24 +51,19 @@ export function serializeError(e: any): SerializedError { return { value: serializeValue(e, value => ({ fallThrough: value })) }; } -export function parseError(error: SerializedError): Error { +export function parseError(error: SerializedError): PlaywrightError { if (!error.error) { if (error.value === undefined) throw new Error('Serialized error must have either an error or a value'); return parseSerializedValue(error.value, undefined); } - if (error.error.name === 'TimeoutError') { - const e = new TimeoutError(error.error.message); - e.stack = error.error.stack || ''; - return e; - } - if (error.error.name === 'TargetClosedError') { - const e = new TargetClosedError(error.error.message); - e.stack = error.error.stack || ''; - return e; - } - const e = new Error(error.error.message); + let e: PlaywrightError; + if (error.error.name === 'TimeoutError') + e = new TimeoutError(error.error.message); + else if (error.error.name === 'TargetClosedError') + e = new TargetClosedError(error.error.message); + else + e = Object.assign(new PlaywrightError(error.error.message), { name: error.error.name }); e.stack = error.error.stack || ''; - e.name = error.error.name; return e; } diff --git a/packages/playwright-core/src/client/frame.ts b/packages/playwright-core/src/client/frame.ts index aaadf17b46ffa..4bf70d2057358 100644 --- a/packages/playwright-core/src/client/frame.ts +++ b/packages/playwright-core/src/client/frame.ts @@ -22,6 +22,7 @@ import { EventEmitter } from './eventEmitter'; import { ChannelOwner } from './channelOwner'; import { addSourceUrlToScript } from './clientHelper'; import { ElementHandle, convertInputFiles, convertSelectOptionValues } from './elementHandle'; +import { PlaywrightError } from './errors'; import { Events } from './events'; import { JSHandle, assertMaxArguments, parseResult, serializeArgument } from './jsHandle'; import { FrameLocator, Locator, testIdAttributeName } from './locator'; @@ -490,20 +491,25 @@ export class Frame extends ChannelOwner implements api.Fr async _expect(expression: string, options: Omit): Promise { const params: channels.FrameExpectParams = { expression, ...options, isNot: !!options.isNot }; params.expectedValue = serializeArgument(options.expectedValue); - const channelResult = await this._channel.expect(params); - const result: ExpectResult = { - matches: channelResult.matches, - log: channelResult.log, - timedOut: channelResult.timedOut, - errorMessage: channelResult.errorMessage, - }; - if (channelResult.received !== undefined && channelResult.matches === !!options.isNot) { - result.received = { - value: channelResult.received.value !== undefined ? parseResult(channelResult.received.value) : undefined, - ariaSnapshot: channelResult.received.ariaSnapshot, + try { + await this._channel.expect(params); + return { matches: !params.isNot }; + } catch (e) { + if (!(e instanceof PlaywrightError) || !e.details) + throw e; + const details = e.details as channels.FrameExpectErrorDetails; + const received = details.received ? { + value: details.received.value !== undefined ? parseResult(details.received.value) : undefined, + ariaSnapshot: details.received.ariaSnapshot, + } : undefined; + return { + matches: !!params.isNot, + received, + log: e.log, + timedOut: details.timedOut, + errorMessage: details.customErrorMessage ? 'Error: ' + details.customErrorMessage : undefined, }; } - return result; } } diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index f2b3811276990..047b6ea9048f4 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -28,7 +28,7 @@ import { Coverage } from './coverage'; import { DisposableObject, DisposableStub } from './disposable'; import { Download } from './download'; import { ElementHandle, determineScreenshotType } from './elementHandle'; -import { TargetClosedError, isTargetClosedError, parseError, serializeError } from './errors'; +import { PlaywrightError, TargetClosedError, isTargetClosedError, parseError, serializeError } from './errors'; import { Events } from './events'; import { FileChooser } from './fileChooser'; import { Frame, verifyLoadState } from './frame'; @@ -625,12 +625,20 @@ export class Page extends ChannelOwner implements api.Page frame: (options.locator as Locator)._frame._channel, selector: (options.locator as Locator)._selector, } : undefined; - return await this._channel.expectScreenshot({ - ...options, - isNot: !!options.isNot, - locator, - mask, - }); + try { + const result = await this._channel.expectScreenshot({ + ...options, + isNot: !!options.isNot, + locator, + mask, + }); + return { actual: result.actual }; + } catch (e) { + if (!(e instanceof PlaywrightError) || !e.details) + throw e; + const details = e.details as channels.PageExpectScreenshotErrorDetails; + return { ...details, errorMessage: e.message }; + } } async title(): Promise { diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index a7f4ffd992310..1d536b6cb6c9b 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -1649,15 +1649,14 @@ scheme.FrameExpectParams = tObject({ isNot: tBoolean, timeout: tFloat, }); -scheme.FrameExpectResult = tObject({ - matches: tBoolean, +scheme.FrameExpectResult = tOptional(tObject({})); +scheme.FrameExpectErrorDetails = tObject({ received: tOptional(tObject({ value: tOptional(tType('SerializedValue')), ariaSnapshot: tOptional(tString), })), timedOut: tOptional(tBoolean), - errorMessage: tOptional(tString), - log: tOptional(tArray(tString)), + customErrorMessage: tOptional(tString), }); scheme.JSHandleInitializer = tObject({ preview: tString, @@ -2412,8 +2411,10 @@ scheme.PageExpectScreenshotParams = tObject({ style: tOptional(tString), }); scheme.PageExpectScreenshotResult = tObject({ + actual: tOptional(tBinary), +}); +scheme.PageExpectScreenshotErrorDetails = tObject({ diff: tOptional(tBinary), - errorMessage: tOptional(tString), actual: tOptional(tBinary), previous: tOptional(tBinary), timedOut: tOptional(tBoolean), diff --git a/packages/playwright-core/src/protocol/validatorPrimitives.ts b/packages/playwright-core/src/protocol/validatorPrimitives.ts index ba15b856d8ada..551497237ae56 100644 --- a/packages/playwright-core/src/protocol/validatorPrimitives.ts +++ b/packages/playwright-core/src/protocol/validatorPrimitives.ts @@ -23,13 +23,13 @@ export type ValidatorContext = { }; export const scheme: { [key: string]: Validator } = {}; -export function findValidator(type: string, method: string, kind: 'Initializer' | 'Event' | 'Params' | 'Result'): Validator { +export function findValidator(type: string, method: string, kind: 'Initializer' | 'Event' | 'Params' | 'Result' | 'ErrorDetails'): Validator { const validator = maybeFindValidator(type, method, kind); if (!validator) throw new ValidationError(`Unknown scheme for ${kind}: ${type}.${method}`); return validator; } -export function maybeFindValidator(type: string, method: string, kind: 'Initializer' | 'Event' | 'Params' | 'Result'): Validator | undefined { +export function maybeFindValidator(type: string, method: string, kind: 'Initializer' | 'Event' | 'Params' | 'Result' | 'ErrorDetails'): Validator | undefined { const schemeName = type + (kind === 'Initializer' ? '' : method[0].toUpperCase() + method.substring(1)) + kind; return scheme[schemeName]; } diff --git a/packages/playwright-core/src/server/dispatchers/dispatcher.ts b/packages/playwright-core/src/server/dispatchers/dispatcher.ts index 7e91fc1e7f7e9..bff31cbddfbd1 100644 --- a/packages/playwright-core/src/server/dispatchers/dispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/dispatcher.ts @@ -22,7 +22,7 @@ import { isUnderTest } from '@utils/debug'; import { assert } from '@isomorphic/assert'; import { monotonicTime } from '@isomorphic/time'; import { rewriteErrorMessage } from '@isomorphic/stackTrace'; -import { ValidationError, createMetadataValidator, createWaitInfoValidator, findValidator } from '../../protocol/validator'; +import { ValidationError, createMetadataValidator, createWaitInfoValidator, findValidator, maybeFindValidator } from '../../protocol/validator'; import { TargetClosedError, isTargetClosedError, serializeError } from '../errors'; import { createRootSdkObject, SdkObject } from '../instrumentation'; import { isProtocolError } from '../protocolError'; @@ -371,6 +371,9 @@ export class DispatcherConnection { rewriteErrorMessage(e, 'Target crashed ' + e.browserLogMessage()); } response.error = serializeError(e); + const detailsValidator = maybeFindValidator(dispatcher._type, method, 'ErrorDetails'); + if (detailsValidator) + response.errorDetails = detailsValidator((e as any)?.details ?? {}, '', this._validatorToWireContext()); // The command handler could have set error in the metadata, do not reset it if there was no exception. callMetadata.error = response.error; } finally { diff --git a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts index 9c038ccc20b4d..a3771e5389cd0 100644 --- a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts @@ -14,10 +14,7 @@ * limitations under the License. */ -import yaml from 'yaml'; -import { parseAriaSnapshotUnsafe } from '@isomorphic/ariaSnapshot'; -import { renderTitleForCall } from '@isomorphic/protocolFormatter'; -import { Frame } from '../frames'; +import { ExpectError, Frame } from '../frames'; import { Dispatcher } from './dispatcher'; import { ElementHandleDispatcher } from './elementHandlerDispatcher'; import { parseArgument, serializeResult } from './jsHandleDispatcher'; @@ -273,23 +270,15 @@ export class FrameDispatcher extends Dispatcher { - let expectedValue = params.expectedValue ? parseArgument(params.expectedValue) : undefined; - if (params.expression === 'to.match.aria' && expectedValue) - expectedValue = parseAriaSnapshotUnsafe(yaml, expectedValue); - progress.log(`${renderTitleForCall(progress.metadata)}${params.timeout ? ` with timeout ${params.timeout}ms` : ''}`); - const result = await this._frame.expect(progress, params.selector, { ...params, expectedValue }); - const channelResult: channels.FrameExpectResult = { - matches: result.matches, - log: result.log, - timedOut: result.timedOut, - errorMessage: result.errorMessage, - }; - if (result.received !== undefined) { - channelResult.received = { - value: result.received.value !== undefined ? serializeResult(result.received.value) : undefined, - ariaSnapshot: result.received.ariaSnapshot, - }; + const expectedValue = params.expectedValue ? parseArgument(params.expectedValue) : undefined; + try { + await this._frame.expect(progress, params.selector, { ...params, expectedValue }); + } catch (e) { + // Without auto-serialization in validators, the received value must be + // serialized into a SerializedValue before it crosses the wire. + if (e instanceof ExpectError && e.details.received && 'value' in e.details.received) + e.details.received.value = serializeResult(e.details.received.value); + throw e; } - return channelResult; } } diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 4afb08a9d07d0..686cc40ce772f 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -15,6 +15,10 @@ * limitations under the License. */ +import yaml from 'yaml'; + +import { parseAriaSnapshotUnsafe } from '@isomorphic/ariaSnapshot'; +import { renderTitleForCall } from '@isomorphic/protocolFormatter'; import { isInvalidSelectorError } from '@isomorphic/selectorParser'; import { ManualPromise } from '@isomorphic/manualPromise'; import { eventsHelper } from '@utils/eventsHelper'; @@ -36,7 +40,6 @@ import { Page, ariaSnapshotForFrame } from './page'; import { isAbortError, nullProgress, ProgressController } from './progress'; import * as types from './types'; import { isSessionClosedError } from './protocolError'; -import { compressCallLog } from './callLog'; import type { ConsoleMessage } from './console'; import type { SelectorInfo } from './frameSelectors'; @@ -95,7 +98,27 @@ export class NavigationAbortedError extends Error { } export type ExpectReceived = { value?: any, ariaSnapshot?: string }; -export type ExpectResult = { matches: boolean, received?: ExpectReceived, log?: string[], timedOut?: boolean, errorMessage?: string }; + +export type ExpectErrorDetails = { + received?: ExpectReceived; + timedOut?: boolean; + customErrorMessage?: string; +}; + +// Thrown by `Frame.expect` on mismatch. `details` is typed against the +// `FrameExpectErrorDetails` schema in protocol.yml; the dispatcher validates +// and serializes it onto `response.errorDetails` for the wire. The error +// message is always a fixed placeholder - any human-readable text travels in +// `details.customErrorMessage` so the client can format it itself. +export class ExpectError extends Error { + readonly details: ExpectErrorDetails; + + constructor(details: ExpectErrorDetails) { + super('Expect failed'); + this.name = 'ExpectError'; + this.details = details; + } +} const kDummyFrameId = ''; @@ -1450,68 +1473,68 @@ export class Frame extends SdkObject { return progress.wait(timeout); } - async expect(progress: Progress, selector: string | undefined, options: FrameExpectParams): Promise { - const lastIntermediateResult: { received?: ExpectReceived, isSet: boolean, errorMessage?: string } = { isSet: false }; - const fixupMetadataError = (result: ExpectResult) => { - // Library mode special case for the expect errors which are return values, not exceptions. - if (result.matches === options.isNot) - progress.metadata.error = { error: { name: 'Expect', message: 'Expect failed' } }; - }; + async expect(progress: Progress, selector: string | undefined, options: FrameExpectParams): Promise { + if (options.expression === 'to.match.aria' && options.expectedValue) { + try { + options = { ...options, expectedValue: parseAriaSnapshotUnsafe(yaml, options.expectedValue) }; + } catch (e) { + // An invalid aria snapshot is a user error - surface it as a typed + // matcher failure instead of letting it escape as a raw rejection. + throw new ExpectError({ customErrorMessage: e.message }); + } + } + progress.log(`${renderTitleForCall(progress.metadata)}${progress.timeout ? ` with timeout ${progress.timeout}ms` : ''}`); + // `isSet` distinguishes "not collected yet" from "collected with received: undefined". + const lastIntermediateResult: { isSet: boolean, received?: ExpectReceived, errorMessage?: string } = { isSet: false }; try { // Step 1: perform locator handlers checkpoint with a specified timeout. if (selector) progress.log(`waiting for ${this._asLocator(selector)}`); - if (!options.noAutoWaiting) - await this._page.performActionPreChecks(progress); + await this._page.performActionPreChecks(progress); // Step 2: perform one-shot expect check without a timeout. // Supports the case of `expect(locator).toBeVisible({ timeout: 1 })` // that should succeed when the locator is already visible. try { const resultOneShot = await this._expectInternal(progress, selector, options, lastIntermediateResult, true); - if (options.noAutoWaiting || resultOneShot.matches !== options.isNot) - return resultOneShot; + if (resultOneShot.matches !== options.isNot) + return; } catch (e) { - if (options.noAutoWaiting || this.isNonRetriableError(e)) + if (this.isNonRetriableError(e)) throw e; // Ignore any other errors from one-shot, we'll handle them during retries. } // Step 3: auto-retry expect with increasing timeouts. Bounded by the total remaining time. - const result = await this.retryWithProgressAndBackoff(progress, async (progress, continuePolling) => { - if (!options.noAutoWaiting) - await this._page.performActionPreChecks(progress); - const { matches, received } = await this._expectInternal(progress, selector, options, lastIntermediateResult, false); + await this.retryWithProgressAndBackoff(progress, async (progress, continuePolling) => { + await this._page.performActionPreChecks(progress); + const { matches } = await this._expectInternal(progress, selector, options, lastIntermediateResult, false); if (matches === options.isNot) { // Keep waiting in these cases: // expect(locator).conditionThatDoesNotMatch // expect(locator).not.conditionThatDoesMatch return continuePolling; } - return { matches, received }; + return true; }); - fixupMetadataError(result); - return result; } catch (e) { - // Q: Why not throw upon isNonRetriableError(e) as in other places? - // A: We want user to receive a friendly message containing the last intermediate result. - const result: ExpectResult = { matches: options.isNot, log: compressCallLog(progress.metadata.log) }; + const details: ExpectErrorDetails = {}; if (isInvalidSelectorError(e)) { - result.errorMessage = 'Error: ' + e.message; + details.customErrorMessage = e.message; } else if (js.isJavaScriptErrorInEvaluate(e)) { - result.errorMessage = e.message; + details.customErrorMessage = e.message.startsWith('Error: ') ? e.message.substring('Error: '.length) : e.message; } else if (lastIntermediateResult.isSet) { - result.received = lastIntermediateResult.received; - result.errorMessage = lastIntermediateResult.errorMessage; + details.received = lastIntermediateResult.received; + if (lastIntermediateResult.errorMessage) + details.customErrorMessage = lastIntermediateResult.errorMessage; } if (e instanceof TimeoutError) - result.timedOut = true; - fixupMetadataError(result); - return result; + details.timedOut = true; + throw new ExpectError(details); } } - private async _expectInternal(progress: Progress, selector: string | undefined, options: FrameExpectParams, lastIntermediateResult: { received?: ExpectReceived, isSet: boolean, errorMessage?: string }, noAbort: boolean) { + private async _expectInternal(progress: Progress, selector: string | undefined, options: FrameExpectParams, lastIntermediateResult: { isSet: boolean, received?: ExpectReceived, errorMessage?: string }, noAbort: boolean) { const progressLog = (text: string) => progress.log(text); // The first expect check, a.k.a. one-shot, always finishes - even when progress is aborted. if (noAbort) @@ -1543,7 +1566,7 @@ export class Frame extends SdkObject { progressLog(log); // Note: missingReceived avoids `unexpected value "undefined"` when element was not found. if (matches === options.isNot) { - lastIntermediateResult.errorMessage = missingReceived ? 'Error: element(s) not found' : undefined; + lastIntermediateResult.errorMessage = missingReceived ? 'element(s) not found' : undefined; lastIntermediateResult.received = received; lastIntermediateResult.isSet = true; if (!missingReceived && !Array.isArray(received?.value)) diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 898adbade9a52..475837ace2c13 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -699,7 +699,7 @@ export class Page extends SdkObject { await this.delegate.updateRequestInterception(); } - async expectScreenshot(progress: Progress, options: ExpectScreenshotOptions): Promise<{ actual?: Buffer, previous?: Buffer, diff?: Buffer, errorMessage?: string, log?: string[], timedOut?: boolean }> { + async expectScreenshot(progress: Progress, options: ExpectScreenshotOptions): Promise<{ actual?: Buffer }> { const locator = options.locator; const rafrafScreenshot = locator ? async (progress: Progress, timeout: number) => { return await locator.frame.rafrafTimeoutScreenshotElementWithProgress(progress, locator.selector, timeout, options || {}); @@ -711,14 +711,10 @@ export class Page extends SdkObject { const comparator = getComparator('image/png'); if (!options.expected && options.isNot) - return { errorMessage: '"not" matcher requires expected result' }; - try { - const format = validateScreenshotOptions(options || {}); - if (format !== 'png') - throw new Error('Only PNG screenshots are supported'); - } catch (error) { - return { errorMessage: error.message }; - } + throw new Error('"not" matcher requires expected result'); + const format = validateScreenshotOptions(options || {}); + if (format !== 'png') + throw new Error('Only PNG screenshots are supported'); let intermediateResult: { actual?: Buffer, previous?: Buffer, @@ -791,12 +787,16 @@ export class Page extends SdkObject { let errorMessage = e.message; if (e instanceof TimeoutError && intermediateResult?.previous) errorMessage = `Failed to take two consecutive stable screenshots.`; - return { + const details: channels.PageExpectScreenshotErrorDetails = { log: compressCallLog(e.message ? [...progress.metadata.log, e.message] : progress.metadata.log), - ...intermediateResult, - errorMessage, + actual: intermediateResult?.actual, + previous: intermediateResult?.previous, + diff: intermediateResult?.diff, timedOut: (e instanceof TimeoutError), }; + const error = new Error(errorMessage); + (error as any).details = details; + throw error; } } diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index e67f56ac505c3..26b3f4e25138b 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -582,9 +582,12 @@ export class Recorder extends EventEmitter implements Instrume private async _performAction(progress: Progress, frame: Frame, action: actions.PerformOnRecordAction) { const actionInContext = await this._createActionInContext(progress, frame, action); this._signalProcessor.addAction(actionInContext); - if (actionInContext.action.name !== 'openPage' && actionInContext.action.name !== 'closePage') - await performAction(progress, this._pageAliases, actionInContext); - actionInContext.endTime = monotonicTime(); + try { + if (actionInContext.action.name !== 'openPage' && actionInContext.action.name !== 'closePage') + await performAction(progress, this._pageAliases, actionInContext); + } finally { + actionInContext.endTime = monotonicTime(); + } } private async _recordAction(progress: Progress, frame: Frame, action: actions.Action) { diff --git a/packages/protocol/spec/frame.yml b/packages/protocol/spec/frame.yml index 4c92922cdfdc8..a2ca0bfd7103b 100644 --- a/packages/protocol/spec/frame.yml +++ b/packages/protocol/spec/frame.yml @@ -766,18 +766,14 @@ Frame: useInnerText: boolean? isNot: boolean timeout: float - returns: - matches: boolean + errorDetails: received: type: object? properties: value: SerializedValue? ariaSnapshot: string? timedOut: boolean? - errorMessage: string? - log: - type: array? - items: string + customErrorMessage: string? flags: snapshot: true pause: true diff --git a/packages/protocol/spec/page.yml b/packages/protocol/spec/page.yml index c1b077c04bf07..a2e3b5371e310 100644 --- a/packages/protocol/spec/page.yml +++ b/packages/protocol/spec/page.yml @@ -189,8 +189,9 @@ Page: clip: Rect? $mixin: CommonScreenshotOptions returns: + actual: binary? + errorDetails: diff: binary? - errorMessage: string? actual: binary? previous: binary? timedOut: boolean? diff --git a/packages/protocol/src/channels.d.ts b/packages/protocol/src/channels.d.ts index f1e8d0c685595..23e255320fb5e 100644 --- a/packages/protocol/src/channels.d.ts +++ b/packages/protocol/src/channels.d.ts @@ -3014,15 +3014,14 @@ export type FrameExpectOptions = { expectedValue?: SerializedArgument, useInnerText?: boolean, }; -export type FrameExpectResult = { - matches: boolean, +export type FrameExpectResult = void; +export type FrameExpectErrorDetails = { received?: { value?: SerializedValue, ariaSnapshot?: string, }, timedOut?: boolean, - errorMessage?: string, - log?: string[], + customErrorMessage?: string, }; export interface FrameEvents { @@ -4309,8 +4308,10 @@ export type PageExpectScreenshotOptions = { style?: string, }; export type PageExpectScreenshotResult = { + actual?: Binary, +}; +export type PageExpectScreenshotErrorDetails = { diff?: Binary, - errorMessage?: string, actual?: Binary, previous?: Binary, timedOut?: boolean, diff --git a/tests/library/inspector/pause.spec.ts b/tests/library/inspector/pause.spec.ts index a7c820343dbb6..74c5dfd61e43a 100644 --- a/tests/library/inspector/pause.spec.ts +++ b/tests/library/inspector/pause.spec.ts @@ -395,6 +395,27 @@ it.describe('pause', () => { expect(error.message).toContain('Not a checkbox or radio button'); }); + it('should populate log with expect failure', async ({ page, recorderPageGetter }) => { + await page.setContent(''); + const scriptPromise = (async () => { + // @ts-ignore + await page.pause({ __testHookKeepTestTimeout: true }); + await expect(page.getByRole('button')).toHaveText('Other', { timeout: 1 }); + })().catch(e => e); + const recorderPage = await recorderPageGetter(); + await recorderPage.click('[title="Resume (F8)"]'); + await recorderPage.waitForSelector('.source-line-error-underline'); + expect(await sanitizeLog(recorderPage)).toEqual([ + 'Pause- XXms', + 'Expect "toHaveText"(page.getByRole(\'button\'))- XXms', + 'Expect "toHaveText" with timeout 1ms', + 'waiting for getByRole(\'button\')', + 'error: Expect failed', + ]); + const error = await scriptPromise; + expect(error.message).toContain('toHaveText'); + }); + it('should populate log with error in waitForEvent', async ({ page, recorderPageGetter }) => { await page.setContent(''); const scriptPromise = (async () => { diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts index 689b6d047184b..c9c3bb8a7fbbe 100644 --- a/tests/library/trace-viewer.spec.ts +++ b/tests/library/trace-viewer.spec.ts @@ -394,8 +394,6 @@ test('should show params and return value', async ({ showTraceViewer }) => { /locator:locator\('button'\)/, /expression:"to.have.text"/, /timeout:10000/, - /matches:true/, - /received:"Click"/, ]); }); diff --git a/tests/page/to-match-aria-snapshot.spec.ts b/tests/page/to-match-aria-snapshot.spec.ts index 0236178bba053..045a7eefaaddf 100644 --- a/tests/page/to-match-aria-snapshot.spec.ts +++ b/tests/page/to-match-aria-snapshot.spec.ts @@ -609,14 +609,21 @@ test('should report error in YAML', async ({ page }) => { const error = await expect(page).toMatchAriaSnapshot(` heading "title" `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Aria snapshot must be a YAML sequence, elements starting with " -"`); + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "heading \\"title\\"" +Error: Aria snapshot must be a YAML sequence, elements starting with " -" +`); } { const error = await expect(page).toMatchAriaSnapshot(` - heading: a: `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Nested mappings are not allowed in compact mappings at line 1, column 12: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading: a:" +Error: Nested mappings are not allowed in compact mappings at line 1, column 12: - heading: a: ^ @@ -631,7 +638,10 @@ test('should report error in YAML keys', async ({ page }) => { const error = await expect(page).toMatchAriaSnapshot(` - heading "title `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Unterminated string: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading \\"title" +Error: Unterminated string: heading "title ^ @@ -642,7 +652,10 @@ heading "title const error = await expect(page).toMatchAriaSnapshot(` - heading /title `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Unterminated regex: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading /title" +Error: Unterminated regex: heading /title ^ @@ -653,7 +666,10 @@ heading /title const error = await expect(page).toMatchAriaSnapshot(` - heading [level=a] `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Value of "level" attribute must be a number: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading [level=a]" +Error: Value of "level" attribute must be a number: heading [level=a] ^ @@ -664,7 +680,10 @@ heading [level=a] const error = await expect(page).toMatchAriaSnapshot(` - heading [expanded=FALSE] `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Value of "expanded" attribute must be a boolean: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading [expanded=FALSE]" +Error: Value of "expanded" attribute must be a boolean: heading [expanded=FALSE] ^ @@ -675,7 +694,10 @@ heading [expanded=FALSE] const error = await expect(page).toMatchAriaSnapshot(` - heading [checked=foo] `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Value of "checked" attribute must be a boolean or "mixed": + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading [checked=foo]" +Error: Value of "checked" attribute must be a boolean or "mixed": heading [checked=foo] ^ @@ -686,7 +708,10 @@ heading [checked=foo] const error = await expect(page).toMatchAriaSnapshot(` - heading [level=] `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Value of "level" attribute must be a number: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading [level=]" +Error: Value of "level" attribute must be a number: heading [level=] ^ @@ -697,7 +722,10 @@ heading [level=] const error = await expect(page).toMatchAriaSnapshot(` - heading [bogus] `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Unsupported attribute [bogus]: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading [bogus]" +Error: Unsupported attribute [bogus]: heading [bogus] ^ @@ -708,7 +736,10 @@ heading [bogus] const error = await expect(page).toMatchAriaSnapshot(` - heading invalid `).catch(e => e); - expect.soft(error.message).toBe(`expect.toMatchAriaSnapshot: Unexpected input: + expect.soft(stripAnsi(error.message)).toBe(`expect(page).toMatchAriaSnapshot(expected) failed + +Expected: "- heading invalid" +Error: Unexpected input: heading invalid ^ diff --git a/utils/generate_channels.js b/utils/generate_channels.js index cd33d24177579..33856d271fb3e 100755 --- a/utils/generate_channels.js +++ b/utils/generate_channels.js @@ -341,6 +341,15 @@ for (const [name, item] of Object.entries(protocol)) { for (const derived of derivedClasses.get(channelName) || []) addScheme(`${derived}${titleCase(methodName)}Result`, `tType('${resultName}')`); + if (method.errorDetails) { + const errorDetailsName = `${channelName}${titleCase(methodName)}ErrorDetails`; + const details = objectType(method.errorDetails, ''); + ts_types.set(errorDetailsName, details.ts); + addScheme(errorDetailsName, details.scheme); + for (const derived of derivedClasses.get(channelName) || []) + addScheme(`${derived}${titleCase(methodName)}ErrorDetails`, `tType('${errorDetailsName}')`); + } + channels_ts.push(` ${methodName}(params${method.parameters ? '' : '?'}: ${paramsName}, progress?: Progress): Promise<${resultName}>;`); } From e181e0f7d674652e601c354c1755b0f8ce8b0ebb Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 18 May 2026 15:58:40 +0100 Subject: [PATCH 2/3] chore(expect): carry expectScreenshot message in customErrorMessage Page.expectScreenshot throws a fixed 'Expect failed' placeholder and puts the human-readable text in errorDetails.customErrorMessage, so the matcher's errorMessage no longer picks up the apiName prefix and call log from the wire error. --- packages/playwright-core/src/client/page.ts | 2 +- packages/playwright-core/src/protocol/validator.ts | 1 + packages/playwright-core/src/server/page.ts | 3 ++- packages/protocol/spec/page.yml | 1 + packages/protocol/src/channels.d.ts | 1 + 5 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index 047b6ea9048f4..183ee7684d35c 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -637,7 +637,7 @@ export class Page extends ChannelOwner implements api.Page if (!(e instanceof PlaywrightError) || !e.details) throw e; const details = e.details as channels.PageExpectScreenshotErrorDetails; - return { ...details, errorMessage: e.message }; + return { ...details, errorMessage: details.customErrorMessage }; } } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 1d536b6cb6c9b..e1d7bf87afd2d 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -2418,6 +2418,7 @@ scheme.PageExpectScreenshotErrorDetails = tObject({ actual: tOptional(tBinary), previous: tOptional(tBinary), timedOut: tOptional(tBoolean), + customErrorMessage: tOptional(tString), log: tOptional(tArray(tString)), }); scheme.PageScreenshotParams = tObject({ diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 475837ace2c13..7be8870ee7f19 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -793,8 +793,9 @@ export class Page extends SdkObject { previous: intermediateResult?.previous, diff: intermediateResult?.diff, timedOut: (e instanceof TimeoutError), + customErrorMessage: errorMessage, }; - const error = new Error(errorMessage); + const error = new Error('Expect failed'); (error as any).details = details; throw error; } diff --git a/packages/protocol/spec/page.yml b/packages/protocol/spec/page.yml index a2e3b5371e310..d37539d8b4dcb 100644 --- a/packages/protocol/spec/page.yml +++ b/packages/protocol/spec/page.yml @@ -195,6 +195,7 @@ Page: actual: binary? previous: binary? timedOut: boolean? + customErrorMessage: string? log: type: array? items: string diff --git a/packages/protocol/src/channels.d.ts b/packages/protocol/src/channels.d.ts index 23e255320fb5e..903b7efc8a9b0 100644 --- a/packages/protocol/src/channels.d.ts +++ b/packages/protocol/src/channels.d.ts @@ -4315,6 +4315,7 @@ export type PageExpectScreenshotErrorDetails = { actual?: Binary, previous?: Binary, timedOut?: boolean, + customErrorMessage?: string, log?: string[], }; export type PageScreenshotParams = { From 19c0ee764808490838ac4f76d4cf6a78aba4bee7 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 19 May 2026 14:17:35 +0100 Subject: [PATCH 3/3] chore(expect): always carry typed error details across the wire Validate ErrorDetails unconditionally on the client (with {} fallback) and wrap expectScreenshot pre-check failures so they surface as typed matcher errors instead of escaping as raw rejections. --- .../playwright-core/src/client/connection.ts | 9 ++---- packages/playwright-core/src/client/errors.ts | 6 +--- packages/playwright-core/src/client/frame.ts | 2 +- packages/playwright-core/src/client/page.ts | 2 +- .../playwright-core/src/protocol/validator.ts | 2 +- .../src/server/dispatchers/frameDispatcher.ts | 4 +-- packages/playwright-core/src/server/frames.ts | 14 +++------ packages/playwright-core/src/server/page.ts | 10 +++---- packages/protocol/spec/page.yml | 2 +- packages/protocol/src/channels.d.ts | 2 +- tests/page/to-match-aria-snapshot.spec.ts | 30 +++++++++++++++++++ 11 files changed, 50 insertions(+), 33 deletions(-) diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index cfe8ff831426c..01c04284bbf1f 100644 --- a/packages/playwright-core/src/client/connection.ts +++ b/packages/playwright-core/src/client/connection.ts @@ -15,7 +15,6 @@ */ import { rewriteErrorMessage } from '@isomorphic/stackTrace'; - import { EventEmitter } from './eventEmitter'; import { Android, AndroidDevice, AndroidSocket } from './android'; import { Artifact } from './artifact'; @@ -227,11 +226,9 @@ export class Connection extends EventEmitter { const parsedError = parseError(error); parsedError.log = log || []; rewriteErrorMessage(parsedError, parsedError.message + formatCallLog(this._platform, log)); - if (errorDetails !== undefined) { - const detailsValidator = maybeFindValidator(callback.type, callback.method, 'ErrorDetails'); - if (detailsValidator) - parsedError.details = detailsValidator(errorDetails, '', this._validatorFromWireContext()); - } + const detailsValidator = maybeFindValidator(callback.type, callback.method, 'ErrorDetails'); + if (detailsValidator) + parsedError.details = detailsValidator(errorDetails ?? {}, '', this._validatorFromWireContext()); callback.reject(parsedError); } else { const validator = findValidator(callback.type, callback.method, 'Result'); diff --git a/packages/playwright-core/src/client/errors.ts b/packages/playwright-core/src/client/errors.ts index c7fcaf1388645..ce4869d00d6e5 100644 --- a/packages/playwright-core/src/client/errors.ts +++ b/packages/playwright-core/src/client/errors.ts @@ -19,13 +19,9 @@ import { parseSerializedValue, serializeValue } from '../protocol/serializers'; import type { SerializedError } from '@protocol/channels'; -// Base class for errors that have crossed the wire. `log` is always populated -// from the response; `details` is set by `Connection.dispatch` when the -// response carries a `errorDetails` payload validated against the protocol's -// `ErrorDetails` schema. export class PlaywrightError extends Error { log: string[] = []; - details?: any; + details?: any; // As declared in the protocol. } export class TimeoutError extends PlaywrightError { diff --git a/packages/playwright-core/src/client/frame.ts b/packages/playwright-core/src/client/frame.ts index 4bf70d2057358..bb116238ea5cf 100644 --- a/packages/playwright-core/src/client/frame.ts +++ b/packages/playwright-core/src/client/frame.ts @@ -495,7 +495,7 @@ export class Frame extends ChannelOwner implements api.Fr await this._channel.expect(params); return { matches: !params.isNot }; } catch (e) { - if (!(e instanceof PlaywrightError) || !e.details) + if (!(e instanceof PlaywrightError)) throw e; const details = e.details as channels.FrameExpectErrorDetails; const received = details.received ? { diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index 183ee7684d35c..6da37f9f5b1b7 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -634,7 +634,7 @@ export class Page extends ChannelOwner implements api.Page }); return { actual: result.actual }; } catch (e) { - if (!(e instanceof PlaywrightError) || !e.details) + if (!(e instanceof PlaywrightError)) throw e; const details = e.details as channels.PageExpectScreenshotErrorDetails; return { ...details, errorMessage: details.customErrorMessage }; diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index e1d7bf87afd2d..0b952f6cc2367 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -2415,10 +2415,10 @@ scheme.PageExpectScreenshotResult = tObject({ }); scheme.PageExpectScreenshotErrorDetails = tObject({ diff: tOptional(tBinary), + customErrorMessage: tOptional(tString), actual: tOptional(tBinary), previous: tOptional(tBinary), timedOut: tOptional(tBoolean), - customErrorMessage: tOptional(tString), log: tOptional(tArray(tString)), }); scheme.PageScreenshotParams = tObject({ diff --git a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts index a3771e5389cd0..8c7af45a22d1c 100644 --- a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { renderTitleForCall } from '@isomorphic/protocolFormatter'; import { ExpectError, Frame } from '../frames'; import { Dispatcher } from './dispatcher'; import { ElementHandleDispatcher } from './elementHandlerDispatcher'; @@ -270,12 +271,11 @@ export class FrameDispatcher extends Dispatcher { + progress.log(`${renderTitleForCall(progress.metadata)}${params.timeout ? ` with timeout ${params.timeout}ms` : ''}`); const expectedValue = params.expectedValue ? parseArgument(params.expectedValue) : undefined; try { await this._frame.expect(progress, params.selector, { ...params, expectedValue }); } catch (e) { - // Without auto-serialization in validators, the received value must be - // serialized into a SerializedValue before it crosses the wire. if (e instanceof ExpectError && e.details.received && 'value' in e.details.received) e.details.received.value = serializeResult(e.details.received.value); throw e; diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 686cc40ce772f..7f2556d988f23 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -16,9 +16,7 @@ */ import yaml from 'yaml'; - import { parseAriaSnapshotUnsafe } from '@isomorphic/ariaSnapshot'; -import { renderTitleForCall } from '@isomorphic/protocolFormatter'; import { isInvalidSelectorError } from '@isomorphic/selectorParser'; import { ManualPromise } from '@isomorphic/manualPromise'; import { eventsHelper } from '@utils/eventsHelper'; @@ -1478,12 +1476,9 @@ export class Frame extends SdkObject { try { options = { ...options, expectedValue: parseAriaSnapshotUnsafe(yaml, options.expectedValue) }; } catch (e) { - // An invalid aria snapshot is a user error - surface it as a typed - // matcher failure instead of letting it escape as a raw rejection. throw new ExpectError({ customErrorMessage: e.message }); } } - progress.log(`${renderTitleForCall(progress.metadata)}${progress.timeout ? ` with timeout ${progress.timeout}ms` : ''}`); // `isSet` distinguishes "not collected yet" from "collected with received: undefined". const lastIntermediateResult: { isSet: boolean, received?: ExpectReceived, errorMessage?: string } = { isSet: false }; try { @@ -1508,14 +1503,14 @@ export class Frame extends SdkObject { // Step 3: auto-retry expect with increasing timeouts. Bounded by the total remaining time. await this.retryWithProgressAndBackoff(progress, async (progress, continuePolling) => { await this._page.performActionPreChecks(progress); - const { matches } = await this._expectInternal(progress, selector, options, lastIntermediateResult, false); + const { matches, received } = await this._expectInternal(progress, selector, options, lastIntermediateResult, false); if (matches === options.isNot) { // Keep waiting in these cases: // expect(locator).conditionThatDoesNotMatch // expect(locator).not.conditionThatDoesMatch return continuePolling; } - return true; + return { matches, received }; }); } catch (e) { const details: ExpectErrorDetails = {}; @@ -1525,8 +1520,7 @@ export class Frame extends SdkObject { details.customErrorMessage = e.message.startsWith('Error: ') ? e.message.substring('Error: '.length) : e.message; } else if (lastIntermediateResult.isSet) { details.received = lastIntermediateResult.received; - if (lastIntermediateResult.errorMessage) - details.customErrorMessage = lastIntermediateResult.errorMessage; + details.customErrorMessage = lastIntermediateResult.errorMessage; } if (e instanceof TimeoutError) details.timedOut = true; @@ -1534,7 +1528,7 @@ export class Frame extends SdkObject { } } - private async _expectInternal(progress: Progress, selector: string | undefined, options: FrameExpectParams, lastIntermediateResult: { isSet: boolean, received?: ExpectReceived, errorMessage?: string }, noAbort: boolean) { + private async _expectInternal(progress: Progress, selector: string | undefined, options: FrameExpectParams, lastIntermediateResult: { received?: ExpectReceived, isSet: boolean, errorMessage?: string }, noAbort: boolean) { const progressLog = (text: string) => progress.log(text); // The first expect check, a.k.a. one-shot, always finishes - even when progress is aborted. if (noAbort) diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 7be8870ee7f19..af5cde5cb8a0b 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -710,11 +710,6 @@ export class Page extends SdkObject { }; const comparator = getComparator('image/png'); - if (!options.expected && options.isNot) - throw new Error('"not" matcher requires expected result'); - const format = validateScreenshotOptions(options || {}); - if (format !== 'png') - throw new Error('Only PNG screenshots are supported'); let intermediateResult: { actual?: Buffer, previous?: Buffer, @@ -731,6 +726,11 @@ export class Page extends SdkObject { }; try { + if (!options.expected && options.isNot) + throw new Error('"not" matcher requires expected result'); + const format = validateScreenshotOptions(options || {}); + if (format !== 'png') + throw new Error('Only PNG screenshots are supported'); let actual: Buffer | undefined; let previous: Buffer | undefined; const pollIntervals = [0, 100, 250, 500]; diff --git a/packages/protocol/spec/page.yml b/packages/protocol/spec/page.yml index d37539d8b4dcb..73a3adc21810b 100644 --- a/packages/protocol/spec/page.yml +++ b/packages/protocol/spec/page.yml @@ -192,10 +192,10 @@ Page: actual: binary? errorDetails: diff: binary? + customErrorMessage: string? actual: binary? previous: binary? timedOut: boolean? - customErrorMessage: string? log: type: array? items: string diff --git a/packages/protocol/src/channels.d.ts b/packages/protocol/src/channels.d.ts index 903b7efc8a9b0..b6e965ca7d523 100644 --- a/packages/protocol/src/channels.d.ts +++ b/packages/protocol/src/channels.d.ts @@ -4312,10 +4312,10 @@ export type PageExpectScreenshotResult = { }; export type PageExpectScreenshotErrorDetails = { diff?: Binary, + customErrorMessage?: string, actual?: Binary, previous?: Binary, timedOut?: boolean, - customErrorMessage?: string, log?: string[], }; export type PageScreenshotParams = { diff --git a/tests/page/to-match-aria-snapshot.spec.ts b/tests/page/to-match-aria-snapshot.spec.ts index 045a7eefaaddf..221ddf1b4692b 100644 --- a/tests/page/to-match-aria-snapshot.spec.ts +++ b/tests/page/to-match-aria-snapshot.spec.ts @@ -613,6 +613,9 @@ test('should report error in YAML', async ({ page }) => { Expected: "heading \\"title\\"" Error: Aria snapshot must be a YAML sequence, elements starting with " -" + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -627,6 +630,9 @@ Error: Nested mappings are not allowed in compact mappings at line 1, column 12: - heading: a: ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } }); @@ -645,6 +651,9 @@ Error: Unterminated string: heading "title ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -659,6 +668,9 @@ Error: Unterminated regex: heading /title ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -673,6 +685,9 @@ Error: Value of "level" attribute must be a number: heading [level=a] ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -687,6 +702,9 @@ Error: Value of "expanded" attribute must be a boolean: heading [expanded=FALSE] ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -701,6 +719,9 @@ Error: Value of "checked" attribute must be a boolean or "mixed": heading [checked=foo] ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -715,6 +736,9 @@ Error: Value of "level" attribute must be a number: heading [level=] ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -729,6 +753,9 @@ Error: Unsupported attribute [bogus]: heading [bogus] ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } @@ -743,6 +770,9 @@ Error: Unexpected input: heading invalid ^ + +Call log: + - Expect "toMatchAriaSnapshot" with timeout 10000ms `); } });