ref: Better UnhandledRejection messages (#2185)

kamilogorek · web-flow · commit 4cee445dcfde · 2019-08-05T14:17:21.000+02:00
diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts
@@ -1,6 +1,15 @@
 import { getCurrentHub } from '@sentry/core';
-import { Event, Integration } from '@sentry/types';
-import { addExceptionTypeValue, isString, logger, normalize, truncate } from '@sentry/utils';
+import { Event, Integration, Severity } from '@sentry/types';
+import {
+  addExceptionTypeValue,
+  isPrimitive,
+  isString,
+  keysToEventMessage,
+  logger,
+  normalize,
+  normalizeToSize,
+  truncate,
+} from '@sentry/utils';
 
 import { shouldIgnoreOnError } from '../helpers';
 import { eventFromStacktrace } from '../parsers';
@@ -46,27 +55,13 @@ export class GlobalHandlers implements Integration {
   public setupOnce(): void {
     Error.stackTraceLimit = 50;
 
-    _subscribe((stack: TraceKitStackTrace, _: boolean, error: Error) => {
-      // TODO: use stack.context to get a valuable information from TraceKit, eg.
-      // [
-      //   0: "  })"
-      //   1: ""
-      //   2: "  function foo () {"
-      //   3: "    Sentry.captureException('some error')"
-      //   4: "    Sentry.captureMessage('some message')"
-      //   5: "    throw 'foo'"
-      //   6: "  }"
-      //   7: ""
-      //   8: "  function bar () {"
-      //   9: "    foo();"
-      //   10: "  }"
-      // ]
+    _subscribe((stack: TraceKitStackTrace, _: boolean, error: any) => {
       if (shouldIgnoreOnError()) {
         return;
       }
       const self = getCurrentHub().getIntegration(GlobalHandlers);
       if (self) {
-        getCurrentHub().captureEvent(self._eventFromGlobalHandler(stack), {
+        getCurrentHub().captureEvent(self._eventFromGlobalHandler(stack, error), {
           data: { stack },
           originalException: error,
         });
@@ -89,7 +84,7 @@ export class GlobalHandlers implements Integration {
    *
    * @param stacktrace TraceKitStackTrace to be converted to an Event.
    */
-  private _eventFromGlobalHandler(stacktrace: TraceKitStackTrace): Event {
+  private _eventFromGlobalHandler(stacktrace: TraceKitStackTrace, error: any): Event {
     if (!isString(stacktrace.message) && stacktrace.mechanism !== 'onunhandledrejection') {
       // There are cases where stacktrace.message is an Event object
       // https://github.com/getsentry/sentry-javascript/issues/1949
@@ -98,6 +93,11 @@ export class GlobalHandlers implements Integration {
       stacktrace.message =
         message.error && isString(message.error.message) ? message.error.message : 'No error message';
     }
+
+    if (stacktrace.mechanism === 'onunhandledrejection' && stacktrace.incomplete) {
+      return this._eventFromIncompleteRejection(stacktrace, error);
+    }
+
     const event = eventFromStacktrace(stacktrace);
 
     const data: { [key: string]: string } = {
@@ -129,4 +129,53 @@ export class GlobalHandlers implements Integration {
 
     return event;
   }
+
+  /**
+   * This function creates an Event from an TraceKitStackTrace that has part of it missing.
+   *
+   * @param stacktrace TraceKitStackTrace to be converted to an Event.
+   */
+  private _eventFromIncompleteRejection(stacktrace: TraceKitStackTrace, error: any): Event {
+    const event: Event = {
+      level: Severity.Error,
+    };
+
+    if (isPrimitive(error)) {
+      event.exception = {
+        values: [
+          {
+            type: 'UnhandledRejection',
+            value: `Non-Error promise rejection captured with value: ${error}`,
+          },
+        ],
+      };
+    } else {
+      event.exception = {
+        values: [
+          {
+            type: 'UnhandledRejection',
+            value: `Non-Error promise rejection captured with keys: ${keysToEventMessage(Object.keys(error).sort())}`,
+          },
+        ],
+      };
+      event.extra = {
+        __serialized__: normalizeToSize(error),
+      };
+    }
+
+    if (event.exception.values && event.exception.values[0]) {
+      event.exception.values[0].mechanism = {
+        data: {
+          incomplete: true,
+          mode: stacktrace.mode,
+          ...(stacktrace.message && { message: stacktrace.message }),
+          ...(stacktrace.name && { name: stacktrace.name }),
+        },
+        handled: false,
+        type: stacktrace.mechanism,
+      };
+    }
+
+    return event;
+  }
 }
diff --git a/packages/browser/src/tracekit.ts b/packages/browser/src/tracekit.ts
@@ -1,6 +1,6 @@
 // tslint:disable
 
-import { getGlobalObject, isError, isErrorEvent, normalize } from '@sentry/utils';
+import { getGlobalObject, isError, isErrorEvent } from '@sentry/utils';
 
 /**
  * @hidden
@@ -29,6 +29,7 @@ export interface StackTrace {
   stack: StackFrame[];
   useragent: string;
   original?: string;
+  incomplete?: boolean;
 }
 
 interface ComputeStackTrace {
@@ -271,12 +272,9 @@ TraceKit._report = (function reportModuleWrapper() {
    * @see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent
    */
   function _traceKitWindowOnUnhandledRejection(e: any) {
-    var err = (e && (e.detail ? e.detail.reason : e.reason)) || e;
+    var err = e && typeof e.reason !== 'undefined' ? e.reason : e;
     var stack = TraceKit._computeStackTrace(err);
     stack.mechanism = 'onunhandledrejection';
-    if (!stack.message) {
-      stack.message = JSON.stringify(normalize(err));
-    }
     _notifyHandlers(stack, true, err);
   }
 
diff --git a/packages/browser/test/integration/suites/builtins.js b/packages/browser/test/integration/suites/builtins.js
@@ -80,7 +80,10 @@ describe("wrapped built-ins", function() {
     }).then(function(summary) {
       if (summary.window.isChrome()) {
         // non-error rejections doesnt provide stacktraces so we can skip the assertion
-        assert.equal(summary.events[0].exception.values[0].value, '"test"');
+        assert.equal(
+          summary.events[0].exception.values[0].value,
+          "Non-Error promise rejection captured with value: test"
+        );
         assert.equal(
           summary.events[0].exception.values[0].type,
           "UnhandledRejection"
@@ -93,6 +96,10 @@ describe("wrapped built-ins", function() {
           summary.events[0].exception.values[0].mechanism.type,
           "onunhandledrejection"
         );
+        assert.equal(
+          summary.events[0].exception.values[0].mechanism.data.incomplete,
+          true
+        );
       }
     });
   });
@@ -108,6 +115,10 @@ describe("wrapped built-ins", function() {
       if (summary.window.isChrome()) {
         // non-error rejections doesnt provide stacktraces so we can skip the assertion
         assert.equal(summary.events[0].exception.values[0].value.length, 253);
+        assert.include(
+          summary.events[0].exception.values[0].value,
+          "Non-Error promise rejection captured with value: "
+        );
         assert.equal(
           summary.events[0].exception.values[0].type,
           "UnhandledRejection"
@@ -120,21 +131,28 @@ describe("wrapped built-ins", function() {
           summary.events[0].exception.values[0].mechanism.type,
           "onunhandledrejection"
         );
+        assert.equal(
+          summary.events[0].exception.values[0].mechanism.data.incomplete,
+          true
+        );
       }
     });
   });
 
   it("should capture unhandledrejection with an object", function() {
     return runInSandbox(sandbox, function() {
       if (isChrome()) {
-        Promise.reject({ a: "b" });
+        Promise.reject({ a: "b", b: "c", c: "d" });
       } else {
         window.resolveTest({ window: window });
       }
     }).then(function(summary) {
       if (summary.window.isChrome()) {
         // non-error rejections doesnt provide stacktraces so we can skip the assertion
-        assert.equal(summary.events[0].exception.values[0].value, '{"a":"b"}');
+        assert.equal(
+          summary.events[0].exception.values[0].value,
+          "Non-Error promise rejection captured with keys: a, b, c"
+        );
         assert.equal(
           summary.events[0].exception.values[0].type,
           "UnhandledRejection"
@@ -147,6 +165,10 @@ describe("wrapped built-ins", function() {
           summary.events[0].exception.values[0].mechanism.type,
           "onunhandledrejection"
         );
+        assert.equal(
+          summary.events[0].exception.values[0].mechanism.data.incomplete,
+          true
+        );
       }
     });
   });
@@ -168,7 +190,10 @@ describe("wrapped built-ins", function() {
     }).then(function(summary) {
       if (summary.window.isChrome()) {
         // non-error rejections doesnt provide stacktraces so we can skip the assertion
-        assert.equal(summary.events[0].exception.values[0].value.length, 253);
+        assert.equal(
+          summary.events[0].exception.values[0].value,
+          "Non-Error promise rejection captured with keys: a, b, c, d, e"
+        );
         assert.equal(
           summary.events[0].exception.values[0].type,
           "UnhandledRejection"
@@ -181,6 +206,10 @@ describe("wrapped built-ins", function() {
           summary.events[0].exception.values[0].mechanism.type,
           "onunhandledrejection"
         );
+        assert.equal(
+          summary.events[0].exception.values[0].mechanism.data.incomplete,
+          true
+        );
       }
     });
   });
diff --git a/packages/types/src/mechanism.ts b/packages/types/src/mechanism.ts
@@ -3,7 +3,7 @@ export interface Mechanism {
   type: string;
   handled: boolean;
   data?: {
-    [key: string]: string;
+    [key: string]: string | boolean;
   };
   synthetic?: boolean;
 }

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,7 @@ export interface Mechanism {`
`3`	`3`	`type: string;`
`4`	`4`	`handled: boolean;`
`5`	`5`	`data?: {`
`6`		`- [key: string]: string;`
	`6`	`+ [key: string]: string \| boolean;`
`7`	`7`	`};`
`8`	`8`	`synthetic?: boolean;`
`9`	`9`	`}`