Unify logging of rejection reasons

Previously, rejections returned in event callbacks would break logging
if their results didn't have a toString() function.

Extract the formatting approach for rejections used in Promise to an
util function and use it for event rejections as well. It can handle all
kinds of reasons and ensures that stack traces will be printed when the
rejection reason is not an error.

Update the Promise rejection reason formatting to reuse code in
util-stacktrace ensuring that the original stack trace will be printed
in case the stack trace filter returns an empty stack trace (e.g. when
the stack trace originates from Tabris.js).

Change-Id: Ib856d5e2811138d6819a62898caeea8b736bc833

src/tabris/Events.js

@@ -3,6 +3,7 @@ import EventObject from './EventObject';
 import {omit} from './util';
 import {hint, toValueString} from './Console';
 import {notify} from './symbols';
+import {formatPromiseRejectionReason} from './util-stacktrace';
 
 export default {
 
@@ -130,9 +131,11 @@ export default {
         for (const callback of this._callbacks[type]) {
           const value = callback.fn.call(callback.ctx || this, dispatchObject);
           if (value instanceof Promise) {
-            value.catch(ex => console.error(
-              `Listener for ${target.constructor.name} event "${type}" rejected:\n${ex.toString()}`
-            ));
+            value.catch(err => {
+              console.error(
+                `Listener for ${target.constructor.name} event "${type}" rejected: ${formatPromiseRejectionReason(err)}`
+              );
+            });
           }
           returnValues.push(value);
         }

src/tabris/Promise.js

@@ -23,7 +23,7 @@
  * THE SOFTWARE.
  */
 
-import {formatStack} from './util-stacktrace';
+import {formatPromiseRejectionReason} from './util-stacktrace';
 
 function asap(fn) {
   setTimeout(fn, 0);
@@ -366,21 +366,19 @@ Promise._onHandle = (promise) => {
     delete rejections[promise[rejectionId]];
   }
 };
-Promise._onReject = (promise, err) => {
+Promise._onReject = (promise, error) => {
   if (promise._deferredState === 0) { // not yet handled
     promise[rejectionId] = lastRejectionId++;
     rejections[promise[rejectionId]] = {
       displayId: null,
-      error: err,
+      error,
       timeout: setTimeout(() => {
         const id = promise[rejectionId];
         rejections[id].displayId = rejectionDisplayId++;
         rejections[id].logged = true;
-        let error = typeof err === 'undefined' ? '' : err;
-        if (!err || !err.stack) {
-          error += '\n' + formatStack(new Error().stack);
-        }
-        console.error(`Uncaught promise rejection (id: ${rejections[id].displayId}) ${error}`);
+        console.error(
+          `Uncaught promise rejection (id: ${rejections[id].displayId}) ${formatPromiseRejectionReason(error)}`
+        );
       }, 0),
       logged: false
     };

src/tabris/util-stacktrace.js

@@ -7,9 +7,14 @@ const iosStackLineRegex = /^(.+)@(.*):([0-9]+):([0-9]+)/;
 const iosStackLineNoNameRegex = /(.*):([0-9]+):([0-9]+)/;
 const urlBaseRegEx = /^[a-z]+:\/\/[^/]+\//;
 
-export function getStackTrace(error) {
+/**
+ * @param {Error} error
+ * @param {boolean} errorStyleFormatting
+ * Prefix trace lines with "at", similarly to how logged errors are formatted in V8.
+ */
+export function getStackTrace(error, errorStyleFormatting = false) {
   try {
-    return getStackArray(error.stack).join('\n');
+    return getStackArray(error.stack).map(line => errorStyleFormatting ? '  at ' + line : line).join('\n');
   } catch (ex) {
     const minimalError = (ex && ex.constructor && ex.message) ? ex.constructor.name + ': ' + ex.message : '';
     warn(`Could not process stack trace (${minimalError || ex}), printing original.`);
@@ -40,12 +45,17 @@ export function formatError(error) {
   return error.constructor.name + ': ' + error.message + '\n' + stack;
 }
 
-/**
- *
- * @param {string} stack
- */
-export function formatStack(stack) {
-  return getStackArray(stack).map(line => '  at ' + line).join('\n');
+export function formatPromiseRejectionReason(reason) {
+  let result;
+  if (reason && typeof reason.toString === 'function') {
+    result = reason.toString();
+  } else {
+    result = typeof reason === 'undefined' ? '' : reason + '';
+  }
+  if (!reason || !reason.stack) {
+    result += '\n' + getStackTrace(new Error(), true);
+  }
+  return result;
 }
 
 export function getCurrentLine(error) {

test/tabris/util-stacktrace.test.js

@@ -2,7 +2,13 @@ import {expect, stub, restore, mockTabris} from '../test';
 import ClientMock from './ClientMock';
 import {addWindowTimerMethods}  from '../../src/tabris/WindowTimers';
 import {create as createApp} from '../../src/tabris/App';
-import {getStackTrace, patchError, formatError, getCurrentLine} from '../../src/tabris/util-stacktrace';
+import {
+  getStackTrace,
+  patchError,
+  formatError,
+  getCurrentLine,
+  formatPromiseRejectionReason
+} from '../../src/tabris/util-stacktrace';
 import PromisePolyfill from '../../src/tabris/Promise';
 
 describe('util-stacktrace', function() {
@@ -224,6 +230,10 @@ showActionSheet (./dist/actionsheet.js:15:25)`
           expect(getStackTrace({stack: stacks[platform].production})).to.equal(stacks.expected.simplified);
         });
 
+        it('prints stack trace with error-style formatting', function() {
+          expect(getStackTrace({stack: stacks[platform].production}, true)).to.equal(stacks.expected.simplifiedError);
+        });
+
         it('prints full stack trace in debug mode (non-minified)', function() {
           expect(getStackTrace({stack: stacks[platform].debug})).to.equal(stacks.expected.full);
         });
@@ -447,6 +457,42 @@ showActionSheet (./dist/actionsheet.js:15:25)`
 
       });
 
+      describe('formatPromiseRejectionReason', function() {
+
+        it('returns stack trace for undefined', function() {
+          const result = formatPromiseRejectionReason();
+
+          expect(result).to.match(/^\n {2}at .*util-stacktrace.js/);
+        });
+
+        it('returns stack trace and reason for null', function() {
+          const result = formatPromiseRejectionReason(null);
+
+          expect(result).to.match(/^null\n {2}at .*util-stacktrace.js/);
+        });
+
+        it('returns stack trace and reason for string', function() {
+          const result = formatPromiseRejectionReason('foo');
+
+          expect(result).to.match(/^foo\n {2}at .*util-stacktrace.js/);
+        });
+
+        it('returns stack trace and reason for stringifiables', function() {
+          const stringifiable = {toString: () => 'foo'};
+          const result = formatPromiseRejectionReason(stringifiable);
+
+          expect(result).to.match(/^foo\n {2}at .*util-stacktrace.js/);
+        });
+
+        it('does not include stack if object has one', function() {
+          const stringifiable = {toString: () => 'foo', stack: 'bar'};
+          const result = formatPromiseRejectionReason(stringifiable);
+
+          expect(result).to.match(/^foo$/);
+        });
+
+      });
+
     });
 
   });