diff --git a/src/common/utilityScriptSerializers.ts b/src/common/utilityScriptSerializers.ts index 8eec2fc7d6..e35a1a11d0 100644 --- a/src/common/utilityScriptSerializers.ts +++ b/src/common/utilityScriptSerializers.ts @@ -15,8 +15,7 @@ */ export function parseEvaluationResultValue(value: any, handles: any[] = []): any { - // { type: 'undefined' } does not even have value. - if (value === 'undefined') + if (value === undefined) return undefined; if (typeof value === 'object') { if (value.v === 'undefined') diff --git a/src/injected/utilityScript.ts b/src/injected/utilityScript.ts index d48c6f1610..c1cc401130 100644 --- a/src/injected/utilityScript.ts +++ b/src/injected/utilityScript.ts @@ -32,6 +32,9 @@ export default class UtilityScript { } jsonValue(returnByValue: true, value: any) { + // Special handling of undefined to work-around multi-step returnByValue handling in WebKit. + if (Object.is(value, undefined)) + return undefined; return serializeAsCallArgument(value, (value: any) => ({ fallThrough: value })); } diff --git a/src/webkit/wkExecutionContext.ts b/src/webkit/wkExecutionContext.ts index 10c6b018c4..0ba8733e02 100644 --- a/src/webkit/wkExecutionContext.ts +++ b/src/webkit/wkExecutionContext.ts @@ -80,8 +80,13 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { throw new Error('Evaluation failed: ' + response.result.description); if (!returnByValue) return utilityScript._context.createHandle(response.result); - if (response.result.objectId) + if (response.result.objectId) { + // Avoid protocol round trip for evaluates that do not return anything. + // Otherwise, we can fail with 'execution context destroyed' without any reason. + if (response.result.type === 'undefined') + return undefined; return await this._returnObjectByValue(utilityScript, response.result.objectId); + } return parseEvaluationResultValue(response.result.value); } catch (error) { throw rewriteError(error); @@ -89,7 +94,6 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { } private async _returnObjectByValue(utilityScript: js.JSHandle, objectId: Protocol.Runtime.RemoteObjectId): Promise { - // This is different from handleJSONValue in that it does not throw. try { const serializeResponse = await this._session.send('Runtime.callFunctionOn', { functionDeclaration: 'object => object' + sourceMap.generateSourceUrl(), @@ -98,13 +102,10 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { returnByValue: true }); if (serializeResponse.wasThrown) - return undefined; + throw new Error('Evaluation failed: ' + serializeResponse.result.description); return parseEvaluationResultValue(serializeResponse.result.value); } catch (error) { - return undefined; - // TODO: we should actually throw an error, but that breaks the common case of undefined - // that is for some reason reported as an object and cannot be accessed after navigation. - // throw rewriteError(error); + throw rewriteError(error); } } diff --git a/test/evaluation.spec.js b/test/evaluation.spec.js index bcc2f94961..8924585d87 100644 --- a/test/evaluation.spec.js +++ b/test/evaluation.spec.js @@ -228,6 +228,10 @@ describe('Page.evaluate', function() { it('should properly serialize undefined fields', async({page}) => { expect(await page.evaluate(() => ({a: undefined}))).toEqual({}); }); + it('should return undefined properties', async({page}) => { + const value = await page.evaluate(() => ({a: undefined})); + expect('a' in value).toBe(true); + }); it('should properly serialize null arguments', async({page}) => { expect(await page.evaluate(x => x, null)).toEqual(null); });