Skip to content

Commit 5ee6494

Browse files
pavelfeldmanpavelfeldman
authored
feat(evaluate): return user-readable error from evaluate (#2329)
1 parent 0a8fa6e commit 5ee6494

File tree

6 files changed

+104
-51
lines changed

6 files changed

+104
-51
lines changed

‎src/chromium/crExecutionContext.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,10 @@ export class CRExecutionContext implements js.ExecutionContextDelegate {
4242

4343
async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise<any> {
4444
if (helper.isString(pageFunction)) {
45-
const contextId = this._contextId;
46-
const expression: string = pageFunction;
47-
const {exceptionDetails, result: remoteObject} = await this._client.send('Runtime.evaluate', {
48-
expression: js.ensureSourceUrl(expression),
49-
contextId,
50-
returnByValue,
51-
awaitPromise: true,
52-
userGesture: true
53-
}).catch(rewriteError);
54-
if (exceptionDetails)
55-
throw new Error('Evaluation failed: ' + getExceptionMessage(exceptionDetails));
56-
return returnByValue ? valueFromRemoteObject(remoteObject) : context.createHandle(remoteObject);
45+
return this._callOnUtilityScript(context,
46+
`evaluate`, [
47+
{ value: js.ensureSourceUrl(pageFunction) },
48+
], returnByValue, () => { });
5749
}
5850

5951
if (typeof pageFunction !== 'function')
@@ -81,15 +73,23 @@ export class CRExecutionContext implements js.ExecutionContextDelegate {
8173
return { value };
8274
});
8375

76+
return this._callOnUtilityScript(context,
77+
'callFunction', [
78+
{ value: functionText },
79+
...values.map(value => ({ value })),
80+
...handles,
81+
], returnByValue, dispose);
82+
}
83+
84+
private async _callOnUtilityScript(context: js.ExecutionContext, method: string, args: Protocol.Runtime.CallArgument[], returnByValue: boolean, dispose: () => void) {
8485
try {
8586
const utilityScript = await context.utilityScript();
8687
const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', {
87-
functionDeclaration: `function (...args) { return this.evaluate(...args) }${js.generateSourceUrl()}`,
88+
functionDeclaration: `function (...args) { return this.${method}(...args) }${js.generateSourceUrl()}`,
8889
objectId: utilityScript._remoteObject.objectId,
8990
arguments: [
90-
{ value: functionText },
91-
...values.map(value => ({ value })),
92-
...handles,
91+
{ value: returnByValue },
92+
...args
9393
],
9494
returnByValue,
9595
awaitPromise: true,

‎src/firefox/ffExecutionContext.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,10 @@ export class FFExecutionContext implements js.ExecutionContextDelegate {
4141

4242
async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise<any> {
4343
if (helper.isString(pageFunction)) {
44-
const payload = await this._session.send('Runtime.evaluate', {
45-
expression: js.ensureSourceUrl(pageFunction),
46-
returnByValue,
47-
executionContextId: this._executionContextId,
48-
}).catch(rewriteError);
49-
checkException(payload.exceptionDetails);
50-
if (returnByValue)
51-
return deserializeValue(payload.result!);
52-
return context.createHandle(payload.result);
44+
return this._callOnUtilityScript(context,
45+
`evaluate`, [
46+
{ value: pageFunction },
47+
], returnByValue, () => {});
5348
}
5449
if (typeof pageFunction !== 'function')
5550
throw new Error(`Expected to get |string| or |function| as the first argument, but got "${pageFunction}" instead.`);
@@ -68,15 +63,23 @@ export class FFExecutionContext implements js.ExecutionContextDelegate {
6863
return { value };
6964
});
7065

66+
return this._callOnUtilityScript(context,
67+
`callFunction`, [
68+
{ value: functionText },
69+
...values.map(value => ({ value })),
70+
...handles,
71+
], returnByValue, dispose);
72+
}
73+
74+
private async _callOnUtilityScript(context: js.ExecutionContext, method: string, args: Protocol.Runtime.CallFunctionArgument[], returnByValue: boolean, dispose: () => void) {
7175
try {
7276
const utilityScript = await context.utilityScript();
7377
const payload = await this._session.send('Runtime.callFunction', {
74-
functionDeclaration: `(utilityScript, ...args) => utilityScript.evaluate(...args)`,
78+
functionDeclaration: `(utilityScript, ...args) => utilityScript.${method}(...args)`,
7579
args: [
7680
{ objectId: utilityScript._remoteObject.objectId, value: undefined },
77-
{ value: functionText },
78-
...values.map(value => ({ value })),
79-
...handles,
81+
{ value: returnByValue },
82+
...args
8083
],
8184
returnByValue,
8285
executionContextId: this._executionContextId

‎src/injected/utilityScript.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@
1515
*/
1616

1717
export default class UtilityScript {
18-
evaluate(functionText: string, ...args: any[]) {
18+
evaluate(returnByValue: boolean, expression: string) {
19+
const result = global.eval(expression);
20+
return returnByValue ? this._serialize(result) : result;
21+
}
22+
23+
callFunction(returnByValue: boolean, functionText: string, ...args: any[]) {
1924
const argCount = args[0] as number;
2025
const handleCount = args[argCount + 1] as number;
2126
const handles = { __proto__: null } as any;
@@ -34,6 +39,19 @@ export default class UtilityScript {
3439
for (let i = 0; i < argCount; i++)
3540
processedArgs[i] = visit(args[i + 1]);
3641
const func = global.eval('(' + functionText + ')');
37-
return func(...processedArgs);
42+
const result = func(...processedArgs);
43+
return returnByValue ? this._serialize(result) : result;
44+
}
45+
46+
private _serialize(value: any): any {
47+
if (value instanceof Error) {
48+
const error = value;
49+
if ('captureStackTrace' in global.Error) {
50+
// v8
51+
return error.stack;
52+
}
53+
return `${error.name}: ${error.message}\n${error.stack}`;
54+
}
55+
return value;
3856
}
3957
}

‎src/webkit/wkExecutionContext.ts

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
5555

5656
async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise<any> {
5757
try {
58-
let response = await this._evaluateRemoteObject(context, pageFunction, args);
59-
if (response.result.type === 'object' && response.result.className === 'Promise') {
58+
let response = await this._evaluateRemoteObject(context, pageFunction, args, returnByValue);
59+
if (response.result.objectId && response.result.className === 'Promise') {
6060
response = await Promise.race([
6161
this._executionContextDestroyedPromise.then(() => contextDestroyedResult),
6262
this._session.send('Runtime.awaitPromise', {
@@ -79,14 +79,15 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
7979
}
8080
}
8181

82-
private async _evaluateRemoteObject(context: js.ExecutionContext, pageFunction: Function | string, args: any[]): Promise<any> {
82+
private async _evaluateRemoteObject(context: js.ExecutionContext, pageFunction: Function | string, args: any[], returnByValue: boolean): Promise<Protocol.Runtime.callFunctionOnReturnValue> {
8383
if (helper.isString(pageFunction)) {
84-
const contextId = this._contextId;
85-
const expression: string = pageFunction;
86-
return await this._session.send('Runtime.evaluate', {
87-
expression: js.ensureSourceUrl(expression),
88-
contextId,
89-
returnByValue: false,
84+
const utilityScript = await context.utilityScript();
85+
const functionDeclaration = `function (returnByValue, pageFunction) { return this.evaluate(returnByValue, pageFunction); }${js.generateSourceUrl()}`;
86+
return await this._session.send('Runtime.callFunctionOn', {
87+
functionDeclaration,
88+
objectId: utilityScript._remoteObject.objectId!,
89+
arguments: [ { value: returnByValue }, { value: pageFunction } ],
90+
returnByValue: false, // We need to return real Promise if that is a promise.
9091
emulateUserGesture: true
9192
});
9293
}
@@ -110,22 +111,22 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
110111

111112
try {
112113
const utilityScript = await context.utilityScript();
113-
const callParams = this._serializeFunctionAndArguments(functionText, values, handles);
114+
const callParams = this._serializeFunctionAndArguments(functionText, values, handles, returnByValue);
114115
return await this._session.send('Runtime.callFunctionOn', {
115116
functionDeclaration: callParams.functionText,
116117
objectId: utilityScript._remoteObject.objectId!,
117-
arguments: [ ...callParams.callArguments ],
118-
returnByValue: false,
118+
arguments: callParams.callArguments,
119+
returnByValue: false, // We need to return real Promise if that is a promise.
119120
emulateUserGesture: true
120121
});
121122
} finally {
122123
dispose();
123124
}
124125
}
125126

126-
private _serializeFunctionAndArguments(originalText: string, values: any[], handles: MaybeCallArgument[]): { functionText: string, callArguments: Protocol.Runtime.CallArgument[] } {
127+
private _serializeFunctionAndArguments(originalText: string, values: any[], handles: MaybeCallArgument[], returnByValue: boolean): { functionText: string, callArguments: Protocol.Runtime.CallArgument[]} {
127128
const callArguments: Protocol.Runtime.CallArgument[] = values.map(value => ({ value }));
128-
let functionText = `function (functionText, ...args) { return this.evaluate(functionText, ...args); }${js.generateSourceUrl()}`;
129+
let functionText = `function (returnByValue, functionText, ...args) { return this.callFunction(returnByValue, functionText, ...args); }${js.generateSourceUrl()}`;
129130
if (handles.some(handle => 'unserializable' in handle)) {
130131
const paramStrings = [];
131132
for (let i = 0; i < callArguments.length; i++)
@@ -138,11 +139,11 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
138139
callArguments.push(handle);
139140
}
140141
}
141-
functionText = `function (functionText, ...a) { return this.evaluate(functionText, ${paramStrings.join(',')}); }${js.generateSourceUrl()}`;
142+
functionText = `function (returnByValue, functionText, ...a) { return this.callFunction(returnByValue, functionText, ${paramStrings.join(',')}); }${js.generateSourceUrl()}`;
142143
} else {
143144
callArguments.push(...(handles as Protocol.Runtime.CallArgument[]));
144145
}
145-
return { functionText, callArguments: [ { value: originalText }, ...callArguments ] };
146+
return { functionText, callArguments: [ { value: returnByValue }, { value: originalText }, ...callArguments ] };
146147

147148
function unserializableToString(arg: any) {
148149
if (Object.is(arg, -0))

‎test/chromium/session.spec.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,15 @@ describe('ChromiumBrowserContext.createSession', function() {
4242
// JS coverage enables and then disables Debugger domain.
4343
await page.coverage.startJSCoverage();
4444
await page.coverage.stopJSCoverage();
45+
page.on('console', console.log);
4546
// generate a script in page and wait for the event.
46-
const [event] = await Promise.all([
47-
new Promise(f => client.on('Debugger.scriptParsed', f)),
47+
await Promise.all([
48+
new Promise(f => client.on('Debugger.scriptParsed', event => {
49+
if (event.url === 'foo.js')
50+
f();
51+
})),
4852
page.evaluate('//# sourceURL=foo.js')
4953
]);
50-
// expect events to be dispatched.
51-
expect(event.url).toBe('foo.js');
5254
});
5355
it('should be able to detach session', async function({page, browser, server}) {
5456
const client = await page.context().newCDPSession(page);

‎test/evaluation.spec.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,35 @@ describe('Page.evaluate', function() {
326326
await page.goto(server.EMPTY_PAGE);
327327
expect(await page.evaluate(() => 2 + 2)).toBe(4);
328328
});
329+
it('should evaluate exception', async({page, server}) => {
330+
const error = await page.evaluate(() => {
331+
return (function functionOnStack() {
332+
return new Error('error message');
333+
})();
334+
});
335+
expect(error).toContain('Error: error message');
336+
expect(error).toContain('functionOnStack');
337+
});
338+
it('should evaluate exception', async({page, server}) => {
339+
const error = await page.evaluate(`new Error('error message')`);
340+
expect(error).toContain('Error: error message');
341+
});
342+
it('should evaluate date as {}', async({page}) => {
343+
const result = await page.evaluate(() => ({ date: new Date() }));
344+
expect(result).toEqual({ date: {} });
345+
});
346+
it('should jsonValue() date as {}', async({page}) => {
347+
const resultHandle = await page.evaluateHandle(() => ({ date: new Date() }));
348+
expect(await resultHandle.jsonValue()).toEqual({ date: {} });
349+
});
350+
it.fail(FFOX)('should not use toJSON when evaluating', async({page, server}) => {
351+
const result = await page.evaluate(() => ({ toJSON: () => 'string', data: 'data' }));
352+
expect(result).toEqual({ data: 'data', toJSON: {} });
353+
});
354+
it.fail(FFOX)('should not use toJSON in jsonValue', async({page, server}) => {
355+
const resultHandle = await page.evaluateHandle(() => ({ toJSON: () => 'string', data: 'data' }));
356+
expect(await resultHandle.jsonValue()).toEqual({ data: 'data', toJSON: {} });
357+
});
329358
});
330359

331360
describe('Page.addInitScript', function() {

0 commit comments

Comments
 (0)