Skip to content

Commit

Permalink
fix(page): add name property to pageerror event (#5970)
Browse files Browse the repository at this point in the history
  • Loading branch information
mxschmitt committed Apr 19, 2021
1 parent 7dccfd4 commit 8ca58e3
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 17 deletions.
1 change: 1 addition & 0 deletions src/protocol/serializers.ts
Expand Up @@ -36,6 +36,7 @@ export function parseError(error: SerializedError): Error {
}
const e = new Error(error.error.message);
e.stack = error.error.stack || '';
e.name = error.error.name;
return e;
}

Expand Down
13 changes: 7 additions & 6 deletions src/server/chromium/crProtocolHelper.ts
Expand Up @@ -21,6 +21,7 @@ import fs from 'fs';
import * as util from 'util';
import * as types from '../types';
import { mkdirIfNeeded } from '../../utils/utils';
import { splitErrorMessage } from '../../utils/stackTrace';

export function getExceptionMessage(exceptionDetails: Protocol.Runtime.ExceptionDetails): string {
if (exceptionDetails.exception)
Expand Down Expand Up @@ -74,18 +75,18 @@ export function exceptionToError(exceptionDetails: Protocol.Runtime.ExceptionDet
const messageWithStack = getExceptionMessage(exceptionDetails);
const lines = messageWithStack.split('\n');
const firstStackTraceLine = lines.findIndex(line => line.startsWith(' at'));
let message = '';
let messageWithName = '';
let stack = '';
if (firstStackTraceLine === -1) {
message = messageWithStack;
messageWithName = messageWithStack;
} else {
message = lines.slice(0, firstStackTraceLine).join('\n');
messageWithName = lines.slice(0, firstStackTraceLine).join('\n');
stack = messageWithStack;
}
const match = message.match(/^[a-zA-Z0-0_]*Error: (.*)$/);
if (match)
message = match[1];
const {name, message} = splitErrorMessage(messageWithName);

const err = new Error(message);
err.stack = stack;
err.name = name;
return err;
}
5 changes: 4 additions & 1 deletion src/server/firefox/ffPage.ts
Expand Up @@ -30,6 +30,7 @@ import { RawKeyboardImpl, RawMouseImpl, RawTouchscreenImpl } from './ffInput';
import { FFNetworkManager } from './ffNetworkManager';
import { Protocol } from './protocol';
import { Progress } from '../progress';
import { splitErrorMessage } from '../../utils/stackTrace';

const UTILITY_WORLD_NAME = '__playwright_utility_world__';

Expand Down Expand Up @@ -221,9 +222,11 @@ export class FFPage implements PageDelegate {
}

_onUncaughtError(params: Protocol.Page.uncaughtErrorPayload) {
const message = params.message.startsWith('Error: ') ? params.message.substring(7) : params.message;
const {name, message} = splitErrorMessage(params.message);

const error = new Error(message);
error.stack = params.stack;
error.name = name;
this._page.emit(Page.Events.PageError, error);
}

Expand Down
4 changes: 3 additions & 1 deletion src/server/webkit/wkPage.ts
Expand Up @@ -18,6 +18,7 @@
import * as jpeg from 'jpeg-js';
import path from 'path';
import * as png from 'pngjs';
import { splitErrorMessage } from '../../utils/stackTrace';
import { assert, createGuid, debugAssert, headersArrayToObject, headersObjectToArray } from '../../utils/utils';
import * as accessibility from '../accessibility';
import * as dialog from '../dialog';
Expand Down Expand Up @@ -490,7 +491,7 @@ export class WKPage implements PageDelegate {
return;
}
if (level === 'error' && source === 'javascript') {
const message = text.startsWith('Error: ') ? text.substring(7) : text;
const {name, message} = splitErrorMessage(text);
const error = new Error(message);
if (event.message.stackTrace) {
error.stack = event.message.stackTrace.map(callFrame => {
Expand All @@ -499,6 +500,7 @@ export class WKPage implements PageDelegate {
} else {
error.stack = '';
}
error.name = name;
this._page.emit(Page.Events.PageError, error);
return;
}
Expand Down
8 changes: 8 additions & 0 deletions src/utils/stackTrace.ts
Expand Up @@ -69,3 +69,11 @@ export function captureStackTrace(): { stack: string, frames: StackFrame[] } {
}
return { stack, frames };
}

export function splitErrorMessage(message: string): { name: string, message: string } {
const separationIdx = message.indexOf(':');
return {
name: separationIdx !== -1 ? message.slice(0, separationIdx) : '',
message: separationIdx !== -1 && separationIdx + 2 <= message.length ? message.substring(separationIdx + 2) : message,
};
}
42 changes: 33 additions & 9 deletions tests/page-event-pageerror.spec.ts
Expand Up @@ -41,7 +41,37 @@ it('should contain sourceURL', async ({page, server, isWebKit}) => {
expect(error.stack).toContain('myscript.js');
});

it('should handle odd values', async ({page, isFirefox}) => {
it('should contain the Error.name property', async ({ page }) => {
const [error] = await Promise.all([
page.waitForEvent('pageerror'),
page.evaluate(() => {
setTimeout(() => {
const error = new Error('my-message');
error.name = 'my-name';
throw error;
}, 0);
})
]);
expect(error.name).toBe('my-name');
expect(error.message).toBe('my-message');
});

it('should support an empty Error.name property', async ({ page }) => {
const [error] = await Promise.all([
page.waitForEvent('pageerror'),
page.evaluate(() => {
setTimeout(() => {
const error = new Error('my-message');
error.name = '';
throw error;
}, 0);
})
]);
expect(error.name).toBe('');
expect(error.message).toBe('my-message');
});

it('should handle odd values', async ({page}) => {
const cases = [
[null, 'null'],
[undefined, 'undefined'],
Expand All @@ -53,14 +83,11 @@ it('should handle odd values', async ({page, isFirefox}) => {
page.waitForEvent('pageerror'),
page.evaluate(value => setTimeout(() => { throw value; }, 0), value),
]);
expect(error.message).toBe(isFirefox ? 'uncaught exception: ' + message : message);
expect(error.message).toBe(message);
}
});

it('should handle object', async ({page, isChromium, isFirefox}) => {
it.fixme(isFirefox);

// Firefox just does not report this error.
it('should handle object', async ({page, isChromium}) => {
const [error] = await Promise.all([
page.waitForEvent('pageerror'),
page.evaluate(() => setTimeout(() => { throw {}; }, 0)),
Expand All @@ -69,10 +96,7 @@ it('should handle object', async ({page, isChromium, isFirefox}) => {
});

it('should handle window', async ({page, isChromium, isFirefox, isElectron}) => {
it.fixme(isFirefox);
it.skip(isElectron);

// Firefox just does not report this error.
const [error] = await Promise.all([
page.waitForEvent('pageerror'),
page.evaluate(() => setTimeout(() => { throw window; }, 0)),
Expand Down

0 comments on commit 8ca58e3

Please sign in to comment.