Skip to content

chore: make expect throw an error with details instead of returning them #36232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/playwright-core/src/client/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,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 { rewriteErrorMessage } from '../utils/isomorphic/stackTrace';

import type { ClientInstrumentation } from './clientInstrumentation';
Expand Down Expand Up @@ -159,7 +159,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', '<RECV ' + JSON.stringify(message));
Expand All @@ -169,7 +169,12 @@ export class Connection extends EventEmitter {
this._callbacks.delete(id);
if (error && !result) {
const parsedError = parseError(error);
rewriteErrorMessage(parsedError, parsedError.message + formatCallLog(this._platform, log));
const validator = maybeFindValidator(callback.type, callback.method, 'ErrorDetails');
parsedError.log = log || [];
if (validator)
parsedError.details = validator(errorDetails, '', this._validatorFromWireContext());
else
rewriteErrorMessage(parsedError, parsedError.message + formatCallLog(this._platform, log));
callback.reject(parsedError);
} else {
const validator = findValidator(callback.type, callback.method, 'Result');
Expand Down
13 changes: 9 additions & 4 deletions packages/playwright-core/src/client/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,19 @@ import { isError } from '../utils/isomorphic/rtti';

import type { SerializedError } from '@protocol/channels';

export class TimeoutError extends Error {
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');
}
Expand All @@ -42,7 +47,7 @@ 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');
Expand All @@ -58,7 +63,7 @@ export function parseError(error: SerializedError): Error {
e.stack = error.error.stack || '';
return e;
}
const e = new Error(error.error.message);
const e = new PlaywrightError(error.error.message);
e.stack = error.error.stack || '';
e.name = error.error.name;
return e;
Expand Down
24 changes: 18 additions & 6 deletions packages/playwright-core/src/client/locator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { ElementHandle } from './elementHandle';
import { TargetClosedError, TimeoutError } from './errors';
import { parseResult, serializeArgument } from './jsHandle';
import { asLocator } from '../utils/isomorphic/locatorGenerators';
import { getByAltTextSelector, getByLabelSelector, getByPlaceholderSelector, getByRoleSelector, getByTestIdSelector, getByTextSelector, getByTitleSelector } from '../utils/isomorphic/locatorUtils';
Expand Down Expand Up @@ -374,12 +375,23 @@ export class Locator implements api.Locator {
}

async _expect(expression: string, options: FrameExpectParams): Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean }> {
const params: channels.FrameExpectParams = { selector: this._selector, expression, ...options, isNot: !!options.isNot };
params.expectedValue = serializeArgument(options.expectedValue);
const result = (await this._frame._channel.expect(params));
if (result.received !== undefined)
result.received = parseResult(result.received);
return result;
try {
const params: channels.FrameExpectParams = { selector: this._selector, expression, ...options, isNot: !!options.isNot };
params.expectedValue = serializeArgument(options.expectedValue);
const result = await this._frame._channel.expect(params);
if (result.received !== undefined)
result.received = parseResult(result.received);
return { matches: !options.isNot, received: result.received };
} catch (error) {
if (error instanceof TimeoutError || error instanceof TargetClosedError) {
// Timeout error comes with extra details.
const details = error.details as channels.FrameExpectErrorDetails;
const received = details.received !== undefined ? parseResult(details.received) : undefined;
return { matches: options.isNot, received, log: error.log, timedOut: error instanceof TimeoutError };
}
// Any other error, e.g. "invalid selector", should be thrown directly.
throw error;
}
}

private _inspect() {
Expand Down
39 changes: 25 additions & 14 deletions packages/playwright-core/src/client/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { evaluationScript } from './clientHelper';
import { Coverage } from './coverage';
import { Download } from './download';
import { ElementHandle, determineScreenshotType } from './elementHandle';
import { TargetClosedError, isTargetClosedError, serializeError } from './errors';
import { PlaywrightError, TargetClosedError, TimeoutError, isTargetClosedError, serializeError } from './errors';
import { Events } from './events';
import { FileChooser } from './fileChooser';
import { Frame, verifyLoadState } from './frame';
Expand Down Expand Up @@ -598,19 +598,30 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
}

async _expectScreenshot(options: ExpectScreenshotOptions): Promise<{ actual?: Buffer, previous?: Buffer, diff?: Buffer, errorMessage?: string, log?: string[], timedOut?: boolean}> {
const mask = options?.mask ? options?.mask.map(locator => ({
frame: (locator as Locator)._frame._channel,
selector: (locator as Locator)._selector,
})) : undefined;
const locator = options.locator ? {
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,
// This extra wrapApiCall avoids prepending apiName to the error message.
return await this._wrapApiCall(async () => {
try {
const mask = options?.mask ? options?.mask.map(locator => ({
frame: (locator as Locator)._frame._channel,
selector: (locator as Locator)._selector,
})) : undefined;
const locator = options.locator ? {
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,
});
} catch (error) {
if (error instanceof PlaywrightError) {
const details = error.details as channels.PageExpectScreenshotErrorDetails;
return { ...details, errorMessage: error.message, log: error.log, timedOut: error instanceof TimeoutError };
}
throw error;
}
});
}

Expand Down
12 changes: 6 additions & 6 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1300,12 +1300,12 @@ 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),
log: tOptional(tArray(tString)),
});
scheme.PageScreenshotParams = tObject({
timeout: tNumber,
Expand Down Expand Up @@ -1889,10 +1889,10 @@ scheme.FrameExpectParams = tObject({
timeout: tNumber,
});
scheme.FrameExpectResult = tObject({
matches: tBoolean,
received: tOptional(tType('SerializedValue')),
timedOut: tOptional(tBoolean),
log: tOptional(tArray(tString)),
});
scheme.FrameExpectErrorDetails = tObject({
received: tOptional(tType('SerializedValue')),
});
scheme.WorkerInitializer = tObject({
url: tString,
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/protocol/validatorPrimitives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
10 changes: 6 additions & 4 deletions packages/playwright-core/src/server/dispatchers/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { EventEmitter } from 'events';

import { eventsHelper } from '../utils/eventsHelper';
import { ValidationError, createMetadataValidator, findValidator } from '../../protocol/validator';
import { ValidationError, createMetadataValidator, findValidator, maybeFindValidator } from '../../protocol/validator';
import { LongStandingScope, assert, monotonicTime, rewriteErrorMessage } from '../../utils';
import { isUnderTest } from '../utils/debug';
import { TargetClosedError, isTargetClosedError, serializeError } from '../errors';
Expand Down Expand Up @@ -376,9 +376,11 @@ export class DispatcherConnection {
rewriteErrorMessage(e, 'Target crashed ' + e.browserLogMessage());
}
}
response.error = serializeError(e);
// The command handler could have set error in the metadata, do not reset it if there was no exception.
callMetadata.error = response.error;
callMetadata.error = serializeError(e);
response.error = callMetadata.error;
const validator = maybeFindValidator(dispatcher._type, method, 'ErrorDetails');
if (validator)
response.errorDetails = validator(callMetadata.errorDetails || {}, '', this._validatorToWireContext());
} finally {
callMetadata.endTime = monotonicTime();
await sdkObject?.instrumentation.onAfterCall(sdkObject, callMetadata);
Expand Down
14 changes: 10 additions & 4 deletions packages/playwright-core/src/server/dispatchers/frameDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,16 @@ export class FrameDispatcher extends Dispatcher<Frame, channels.FrameChannel, Br
let expectedValue = params.expectedValue ? parseArgument(params.expectedValue) : undefined;
if (params.expression === 'to.match.aria' && expectedValue)
expectedValue = parseAriaSnapshotUnsafe(yaml, expectedValue);
const result = await this._frame.expect(metadata, params.selector, { ...params, expectedValue });
if (result.received !== undefined)
result.received = serializeResult(result.received);
return result;
try {
const result = await this._frame.expect(metadata, params.selector, { ...params, expectedValue });
if (result.received !== undefined)
result.received = serializeResult(result.received);
return result;
} catch (e) {
if (metadata.errorDetails?.received !== undefined)
metadata.errorDetails.received = serializeResult(metadata.errorDetails.received);
throw e;
}
}

async ariaSnapshot(params: channels.FrameAriaSnapshotParams, metadata: CallMetadata): Promise<channels.FrameAriaSnapshotResult> {
Expand Down
Loading
Loading