From 7fabf32172c5773a1c23f94844dfa799b6461174 Mon Sep 17 00:00:00 2001
From: Brian Schiller <bgschiller@gmail.com>
Date: Mon, 14 Jan 2019 15:30:50 -0700
Subject: [PATCH] feat(executioncontext): warn on nested js handle (#3591)

ExecutionContext.evaluateHandle accepts arguments that are either
serializable, or JSHandles. A potential confusion is that it *does not*
accept arguments that *contain* JSHandles.

This patch adds a log message warning when it encounters that situation.

Fixes #3562
---
 lib/ExecutionContext.js | 25 ++++++++++++++++---------
 test/jshandle.spec.js   |  9 +++++++++
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/lib/ExecutionContext.js b/lib/ExecutionContext.js
index 8b6ebcf5581..f0b4af7fb52 100644
--- a/lib/ExecutionContext.js
+++ b/lib/ExecutionContext.js
@@ -111,15 +111,22 @@ class ExecutionContext {
         throw new Error('Passed function is not well-serializable!');
       }
     }
-
-    const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', {
-      functionDeclaration: functionText + '\n' + suffix + '\n',
-      executionContextId: this._contextId,
-      arguments: args.map(convertArgument.bind(this)),
-      returnByValue: false,
-      awaitPromise: true,
-      userGesture: true
-    }).catch(rewriteError);
+    let callFunctionOnPromise;
+    try {
+      callFunctionOnPromise = this._client.send('Runtime.callFunctionOn', {
+        functionDeclaration: functionText + '\n' + suffix + '\n',
+        executionContextId: this._contextId,
+        arguments: args.map(convertArgument.bind(this)),
+        returnByValue: false,
+        awaitPromise: true,
+        userGesture: true
+      });
+    } catch (err) {
+      if (err instanceof TypeError && err.message === 'Converting circular structure to JSON')
+        err.message += ' Are you passing a nested JSHandle?';
+      throw err;
+    }
+    const { exceptionDetails, result: remoteObject } = await callFunctionOnPromise.catch(rewriteError);
     if (exceptionDetails)
       throw new Error('Evaluation failed: ' + helper.getExceptionMessage(exceptionDetails));
     return createJSHandle(this, remoteObject);
diff --git a/test/jshandle.spec.js b/test/jshandle.spec.js
index 26bc52230a3..fdd6b5bcab8 100644
--- a/test/jshandle.spec.js
+++ b/test/jshandle.spec.js
@@ -34,6 +34,15 @@ module.exports.addTests = function({testRunner, expect}) {
       const isFive = await page.evaluate(e => Object.is(e, 5), aHandle);
       expect(isFive).toBeTruthy();
     });
+    it('should warn on nested object handles', async({page, server}) => {
+      const aHandle = await page.evaluateHandle(() => document.body);
+      let error = null;
+      await page.evaluateHandle(
+          opts => opts.elem.querySelector('p'),
+          { elem: aHandle }
+      ).catch(e => error = e);
+      expect(error.message).toContain('Are you passing a nested JSHandle?');
+    });
   });
 
   describe('JSHandle.getProperty', function() {