From a164524c72443920e0f008d62c5362c08a7794a0 Mon Sep 17 00:00:00 2001
From: JoelEinbinder <joel.einbinder@gmail.com>
Date: Fri, 8 Dec 2017 22:05:46 -0500
Subject: [PATCH] feat(Connection): nicer stack traces on protocol errors
 (#1383)

This rewrites protocol errors to have the stack trace of the call site.
---
 lib/Connection.js | 28 +++++++++++++++++++---------
 test/test.js      | 11 +++++++++++
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/lib/Connection.js b/lib/Connection.js
index a54988eff68..a093c750c75 100644
--- a/lib/Connection.js
+++ b/lib/Connection.js
@@ -42,7 +42,7 @@ class Connection extends EventEmitter {
     super();
     this._url = url;
     this._lastId = 0;
-    /** @type {!Map<number, {resolve: function, reject: function, method: string}>}*/
+    /** @type {!Map<number, {resolve: function, reject: function, error: !Error, method: string}>}*/
     this._callbacks = new Map();
     this._delay = delay;
 
@@ -71,7 +71,7 @@ class Connection extends EventEmitter {
     debugProtocol('SEND ► ' + message);
     this._ws.send(message);
     return new Promise((resolve, reject) => {
-      this._callbacks.set(id, {resolve, reject, method});
+      this._callbacks.set(id, {resolve, reject, error: new Error(), method});
     });
   }
 
@@ -94,7 +94,7 @@ class Connection extends EventEmitter {
       const callback = this._callbacks.get(object.id);
       this._callbacks.delete(object.id);
       if (object.error)
-        callback.reject(new Error(`Protocol error (${callback.method}): ${object.error.message} ${object.error.data}`));
+        callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): ${object.error.message} ${object.error.data}`));
       else
         callback.resolve(object.result);
     } else {
@@ -121,7 +121,7 @@ class Connection extends EventEmitter {
     }
     this._ws.removeAllListeners();
     for (const callback of this._callbacks.values())
-      callback.reject(new Error(`Protocol error (${callback.method}): Target closed.`));
+      callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
     this._callbacks.clear();
     for (const session of this._sessions.values())
       session._onClosed();
@@ -154,7 +154,7 @@ class Session extends EventEmitter {
   constructor(connection, targetId, sessionId) {
     super();
     this._lastId = 0;
-    /** @type {!Map<number, {resolve: function, reject: function, method: string}>}*/
+    /** @type {!Map<number, {resolve: function, reject: function, error: !Error, method: string}>}*/
     this._callbacks = new Map();
     this._connection = connection;
     this._targetId = targetId;
@@ -185,10 +185,10 @@ class Session extends EventEmitter {
         return;
       const callback = this._callbacks.get(id);
       this._callbacks.delete(id);
-      callback.reject(e);
+      callback.reject(rewriteError(callback.error, e && e.message));
     });
     return new Promise((resolve, reject) => {
-      this._callbacks.set(id, {resolve, reject, method});
+      this._callbacks.set(id, {resolve, reject, error: new Error(), method});
     });
   }
 
@@ -202,7 +202,7 @@ class Session extends EventEmitter {
       const callback = this._callbacks.get(object.id);
       this._callbacks.delete(object.id);
       if (object.error)
-        callback.reject(new Error(`Protocol error (${callback.method}): ${object.error.message} ${object.error.data}`));
+        callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): ${object.error.message} ${object.error.data}`));
       else
         callback.resolve(object.result);
     } else {
@@ -218,10 +218,20 @@ class Session extends EventEmitter {
 
   _onClosed() {
     for (const callback of this._callbacks.values())
-      callback.reject(new Error(`Protocol error (${callback.method}): Target closed.`));
+      callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`));
     this._callbacks.clear();
     this._connection = null;
   }
 }
 
+/**
+ * @param {!Error} error
+ * @param {string} message
+ * @return {!Error}
+ */
+function rewriteError(error, message) {
+  error.message = message;
+  return error;
+}
+
 module.exports = {Connection, Session};
diff --git a/test/test.js b/test/test.js
index b9be433bc35..4338b977433 100644
--- a/test/test.js
+++ b/test/test.js
@@ -3271,6 +3271,17 @@ describe('Page', function() {
       await newPage.close();
     }));
   });
+
+  describe('Connection', function() {
+    it('should throw nice errors', SX(async function() {
+      const error = await theSourceOfTheProblems().catch(error => error);
+      expect(error.stack).toContain('theSourceOfTheProblems');
+      expect(error.message).toContain('ThisCommand.DoesNotExist');
+      async function theSourceOfTheProblems() {
+        await page._client.send('ThisCommand.DoesNotExist');
+      }
+    }));
+  });
 });
 
 if (process.env.COVERAGE) {