From b2f94909a1ca0b9ac81b12f2dadef61415cfda7f Mon Sep 17 00:00:00 2001
From: Joel Einbinder <joel.einbinder@gmail.com>
Date: Mon, 10 Jun 2019 17:35:40 -0700
Subject: [PATCH] fix(eval): be able to throw protocol like errors (#4551)

---
 lib/ExecutionContext.js | 20 +++++++++-----------
 test/evaluation.spec.js |  8 ++++++++
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/lib/ExecutionContext.js b/lib/ExecutionContext.js
index b1b1fcba114..9a4ac6236b7 100644
--- a/lib/ExecutionContext.js
+++ b/lib/ExecutionContext.js
@@ -42,18 +42,10 @@ class ExecutionContext {
   /**
    * @param {Function|string} pageFunction
    * @param {...*} args
-   * @return {!Promise<(!Object|undefined)>}
+   * @return {!Promise<*>}
    */
   async evaluate(pageFunction, ...args) {
-    try {
-      return await this._evaluateInternal(true /* returnByValue */, pageFunction, ...args);
-    } catch (error) {
-      if (error.message.includes('Object reference chain is too long'))
-        return;
-      if (error.message.includes('Object couldn\'t be returned by value'))
-        return;
-      throw error;
-    }
+    return await this._evaluateInternal(true /* returnByValue */, pageFunction, ...args);
   }
 
   /**
@@ -66,9 +58,10 @@ class ExecutionContext {
   }
 
   /**
+   * @param {boolean} returnByValue
    * @param {Function|string} pageFunction
    * @param {...*} args
-   * @return {!Promise<!JSHandle>}
+   * @return {!Promise<*>}
    */
   async _evaluateInternal(returnByValue, pageFunction, ...args) {
     const suffix = `//# sourceURL=${EVALUATION_SCRIPT_URL}`;
@@ -165,6 +158,11 @@ class ExecutionContext {
      * @return {!Protocol.Runtime.evaluateReturnValue}
      */
     function rewriteError(error) {
+      if (error.message.includes('Object reference chain is too long'))
+        return {result: {type: 'undefined'}};
+      if (error.message.includes('Object couldn\'t be returned by value'))
+        return {result: {type: 'undefined'}};
+
       if (error.message.endsWith('Cannot find context with specified id'))
         throw new Error('Execution context was destroyed, most likely because of a navigation.');
       throw error;
diff --git a/test/evaluation.spec.js b/test/evaluation.spec.js
index b4a9daf364f..23b3b18f2fb 100644
--- a/test/evaluation.spec.js
+++ b/test/evaluation.spec.js
@@ -182,6 +182,14 @@ module.exports.addTests = function({testRunner, expect}) {
       });
       expect(result).toBe(undefined);
     });
+    it_fails_ffox('should be able to throw a tricky error', async({page, server}) => {
+      const windowHandle = await page.evaluateHandle(() => window);
+      const errorText = await windowHandle.jsonValue().catch(e => e.message);
+      const error = await page.evaluate(errorText => {
+        throw new Error(errorText);
+      }, errorText).catch(e => e);
+      expect(error.message).toContain(errorText);
+    });
     it('should accept a string', async({page, server}) => {
       const result = await page.evaluate('1 + 2');
       expect(result).toBe(3);