fix(pageerror): report correct error message and stack (#1862)

The error stack matches the browser format.

Author: dgozman

src/chromium/crProtocolHelper.ts

@@ -91,9 +91,21 @@ export function toConsoleMessageLocation(stackTrace: Protocol.Runtime.StackTrace
 }
 
 export function exceptionToError(exceptionDetails: Protocol.Runtime.ExceptionDetails): Error {
-  const message = getExceptionMessage(exceptionDetails);
+  const messageWithStack = getExceptionMessage(exceptionDetails);
+  const lines = messageWithStack.split('\n');
+  const firstStackTraceLine = lines.findIndex(line => line.startsWith('    at'));
+  let message = '';
+  let stack = '';
+  if (firstStackTraceLine === -1) {
+    message = messageWithStack;
+  } else {
+    message = lines.slice(0, firstStackTraceLine).join('\n');
+    stack = messageWithStack;
+  }
+  const match = message.match(/^[a-zA-Z0-0_]*Error: (.*)$/);
+  if (match)
+    message = match[1];
   const err = new Error(message);
-  // Don't report clientside error with a node stack attached
-  err.stack = 'Error: ' + err.message; // Stack is supposed to contain error message as the first line.
+  err.stack = stack;
   return err;
 }

src/firefox/ffPage.ts

@@ -184,7 +184,8 @@ export class FFPage implements PageDelegate {
   }
 
   _onUncaughtError(params: Protocol.Page.uncaughtErrorPayload) {
-    const error = new Error(params.message);
+    const message = params.message.startsWith('Error: ') ? params.message.substring(7) : params.message;
+    const error = new Error(message);
     error.stack = params.stack;
     this._page.emit(Events.Page.PageError, error);
   }

src/helper.ts

@@ -149,16 +149,13 @@ class Helper {
       Helper.removeEventListeners([listener]);
       clearTimeout(eventTimeout);
     }
-    const result = await Promise.race([promise, abortPromise]).then(r => {
+    return await Promise.race([promise, abortPromise]).then(r => {
       cleanup();
       return r;
     }, e => {
       cleanup();
       throw e;
     });
-    if (result instanceof Error)
-      throw result;
-    return result;
   }
 
   static async waitWithTimeout<T>(promise: Promise<T>, taskName: string, timeout: number): Promise<T> {

src/webkit/wkPage.ts

@@ -454,8 +454,15 @@ export class WKPage implements PageDelegate {
       return;
     }
     if (level === 'error' && source === 'javascript') {
-      const error = new Error(text);
-      error.stack = 'Error: ' + error.message; // Nullify stack. Stack is supposed to contain error message as the first line.
+      const message = text.startsWith('Error: ') ? text.substring(7) : text;
+      const error = new Error(message);
+      if (event.message.stackTrace) {
+        error.stack = event.message.stackTrace.map(callFrame => {
+          return `${callFrame.functionName}@${callFrame.url}:${callFrame.lineNumber}:${callFrame.columnNumber}`;
+        }).join('\n');
+      } else {
+        error.stack = '';
+      }
       this._page.emit(Events.Page.PageError, error);
       return;
     }

test/assets/error.html

@@ -11,6 +11,8 @@
 }
 
 function c() {
-    throw new Error('Fancy error!');
+    window.e = new Error('Fancy error!');
+    throw window.e;
 }
+//# sourceURL=myscript.js
 </script>

test/page.spec.js

@@ -493,13 +493,18 @@ describe('Page.exposeFunction', function() {
 
 describe('Page.Events.PageError', function() {
   it('should fire', async({page, server}) => {
-    let error = null;
-    page.once('pageerror', e => error = e);
-    await Promise.all([
+    const [error] = await Promise.all([
+      page.waitForEvent('pageerror'),
       page.goto(server.PREFIX + '/error.html'),
-      new Promise(f => page.on('pageerror', f))
     ]);
-    expect(error.message).toContain('Fancy');
+    expect(error.name).toBe('Error');
+    expect(error.message).toBe('Fancy error!');
+    let stack = await page.evaluate(() => window.e.stack);
+    // Note that WebKit does not use sourceURL for some reason and reports the stack of the 'throw' statement
+    // instead of the Error constructor call.
+    if (WEBKIT)
+      stack = stack.replace('14:25', '15:19');
+    expect(error.stack).toBe(stack);
   });
 });