From 632b90efaebc6086cd8c6a796ab75f8fe6eab65c Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <lushnikov@chromium.org>
Date: Fri, 16 Jun 2017 11:21:44 -0700
Subject: [PATCH] Page.Events.Error should throw an proper error

This patch:
- renames Page.Events.Exception in Page.Events.Error
- dispatches an error which has a page stack as its message
---
 lib/Page.js             | 56 ++++++++++++++++++++++++-----------------
 phantom_shim/WebPage.js |  2 +-
 test/test.js            | 12 ++++++++-
 3 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/lib/Page.js b/lib/Page.js
index 39a0b14e..a73b6dd0 100644
--- a/lib/Page.js
+++ b/lib/Page.js
@@ -213,11 +213,12 @@ class Page extends EventEmitter {
         return this._userAgent;
     }
 
-    _handleException(exceptionDetails) {
-        var stack = [];
-        if (exceptionDetails.stackTrace)
-            stack = exceptionDetails.stackTrace.callFrames.map(cf => cf.url);
-        this.emit(Page.Events.Exception, exceptionDetails.exception.description, stack);
+    /**
+     * @param {!Object} exceptionDetails
+     */
+    async _handleException(exceptionDetails) {
+        var message = await this._getExceptionMessage(exceptionDetails);
+        this.emit(Page.Events.Error, new Error(message));
     }
 
     _onConsoleAPI(event) {
@@ -330,8 +331,8 @@ class Page extends EventEmitter {
             expression: code
         });
         if (response.exceptionDetails) {
-            await throwException(this._client, response.exceptionDetails);
-            return;
+            var message = await this._getExceptionMessage(response.exceptionDetails);
+            throw new Error('Evaluation failed: ' + message);
         }
 
         var remoteObject = response.result;
@@ -348,29 +349,38 @@ class Page extends EventEmitter {
             objectId: remoteObject.objectId
         });
         if (response.exceptionDetails) {
-            await throwException(this._client, response.exceptionDetails);
-            return;
+            var message = await this._getExceptionMessage(response.exceptionDetails);
+            throw new Error('Evaluation failed with ' + message);
         }
 
         return response.result.value;
+    }
 
-        /**
-         * @param {!Object} exceptionDetails
-         * @return {!Promise}
-         */
-        async function throwException(client, exceptionDetails) {
-            var exception = exceptionDetails.exception;
-            if (!exception) {
-                throw new Error('Evaluation failed with ' + exceptionDetails.text);
-                return;
-            }
-            var response = await client.send('Runtime.callFunctionOn', {
+    /**
+     * @param {!Object} exceptionDetails
+     * @return {string}
+     */
+    async _getExceptionMessage(exceptionDetails) {
+        var message = '';
+        var exception = exceptionDetails.exception;
+        if (exception) {
+            var response = await this._client.send('Runtime.callFunctionOn', {
                 objectId: exception.objectId,
-                functionDeclaration: 'function() { return this.toString(); }',
+                functionDeclaration: 'function() { return this.message; }',
                 returnByValue: true,
             });
-            throw new Error('Evaluation failed with ' + response.result.value);
+            message = response.result.value;
+        } else {
+            message = exceptionDetails.text;
         }
+        if (exceptionDetails.stackTrace) {
+            for (var callframe of exceptionDetails.stackTrace.callFrames) {
+                var location = callframe.url + ':' + callframe.lineNumber + ':' + callframe.columnNumber;
+                var functionName = callframe.functionName || '<anonymous>';
+                message += `\n    at ${functionName} (${location})`;
+            }
+        }
+        return message;
     }
 
     /**
@@ -583,7 +593,7 @@ function convertPrintParameterToInches(parameter) {
 Page.Events = {
     ConsoleMessage: 'consolemessage',
     Dialog: 'dialog',
-    Exception: 'exception',
+    Error: 'error',
     ResourceLoadingFailed: 'resourceloadingfailed',
     ResponseReceived: 'responsereceived',
 };
diff --git a/phantom_shim/WebPage.js b/phantom_shim/WebPage.js
index 65ee03c9..cebe70d2 100644
--- a/phantom_shim/WebPage.js
+++ b/phantom_shim/WebPage.js
@@ -64,7 +64,7 @@ class WebPage {
         this._pageEvents.on(PageEvents.Confirm, message => this._onConfirm(message));
         this._pageEvents.on(PageEvents.Alert, message => this._onAlert(message));
         this._pageEvents.on(PageEvents.Dialog, dialog => this._onDialog(dialog));
-        this._pageEvents.on(PageEvents.Exception, (exception, stack) => (this._onError || noop).call(null, exception, stack));
+        this._pageEvents.on(PageEvents.Error, error => (this._onError || noop).call(null, error.message, error.stack));
     }
 
     /**
diff --git a/test/test.js b/test/test.js
index d0172d98..ffe3809b 100644
--- a/test/test.js
+++ b/test/test.js
@@ -72,7 +72,7 @@ describe('Puppeteer', function() {
                 error = e;
             }
             expect(error).toBeTruthy();
-            expect(error.message).toContain('ReferenceError');
+            expect(error.message).toContain('not is not defined');
         }));
     });
 
@@ -186,6 +186,16 @@ describe('Puppeteer', function() {
             expect(result).toBe('answer!');
         }));
     });
+
+    describe('Page.Events.Error', function() {
+        it('should fire', function(done) {
+            page.on('error', error => {
+                expect(error.message).toContain('Fancy');
+                done();
+            });
+            page.navigate(STATIC_PREFIX + '/error.html');
+        });
+    });
 });
 
 // Since Jasmine doesn't like async functions, they should be wrapped