fix: Scope breadcrumb handling (#2014)

* fix: Scope breadcrumb handling

* fix: Tests

* meta: Changelog

Author: HazAT

CHANGELOG.md

@@ -3,6 +3,8 @@
 ## Unreleased
 
 - [utils] fix: Remove node types
+- [hub] fix: Make sure all breadcrumbs have a timestamp
+- [hub] fix: Merge event with scope breadcrumbs instead of only using event breadcrumbs
 
 ## 5.0.7
 

packages/core/test/lib/base.test.ts

@@ -265,16 +265,15 @@ describe('BaseClient', () => {
     });
 
     test('adds breadcrumbs', () => {
-      expect.assertions(1);
+      expect.assertions(4);
       const client = new TestClient({ dsn: PUBLIC_DSN });
       const scope = new Scope();
       scope.addBreadcrumb({ message: 'breadcrumb' }, 100);
       client.captureEvent({ message: 'message' }, undefined, scope);
-      expect(TestBackend.instance!.event!).toEqual({
-        breadcrumbs: [{ message: 'breadcrumb' }],
-        event_id: '42',
-        message: 'message',
-      });
+      expect(TestBackend.instance!.event!).toHaveProperty('event_id', '42');
+      expect(TestBackend.instance!.event!).toHaveProperty('message', 'message');
+      expect(TestBackend.instance!.event!).toHaveProperty('breadcrumbs');
+      expect(TestBackend.instance!.event!.breadcrumbs![0]).toHaveProperty('message', 'breadcrumb');
     });
 
     test('limits previously saved breadcrumbs', () => {

packages/hub/src/hub.ts

@@ -205,7 +205,8 @@ export class Hub implements HubInterface {
       return;
     }
 
-    const { beforeBreadcrumb, maxBreadcrumbs = DEFAULT_BREADCRUMBS } = top.client.getOptions();
+    const { beforeBreadcrumb = null, maxBreadcrumbs = DEFAULT_BREADCRUMBS } =
+      (top.client.getOptions && top.client.getOptions()) || {};
 
     if (maxBreadcrumbs <= 0) {
       return;

packages/hub/src/scope.ts

@@ -201,10 +201,13 @@ export class Scope implements ScopeInterface {
    * @inheritDoc
    */
   public addBreadcrumb(breadcrumb: Breadcrumb, maxBreadcrumbs?: number): this {
+    const timestamp = new Date().getTime() / 1000;
+    const mergedBreadcrumb = { timestamp, ...breadcrumb };
+
     this._breadcrumbs =
       maxBreadcrumbs !== undefined && maxBreadcrumbs >= 0
-        ? [...this._breadcrumbs, normalize(breadcrumb)].slice(-maxBreadcrumbs)
-        : [...this._breadcrumbs, normalize(breadcrumb)];
+        ? [...this._breadcrumbs, normalize(mergedBreadcrumb)].slice(-maxBreadcrumbs)
+        : [...this._breadcrumbs, normalize(mergedBreadcrumb)];
     this._notifyScopeListeners();
     return this;
   }
@@ -266,10 +269,8 @@ export class Scope implements ScopeInterface {
 
     this._applyFingerprint(event);
 
-    const hasNoBreadcrumbs = !event.breadcrumbs || event.breadcrumbs.length === 0;
-    if (hasNoBreadcrumbs && this._breadcrumbs.length > 0) {
-      event.breadcrumbs = this._breadcrumbs;
-    }
+    event.breadcrumbs = [...(event.breadcrumbs || []), ...this._breadcrumbs];
+    event.breadcrumbs = event.breadcrumbs.length > 0 ? event.breadcrumbs : undefined;
 
     return this._notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint);
   }

packages/hub/test/hub.test.ts

@@ -225,4 +225,29 @@ describe('Hub', () => {
     });
     expect(currentHub).toBe(getCurrentHub());
   });
+
+  describe('breadcrumbs', () => {
+    test('withScope', () => {
+      expect.assertions(6);
+      const hub = new Hub(clientFn);
+      hub.addBreadcrumb({ message: 'My Breadcrumb' });
+      hub.withScope(scope => {
+        scope.addBreadcrumb({ message: 'scope breadcrumb' });
+        const event: Event = {};
+        scope
+          .applyToEvent(event)
+          .then((appliedEvent: Event | null) => {
+            expect(appliedEvent).toBeTruthy();
+            expect(appliedEvent!.breadcrumbs).toHaveLength(2);
+            expect(appliedEvent!.breadcrumbs![0].message).toEqual('My Breadcrumb');
+            expect(appliedEvent!.breadcrumbs![0]).toHaveProperty('timestamp');
+            expect(appliedEvent!.breadcrumbs![1].message).toEqual('scope breadcrumb');
+            expect(appliedEvent!.breadcrumbs![1]).toHaveProperty('timestamp');
+          })
+          .catch(e => {
+            console.error(e);
+          });
+      });
+    });
+  });
 });

packages/hub/test/scope.test.ts

@@ -68,7 +68,7 @@ describe('Scope', () => {
     test('add', () => {
       const scope = new Scope();
       scope.addBreadcrumb({ message: 'test' }, 100);
-      expect((scope as any)._breadcrumbs).toEqual([{ message: 'test' }]);
+      expect((scope as any)._breadcrumbs[0]).toHaveProperty('message', 'test');
     });
   });
 
@@ -128,20 +128,20 @@ describe('Scope', () => {
       expect(processedEvent!.user).toEqual({ id: '1' });
       expect(processedEvent!.fingerprint).toEqual(['abcd']);
       expect(processedEvent!.level).toEqual('warning');
-      expect(processedEvent!.breadcrumbs).toEqual([{ message: 'test' }]);
+      expect(processedEvent!.breadcrumbs![0]).toHaveProperty('message', 'test');
     });
   });
 
   test('applyToEvent merge', () => {
-    expect.assertions(5);
+    expect.assertions(7);
     const scope = new Scope();
     scope.setExtra('a', 2);
     scope.setTag('a', 'b');
     scope.setUser({ id: '1' });
     scope.setFingerprint(['abcd']);
     scope.addBreadcrumb({ message: 'test' }, 100);
     const event: Event = {
-      breadcrumbs: [{ message: 'test2' }],
+      breadcrumbs: [{ message: 'test1' }],
       extra: { b: 3 },
       fingerprint: ['efgh'],
       tags: { b: 'c' },
@@ -152,7 +152,9 @@ describe('Scope', () => {
       expect(processedEvent!.tags).toEqual({ a: 'b', b: 'c' });
       expect(processedEvent!.user).toEqual({ id: '3' });
       expect(processedEvent!.fingerprint).toEqual(['efgh', 'abcd']);
-      expect(processedEvent!.breadcrumbs).toEqual([{ message: 'test2' }]);
+      expect(processedEvent!.breadcrumbs).toHaveLength(2);
+      expect(processedEvent!.breadcrumbs![0]).toHaveProperty('message', 'test1');
+      expect(processedEvent!.breadcrumbs![1]).toHaveProperty('message', 'test');
     });
   });