fix: throw from eval methods if >1 argument is passed (#2043)

src/frames.ts

@@ -21,7 +21,7 @@ import { ConsoleMessage } from './console';
 import * as dom from './dom';
 import { TimeoutError, NotConnectedError } from './errors';
 import { Events } from './events';
-import { assert, helper, RegisteredListener } from './helper';
+import { assert, helper, RegisteredListener, assertMaxArguments } from './helper';
 import * as js from './javascript';
 import * as network from './network';
 import { Page } from './page';
@@ -415,6 +415,7 @@ export class Frame {
   async evaluateHandle<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<types.SmartHandle<R>>;
   async evaluateHandle<R>(pageFunction: types.Func1<void, R>, arg?: any): Promise<types.SmartHandle<R>>;
   async evaluateHandle<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<types.SmartHandle<R>> {
+    assertMaxArguments(arguments.length, 2);
     const context = await this._mainContext();
     return context.evaluateHandleInternal(pageFunction, arg);
   }
@@ -422,6 +423,7 @@ export class Frame {
   async evaluate<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<R>;
   async evaluate<R>(pageFunction: types.Func1<void, R>, arg?: any): Promise<R>;
   async evaluate<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<R> {
+    assertMaxArguments(arguments.length, 2);
     const context = await this._mainContext();
     return context.evaluateInternal(pageFunction, arg);
   }
@@ -464,6 +466,7 @@ export class Frame {
   async $eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element, Arg, R>, arg: Arg): Promise<R>;
   async $eval<R>(selector: string, pageFunction: types.FuncOn<Element, void, R>, arg?: any): Promise<R>;
   async $eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element, Arg, R>, arg: Arg): Promise<R> {
+    assertMaxArguments(arguments.length, 3);
     const handle = await this.$(selector);
     if (!handle)
       throw new Error(`Error: failed to find element matching selector "${selector}"`);
@@ -475,6 +478,7 @@ export class Frame {
   async $$eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element[], Arg, R>, arg: Arg): Promise<R>;
   async $$eval<R>(selector: string, pageFunction: types.FuncOn<Element[], void, R>, arg?: any): Promise<R>;
   async $$eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element[], Arg, R>, arg: Arg): Promise<R> {
+    assertMaxArguments(arguments.length, 3);
     const arrayHandle = await selectors._queryArray(this, selector);
     const result = await arrayHandle.evaluate(pageFunction, arg);
     arrayHandle.dispose();

src/helper.ts

@@ -337,6 +337,10 @@ export function assert(value: any, message?: string): asserts value {
     throw new Error(message);
 }
 
+export function assertMaxArguments(count: number, max: number): asserts count {
+  assert(count <= max, 'Too many arguments. If you need to pass more than 1 argument to the function wrap them in an object.');
+}
+
 export function getFromENV(name: string) {
   let value = process.env[name];
   value = value || process.env[`npm_config_${name.toLowerCase()}`];

src/page.ts

@@ -17,7 +17,7 @@
 
 import * as dom from './dom';
 import * as frames from './frames';
-import { assert, helper, Listener } from './helper';
+import { assert, helper, Listener, assertMaxArguments } from './helper';
 import * as input from './input';
 import * as js from './javascript';
 import * as network from './network';
@@ -228,18 +228,21 @@ export class Page extends ExtendedEventEmitter implements InnerLogger {
   async evaluateHandle<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<types.SmartHandle<R>>;
   async evaluateHandle<R>(pageFunction: types.Func1<void, R>, arg?: any): Promise<types.SmartHandle<R>>;
   async evaluateHandle<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<types.SmartHandle<R>> {
+    assertMaxArguments(arguments.length, 2);
     return this.mainFrame().evaluateHandle(pageFunction, arg);
   }
 
   async $eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element, Arg, R>, arg: Arg): Promise<R>;
   async $eval<R>(selector: string, pageFunction: types.FuncOn<Element, void, R>, arg?: any): Promise<R>;
   async $eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element, Arg, R>, arg: Arg): Promise<R> {
+    assertMaxArguments(arguments.length, 3);
     return this.mainFrame().$eval(selector, pageFunction, arg);
   }
 
   async $$eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element[], Arg, R>, arg: Arg): Promise<R>;
   async $$eval<R>(selector: string, pageFunction: types.FuncOn<Element[], void, R>, arg?: any): Promise<R>;
   async $$eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element[], Arg, R>, arg: Arg): Promise<R> {
+    assertMaxArguments(arguments.length, 3);
     return this.mainFrame().$$eval(selector, pageFunction, arg);
   }
 
@@ -373,6 +376,7 @@ export class Page extends ExtendedEventEmitter implements InnerLogger {
   async evaluate<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<R>;
   async evaluate<R>(pageFunction: types.Func1<void, R>, arg?: any): Promise<R>;
   async evaluate<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<R> {
+    assertMaxArguments(arguments.length, 2);
     return this.mainFrame().evaluate(pageFunction, arg);
   }
 
@@ -568,12 +572,14 @@ export class Worker extends EventEmitter {
   async evaluate<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<R>;
   async evaluate<R>(pageFunction: types.Func1<void, R>, arg?: any): Promise<R>;
   async evaluate<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<R> {
+    assertMaxArguments(arguments.length, 2);
     return (await this._executionContextPromise).evaluateInternal(pageFunction, arg);
   }
 
   async evaluateHandle<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<types.SmartHandle<R>>;
   async evaluateHandle<R>(pageFunction: types.Func1<void, R>, arg?: any): Promise<types.SmartHandle<R>>;
   async evaluateHandle<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg): Promise<types.SmartHandle<R>> {
+    assertMaxArguments(arguments.length, 2);
     return (await this._executionContextPromise).evaluateHandleInternal(pageFunction, arg);
   }
 }

test/evaluation.spec.js

@@ -162,6 +162,24 @@ describe('Page.evaluate', function() {
     const result = await page.evaluate(() => -Infinity);
     expect(Object.is(result, -Infinity)).toBe(true);
   });
+  it('should throw when passed more than one parameter', async({page, server}) => {
+    const expectThrow = async f => {
+      let error;
+      await f().catch(e => error = e);
+      expect('' + error).toContain('Too many arguments');
+    }
+    await expectThrow(() => page.evaluate((a, b) => false, 1, 2));
+    await expectThrow(() => page.evaluateHandle((a, b) => false, 1, 2));
+    await expectThrow(() => page.$eval('sel', (a, b) => false, 1, 2));
+    await expectThrow(() => page.$$eval('sel', (a, b) => false, 1, 2));
+    await expectThrow(() => page.evaluate((a, b) => false, 1, 2));
+    const frame = page.mainFrame();
+    await expectThrow(() => frame.evaluate((a, b) => false, 1, 2));
+    await expectThrow(() => frame.evaluateHandle((a, b) => false, 1, 2));
+    await expectThrow(() => frame.$eval('sel', (a, b) => false, 1, 2));
+    await expectThrow(() => frame.$$eval('sel', (a, b) => false, 1, 2));
+    await expectThrow(() => frame.evaluate((a, b) => false, 1, 2));
+  });
   it('should accept "undefined" as one of multiple parameters', async({page, server}) => {
     const result = await page.evaluate(({ a, b }) => Object.is(a, undefined) && Object.is(b, 'foo'), { a: undefined, b: 'foo' });
     expect(result).toBe(true);