fix: Add type/value to exception in case of no error (#1803)

* fix: Add type/value to exception in case of no error

* fix: Init first element in array

* fix: Test and stacktrace

* fix: Unhandled promise rejections

Author: HazAT

CHANGELOG.md

@@ -10,6 +10,7 @@
   `7` lines pre/post
 - [core]: ref: Change way how transports are initialized
 - [core]: ref: Rename `RequestBuffer` to `PromiseBuffer`, also introduce limit
+- [browser] fix: Prevent empty exception values
 
 ## 4.4.2
 

packages/browser/src/backend.ts

@@ -2,7 +2,7 @@ import { BaseBackend, Options, SentryError } from '@sentry/core';
 import { SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types';
 import { isDOMError, isDOMException, isError, isErrorEvent, isPlainObject } from '@sentry/utils/is';
 import { supportsBeacon, supportsFetch } from '@sentry/utils/supports';
-import { eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
+import { addExceptionTypeValue, eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
 import { computeStackTrace } from './tracekit';
 import { BeaconTransport, FetchTransport, XHRTransport } from './transports';
 
@@ -87,6 +87,7 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
       const message = ex.message ? `${name}: ${ex.message}` : name;
 
       event = await this.eventFromMessage(message, undefined, hint);
+      addExceptionTypeValue(event, message);
     } else if (isError(exception as Error)) {
       // we have a real Error object, do nothing
       event = eventFromStacktrace(computeStackTrace(exception as Error));
@@ -96,6 +97,7 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
       // which is much better than creating new group when any key/value change
       const ex = exception as {};
       event = eventFromPlainObject(ex, hint.syntheticException);
+      addExceptionTypeValue(event, 'Custom Object');
     } else {
       // If none of previous checks were valid, then it means that
       // it's not a DOMError/DOMException
@@ -105,6 +107,7 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
       // So bail out and capture it as a simple message:
       const ex = exception as string;
       event = await this.eventFromMessage(ex, undefined, hint);
+      addExceptionTypeValue(event, `${ex}`);
     }
 
     event = {

packages/browser/src/parsers.ts

@@ -92,3 +92,16 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
     .slice(0, STACKTRACE_LIMIT)
     .reverse();
 }
+
+/**
+ * Adds exception values, type and value to an synthetic Exception.
+ * @param event The event to modify.
+ * @param message Message to be added.
+ */
+export function addExceptionTypeValue(event: SentryEvent, message: string): void {
+  event.exception = event.exception || {};
+  event.exception.values = event.exception.values || [];
+  event.exception.values[0] = event.exception.values[0] || {};
+  event.exception.values[0].value = event.exception.values[0].value || message;
+  event.exception.values[0].type = event.exception.values[0].type || 'Error';
+}

packages/browser/test/integration/test.js

@@ -244,6 +244,25 @@ for (var idx in frames) {
           );
         });
 
+        it('should have exception with type and value', function(done) {
+          var iframe = this.iframe;
+          iframeExecute(
+            iframe,
+            done,
+            function() {
+              Sentry.captureException('this is my test exception');
+            },
+            function(sentryData) {
+              if (debounceAssertEventCount(sentryData, 1, done)) {
+                var sentryData = sentryData[0];
+                assert.isNotEmpty(sentryData.exception.values[0].value);
+                assert.isNotEmpty(sentryData.exception.values[0].type);
+                done();
+              }
+            }
+          );
+        });
+
         it('should reject duplicate, back-to-back errors from captureException', function(done) {
           var iframe = this.iframe;
           iframeExecute(

packages/core/package.json

@@ -42,7 +42,7 @@
     "fix:prettier": "prettier --write \"{src,test}/**/*.ts\"",
     "fix:tslint": "tslint --fix -t stylish -p .",
     "test": "jest",
-    "test:watch": "jest --watch --notify"
+    "test:watch": "jest --watch"
   },
   "jest": {
     "collectCoverage": true,

packages/core/src/promisebuffer.ts

@@ -27,7 +27,14 @@ export class PromiseBuffer<T> {
     if (this.buffer.indexOf(task) === -1) {
       this.buffer.push(task);
     }
-    task.then(async () => this.remove(task)).catch(async () => this.remove(task));
+    task
+      .then(async () => this.remove(task))
+      .catch(async () =>
+        this.remove(task).catch(() => {
+          // We have to add this catch here otherwise we have an unhandledPromiseRejection
+          // because it's a new Promise chain.
+        }),
+      );
     return task;
   }
 

packages/core/test/lib/promisebuffer.test.ts

@@ -92,4 +92,14 @@ describe('PromiseBuffer', () => {
     });
     jest.runAllTimers();
   });
+
+  test('rejecting', async () => {
+    expect.assertions(1);
+    const q = new PromiseBuffer<void>();
+    const p = new Promise<void>((_, reject) => setTimeout(reject, 1));
+    jest.runAllTimers();
+    return q.add(p).catch(() => {
+      expect(true).toBe(true);
+    });
+  });
 });

packages/node/test/index.test.ts

@@ -127,7 +127,7 @@ describe('SentryNode', () => {
           // for our own captureMessage/captureException calls yet
           expect(event.breadcrumbs!).toHaveLength(2);
           done();
-          return event;
+          return null;
         },
         dsn,
       });
@@ -164,7 +164,7 @@ describe('SentryNode', () => {
             expect(event.exception!.values![0].value).toBe('test');
             expect(event.exception!.values![0].stacktrace).toBeTruthy();
             done();
-            return event;
+            return null;
           },
           dsn,
         }),
@@ -249,7 +249,7 @@ describe('SentryNode', () => {
           expect(event.message).toBe('test domain');
           expect(event.exception).toBeUndefined();
           done();
-          return event;
+          return null;
         },
         dsn,
       });