From 5ee6494032ecb532c67179dbf5e52e4b2a1fd0de Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 21 May 2020 16:00:55 -0700 Subject: [PATCH] feat(evaluate): return user-readable error from evaluate (#2329) --- src/chromium/crExecutionContext.ts | 36 +++++++++++++++--------------- src/firefox/ffExecutionContext.ts | 33 ++++++++++++++------------- src/injected/utilityScript.ts | 22 ++++++++++++++++-- src/webkit/wkExecutionContext.ts | 33 ++++++++++++++------------- test/chromium/session.spec.js | 10 +++++---- test/evaluation.spec.js | 29 ++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 55 deletions(-) diff --git a/src/chromium/crExecutionContext.ts b/src/chromium/crExecutionContext.ts index 3e82a05a83..aa07c130df 100644 --- a/src/chromium/crExecutionContext.ts +++ b/src/chromium/crExecutionContext.ts @@ -42,18 +42,10 @@ export class CRExecutionContext implements js.ExecutionContextDelegate { async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { if (helper.isString(pageFunction)) { - const contextId = this._contextId; - const expression: string = pageFunction; - const {exceptionDetails, result: remoteObject} = await this._client.send('Runtime.evaluate', { - expression: js.ensureSourceUrl(expression), - contextId, - returnByValue, - awaitPromise: true, - userGesture: true - }).catch(rewriteError); - if (exceptionDetails) - throw new Error('Evaluation failed: ' + getExceptionMessage(exceptionDetails)); - return returnByValue ? valueFromRemoteObject(remoteObject) : context.createHandle(remoteObject); + return this._callOnUtilityScript(context, + `evaluate`, [ + { value: js.ensureSourceUrl(pageFunction) }, + ], returnByValue, () => { }); } if (typeof pageFunction !== 'function') @@ -81,15 +73,23 @@ export class CRExecutionContext implements js.ExecutionContextDelegate { return { value }; }); - try { - const utilityScript = await context.utilityScript(); - const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', { - functionDeclaration: `function (...args) { return this.evaluate(...args) }${js.generateSourceUrl()}`, - objectId: utilityScript._remoteObject.objectId, - arguments: [ + return this._callOnUtilityScript(context, + 'callFunction', [ { value: functionText }, ...values.map(value => ({ value })), ...handles, + ], returnByValue, dispose); + } + + private async _callOnUtilityScript(context: js.ExecutionContext, method: string, args: Protocol.Runtime.CallArgument[], returnByValue: boolean, dispose: () => void) { + try { + const utilityScript = await context.utilityScript(); + const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', { + functionDeclaration: `function (...args) { return this.${method}(...args) }${js.generateSourceUrl()}`, + objectId: utilityScript._remoteObject.objectId, + arguments: [ + { value: returnByValue }, + ...args ], returnByValue, awaitPromise: true, diff --git a/src/firefox/ffExecutionContext.ts b/src/firefox/ffExecutionContext.ts index 366a70581e..80420c30d5 100644 --- a/src/firefox/ffExecutionContext.ts +++ b/src/firefox/ffExecutionContext.ts @@ -41,15 +41,10 @@ export class FFExecutionContext implements js.ExecutionContextDelegate { async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { if (helper.isString(pageFunction)) { - const payload = await this._session.send('Runtime.evaluate', { - expression: js.ensureSourceUrl(pageFunction), - returnByValue, - executionContextId: this._executionContextId, - }).catch(rewriteError); - checkException(payload.exceptionDetails); - if (returnByValue) - return deserializeValue(payload.result!); - return context.createHandle(payload.result); + return this._callOnUtilityScript(context, + `evaluate`, [ + { value: pageFunction }, + ], returnByValue, () => {}); } if (typeof pageFunction !== 'function') 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 { return { value }; }); - try { - const utilityScript = await context.utilityScript(); - const payload = await this._session.send('Runtime.callFunction', { - functionDeclaration: `(utilityScript, ...args) => utilityScript.evaluate(...args)`, - args: [ - { objectId: utilityScript._remoteObject.objectId, value: undefined }, + return this._callOnUtilityScript(context, + `callFunction`, [ { value: functionText }, ...values.map(value => ({ value })), ...handles, + ], returnByValue, dispose); + } + + private async _callOnUtilityScript(context: js.ExecutionContext, method: string, args: Protocol.Runtime.CallFunctionArgument[], returnByValue: boolean, dispose: () => void) { + try { + const utilityScript = await context.utilityScript(); + const payload = await this._session.send('Runtime.callFunction', { + functionDeclaration: `(utilityScript, ...args) => utilityScript.${method}(...args)`, + args: [ + { objectId: utilityScript._remoteObject.objectId, value: undefined }, + { value: returnByValue }, + ...args ], returnByValue, executionContextId: this._executionContextId diff --git a/src/injected/utilityScript.ts b/src/injected/utilityScript.ts index b86b7f03fe..d216abb856 100644 --- a/src/injected/utilityScript.ts +++ b/src/injected/utilityScript.ts @@ -15,7 +15,12 @@ */ export default class UtilityScript { - evaluate(functionText: string, ...args: any[]) { + evaluate(returnByValue: boolean, expression: string) { + const result = global.eval(expression); + return returnByValue ? this._serialize(result) : result; + } + + callFunction(returnByValue: boolean, functionText: string, ...args: any[]) { const argCount = args[0] as number; const handleCount = args[argCount + 1] as number; const handles = { __proto__: null } as any; @@ -34,6 +39,19 @@ export default class UtilityScript { for (let i = 0; i < argCount; i++) processedArgs[i] = visit(args[i + 1]); const func = global.eval('(' + functionText + ')'); - return func(...processedArgs); + const result = func(...processedArgs); + return returnByValue ? this._serialize(result) : result; + } + + private _serialize(value: any): any { + if (value instanceof Error) { + const error = value; + if ('captureStackTrace' in global.Error) { + // v8 + return error.stack; + } + return `${error.name}: ${error.message}\n${error.stack}`; + } + return value; } } diff --git a/src/webkit/wkExecutionContext.ts b/src/webkit/wkExecutionContext.ts index 62396e8b4d..b79a7fa994 100644 --- a/src/webkit/wkExecutionContext.ts +++ b/src/webkit/wkExecutionContext.ts @@ -55,8 +55,8 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { try { - let response = await this._evaluateRemoteObject(context, pageFunction, args); - if (response.result.type === 'object' && response.result.className === 'Promise') { + let response = await this._evaluateRemoteObject(context, pageFunction, args, returnByValue); + if (response.result.objectId && response.result.className === 'Promise') { response = await Promise.race([ this._executionContextDestroyedPromise.then(() => contextDestroyedResult), this._session.send('Runtime.awaitPromise', { @@ -79,14 +79,15 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { } } - private async _evaluateRemoteObject(context: js.ExecutionContext, pageFunction: Function | string, args: any[]): Promise { + private async _evaluateRemoteObject(context: js.ExecutionContext, pageFunction: Function | string, args: any[], returnByValue: boolean): Promise { if (helper.isString(pageFunction)) { - const contextId = this._contextId; - const expression: string = pageFunction; - return await this._session.send('Runtime.evaluate', { - expression: js.ensureSourceUrl(expression), - contextId, - returnByValue: false, + const utilityScript = await context.utilityScript(); + const functionDeclaration = `function (returnByValue, pageFunction) { return this.evaluate(returnByValue, pageFunction); }${js.generateSourceUrl()}`; + return await this._session.send('Runtime.callFunctionOn', { + functionDeclaration, + objectId: utilityScript._remoteObject.objectId!, + arguments: [ { value: returnByValue }, { value: pageFunction } ], + returnByValue: false, // We need to return real Promise if that is a promise. emulateUserGesture: true }); } @@ -110,12 +111,12 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { try { const utilityScript = await context.utilityScript(); - const callParams = this._serializeFunctionAndArguments(functionText, values, handles); + const callParams = this._serializeFunctionAndArguments(functionText, values, handles, returnByValue); return await this._session.send('Runtime.callFunctionOn', { functionDeclaration: callParams.functionText, objectId: utilityScript._remoteObject.objectId!, - arguments: [ ...callParams.callArguments ], - returnByValue: false, + arguments: callParams.callArguments, + returnByValue: false, // We need to return real Promise if that is a promise. emulateUserGesture: true }); } finally { @@ -123,9 +124,9 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { } } - private _serializeFunctionAndArguments(originalText: string, values: any[], handles: MaybeCallArgument[]): { functionText: string, callArguments: Protocol.Runtime.CallArgument[] } { + private _serializeFunctionAndArguments(originalText: string, values: any[], handles: MaybeCallArgument[], returnByValue: boolean): { functionText: string, callArguments: Protocol.Runtime.CallArgument[]} { const callArguments: Protocol.Runtime.CallArgument[] = values.map(value => ({ value })); - let functionText = `function (functionText, ...args) { return this.evaluate(functionText, ...args); }${js.generateSourceUrl()}`; + let functionText = `function (returnByValue, functionText, ...args) { return this.callFunction(returnByValue, functionText, ...args); }${js.generateSourceUrl()}`; if (handles.some(handle => 'unserializable' in handle)) { const paramStrings = []; for (let i = 0; i < callArguments.length; i++) @@ -138,11 +139,11 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { callArguments.push(handle); } } - functionText = `function (functionText, ...a) { return this.evaluate(functionText, ${paramStrings.join(',')}); }${js.generateSourceUrl()}`; + functionText = `function (returnByValue, functionText, ...a) { return this.callFunction(returnByValue, functionText, ${paramStrings.join(',')}); }${js.generateSourceUrl()}`; } else { callArguments.push(...(handles as Protocol.Runtime.CallArgument[])); } - return { functionText, callArguments: [ { value: originalText }, ...callArguments ] }; + return { functionText, callArguments: [ { value: returnByValue }, { value: originalText }, ...callArguments ] }; function unserializableToString(arg: any) { if (Object.is(arg, -0)) diff --git a/test/chromium/session.spec.js b/test/chromium/session.spec.js index 2c4c2e686f..d4083c3dd7 100644 --- a/test/chromium/session.spec.js +++ b/test/chromium/session.spec.js @@ -42,13 +42,15 @@ describe('ChromiumBrowserContext.createSession', function() { // JS coverage enables and then disables Debugger domain. await page.coverage.startJSCoverage(); await page.coverage.stopJSCoverage(); + page.on('console', console.log); // generate a script in page and wait for the event. - const [event] = await Promise.all([ - new Promise(f => client.on('Debugger.scriptParsed', f)), + await Promise.all([ + new Promise(f => client.on('Debugger.scriptParsed', event => { + if (event.url === 'foo.js') + f(); + })), page.evaluate('//# sourceURL=foo.js') ]); - // expect events to be dispatched. - expect(event.url).toBe('foo.js'); }); it('should be able to detach session', async function({page, browser, server}) { const client = await page.context().newCDPSession(page); diff --git a/test/evaluation.spec.js b/test/evaluation.spec.js index 9cc99b5247..5b26e34784 100644 --- a/test/evaluation.spec.js +++ b/test/evaluation.spec.js @@ -326,6 +326,35 @@ describe('Page.evaluate', function() { await page.goto(server.EMPTY_PAGE); expect(await page.evaluate(() => 2 + 2)).toBe(4); }); + it('should evaluate exception', async({page, server}) => { + const error = await page.evaluate(() => { + return (function functionOnStack() { + return new Error('error message'); + })(); + }); + expect(error).toContain('Error: error message'); + expect(error).toContain('functionOnStack'); + }); + it('should evaluate exception', async({page, server}) => { + const error = await page.evaluate(`new Error('error message')`); + expect(error).toContain('Error: error message'); + }); + it('should evaluate date as {}', async({page}) => { + const result = await page.evaluate(() => ({ date: new Date() })); + expect(result).toEqual({ date: {} }); + }); + it('should jsonValue() date as {}', async({page}) => { + const resultHandle = await page.evaluateHandle(() => ({ date: new Date() })); + expect(await resultHandle.jsonValue()).toEqual({ date: {} }); + }); + it.fail(FFOX)('should not use toJSON when evaluating', async({page, server}) => { + const result = await page.evaluate(() => ({ toJSON: () => 'string', data: 'data' })); + expect(result).toEqual({ data: 'data', toJSON: {} }); + }); + it.fail(FFOX)('should not use toJSON in jsonValue', async({page, server}) => { + const resultHandle = await page.evaluateHandle(() => ({ toJSON: () => 'string', data: 'data' })); + expect(await resultHandle.jsonValue()).toEqual({ data: 'data', toJSON: {} }); + }); }); describe('Page.addInitScript', function() {