fix(runner): Properly attribute error messages to locator handlers

This commit is contained in:
Adam Gastineau 2024-12-18 11:15:57 -08:00
parent aabbcbf41d
commit 1ac1ca64bf
6 changed files with 111 additions and 28 deletions

View file

@ -25,6 +25,7 @@ import { zones } from '../utils/zones';
import type { ClientInstrumentation } from './clientInstrumentation';
import type { Connection } from './connection';
import type { Logger } from './types';
import { isOverriddenAPIError } from './errors';
type Listener = (...args: any[]) => void;
@ -203,15 +204,18 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
return result;
} catch (e) {
const innerError = ((process.env.PWDEBUGIMPL || isUnderTest()) && e.stack) ? '\n<inner error>\n' + e.stack : '';
if (apiName && !apiName.includes('<anonymous>'))
e.message = apiName + ': ' + e.message;
const computedAPIName = isOverriddenAPIError(e) ? e.apiNameOverride : apiName;
if (computedAPIName && !computedAPIName.includes('<anonymous>'))
e.message = computedAPIName + ': ' + e.message;
const stackFrames = '\n' + stringifyStackFrames(stackTrace.frames).join('\n') + innerError;
if (stackFrames.trim())
e.stack = e.message + stackFrames;
else
e.stack = '';
csi?.onApiCallEnd(callCookie, e);
logApiCall(logger, `<= ${apiName} failed`, isInternal);
logApiCall(logger, `<= ${computedAPIName} failed`, isInternal);
throw e;
}
}

View file

@ -35,6 +35,21 @@ export function isTargetClosedError(error: Error) {
return error instanceof TargetClosedError;
}
export class OverriddenAPIError extends Error {
public apiNameOverride: string;
constructor(error: Exclude<SerializedError['error'], undefined>, apiNameOverride: string) {
super(error.message);
this.name = error.name;
this.stack = error.stack;
this.apiNameOverride = apiNameOverride;
}
}
export function isOverriddenAPIError(error: Error) {
return error instanceof OverriddenAPIError;
}
export function serializeError(e: any): SerializedError {
if (isError(e))
return { error: { message: e.message, stack: e.stack, name: e.name } };
@ -47,6 +62,12 @@ export function parseError(error: SerializedError): Error {
throw new Error('Serialized error must have either an error or a value');
return parseSerializedValue(error.value, undefined);
}
if (error.error.apiNameOverride) {
const e = new OverriddenAPIError(error.error, error.error.apiNameOverride);
e.stack = error.error.stack;
e.name = error.error.name;
return e;
}
if (error.error.name === 'TimeoutError') {
const e = new TimeoutError(error.error.message);
e.stack = error.error.stack || '';

View file

@ -17,6 +17,7 @@
import type { SerializedError } from '@protocol/channels';
import { isError } from '../utils';
import { parseSerializedValue, serializeValue } from '../protocol/serializers';
import { JavaScriptErrorInEvaluate } from './javascript';
class CustomError extends Error {
constructor(message: string) {
@ -37,9 +38,43 @@ export function isTargetClosedError(error: Error) {
return error instanceof TargetClosedError || error.name === 'TargetClosedError';
}
export class OverriddenAPIError extends JavaScriptErrorInEvaluate {
public apiNameOverride: string;
constructor(error: JavaScriptErrorInEvaluate, apiNameOverride: string) {
super(error.message);
this.name = error.name;
this.stack = error.stack;
this.apiNameOverride = apiNameOverride;
}
}
export function isOverriddenAPIError(error: Error) {
return error instanceof OverriddenAPIError;
}
export function serializeError(e: any): SerializedError {
if (isError(e))
return { error: { message: e.message, stack: e.stack, name: e.name } };
if (isError(e)) {
const serializedError: {
error: {
message: any,
stack: any,
name: string,
apiNameOverride?: string;
}
} = {
error: {
message: e.message,
stack: e.stack,
name: e.name
}
};
if (isOverriddenAPIError(e))
serializedError.error.apiNameOverride = e.apiNameOverride;
return serializedError;
}
return { value: serializeValue(e, value => ({ fallThrough: value })) };
}

View file

@ -43,7 +43,7 @@ import type { TimeoutOptions } from '../common/types';
import { isInvalidSelectorError } from '../utils/isomorphic/selectorParser';
import { parseEvaluationResultValue, source } from './isomorphic/utilityScriptSerializers';
import type { SerializedValue } from './isomorphic/utilityScriptSerializers';
import { TargetClosedError, TimeoutError } from './errors';
import { OverriddenAPIError, TargetClosedError, TimeoutError } from './errors';
import { asLocator } from '../utils';
import { helper } from './helper';
@ -494,6 +494,7 @@ export class Page extends SdkObject {
// Do not run locator handlers from inside locator handler callbacks to avoid deadlocks.
if (this._locatorHandlerRunningCounter)
return;
try {
for (const [uid, handler] of this._locatorHandlers) {
if (!handler.resolved) {
if (await this.mainFrame().isVisibleInternal(handler.selector, { strict: true })) {
@ -519,6 +520,11 @@ export class Page extends SdkObject {
progress.log(` interception handler has finished, continuing`);
}
}
} catch (e) {
// Since this checkpoint occurs during the evaluation of a different public API call, any errors are automatically attributed
// to that API call, rather than the locator handler. Mark the error from the locator handler
throw new OverriddenAPIError(e, 'page.addLocatorHandler');
}
}
async emulateMedia(options: Partial<EmulatedMedia>) {

View file

@ -281,6 +281,7 @@ export type SerializedError = {
message: string,
name: string,
stack?: string,
apiNameOverride?: string,
},
value?: SerializedValue,
};

View file

@ -371,3 +371,19 @@ test('should removeLocatorHandler', async ({ page, server }) => {
await expect(page.locator('#interstitial')).toBeVisible();
expect(error.message).toContain('Timeout 3000ms exceeded');
});
test('should attribute errors to the locator handler', async ({ page }) => {
await page.setContent(`<html><body><script>setTimeout(() => {document.body.innerHTML = '<div><ul><li>Foo</li></ul><ul><li>Bar</li></ul></div>'}, 100)</script></body></html>`);
await page.addLocatorHandler(page.locator('ul'), async locator => Promise.resolve());
try {
// Perform an operation that will trigger the locator handler
await page.locator('ul > li').first().boundingBox();
// Unreachable
expect(false).toBeTruthy();
} catch (e) {
expect(e.message).toContain(`page.addLocatorHandler: Error: strict mode violation: locator('ul') resolved to 2 elements:`);
}
});