fix(core): Only generate eventIds in client (#6247)

This changes moves all generation of eventId uuids to the client. Since we can't change the signature of `hub.captureException` and similar, we return a event id of zeros, `00000000000000000000000000000000`, if the client method returns undefined.
getsentry · Nov 22, 2022 · 3fe5f7a · 3fe5f7a
1 parent 72c6855
commit 3fe5f7a
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 68 deletions.
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -127,8 +127,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
       return;
     }
 
-    let eventId: string | undefined = hint && hint.event_id;
-
+    let eventId: string | undefined;
     this._process(
       this.eventFromException(exception, hint)
         .then(event => this._captureEvent(event, hint, scope))
@@ -150,7 +149,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     hint?: EventHint,
     scope?: Scope,
   ): string | undefined {
-    let eventId: string | undefined = hint && hint.event_id;
+    let eventId: string | undefined;
 
     const promisedEvent = isPrimitive(message)
       ? this.eventFromMessage(String(message), level, hint)
@@ -177,7 +176,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
       return;
     }
 
-    let eventId: string | undefined = hint && hint.event_id;
+    let eventId: string | undefined;
 
     this._process(
       this._captureEvent(event, hint, scope).then(result => {

diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts
@@ -27,12 +27,13 @@ import {
   GLOBAL_OBJ,
   isNodeEnv,
   logger,
-  uuid4,
 } from '@sentry/utils';
 
 import { Scope } from './scope';
 import { closeSession, makeSession, updateSession } from './session';
 
+const NIL_EVENT_ID = '00000000000000000000000000000000';
+
 /**
  * API compatibility version of this hub.
  *
@@ -184,21 +185,20 @@ export class Hub implements HubInterface {
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
   public captureException(exception: any, hint?: EventHint): string {
-    const eventId = (this._lastEventId = hint && hint.event_id ? hint.event_id : uuid4());
     const syntheticException = new Error('Sentry syntheticException');
-    this._withClient((client, scope) => {
-      client.captureException(
-        exception,
-        {
-          originalException: exception,
-          syntheticException,
-          ...hint,
-          event_id: eventId,
-        },
-        scope,
-      );
-    });
-    return eventId;
+    this._lastEventId =
+      this._withClient((client, scope) => {
+        return client.captureException(
+          exception,
+          {
+            originalException: exception,
+            syntheticException,
+            ...hint,
+          },
+          scope,
+        );
+      }) || NIL_EVENT_ID;
+    return this._lastEventId;
   }
 
   /**
@@ -210,37 +210,37 @@ export class Hub implements HubInterface {
     level?: Severity | SeverityLevel,
     hint?: EventHint,
   ): string {
-    const eventId = (this._lastEventId = hint && hint.event_id ? hint.event_id : uuid4());
     const syntheticException = new Error(message);
-    this._withClient((client, scope) => {
-      client.captureMessage(
-        message,
-        level,
-        {
-          originalException: message,
-          syntheticException,
-          ...hint,
-          event_id: eventId,
-        },
-        scope,
-      );
-    });
-    return eventId;
+    this._lastEventId =
+      this._withClient((client, scope) => {
+        return client.captureMessage(
+          message,
+          level,
+          {
+            originalException: message,
+            syntheticException,
+            ...hint,
+          },
+          scope,
+        );
+      }) || NIL_EVENT_ID;
+    return this._lastEventId;
   }
 
   /**
    * @inheritDoc
    */
   public captureEvent(event: Event, hint?: EventHint): string {
-    const eventId = hint && hint.event_id ? hint.event_id : uuid4();
+    const clientId =
+      this._withClient((client, scope) => {
+        return client.captureEvent(event, { ...hint }, scope);
+      }) || NIL_EVENT_ID;
+
     if (event.type !== 'transaction') {
-      this._lastEventId = eventId;
+      this._lastEventId = clientId;
     }
 
-    this._withClient((client, scope) => {
-      client.captureEvent(event, { ...hint, event_id: eventId }, scope);
-    });
-    return eventId;
+    return clientId;
   }
 
   /**
@@ -469,11 +469,9 @@ export class Hub implements HubInterface {
    * @param method The method to call on the client.
    * @param args Arguments to pass to the client function.
    */
-  private _withClient(callback: (client: Client, scope: Scope | undefined) => void): void {
+  private _withClient<T>(callback: (client: Client, scope: Scope | undefined) => T): T | undefined {
     const { scope, client } = this.getStackTop();
-    if (client) {
-      callback(client, scope);
-    }
+    return client && callback(client, scope);
   }
 
   /**

diff --git a/packages/hub/test/hub.test.ts b/packages/hub/test/hub.test.ts
@@ -7,11 +7,13 @@ import { getCurrentHub, Hub, Scope } from '../src';
 
 const clientFn: any = jest.fn();
 
+const MOCK_EVENT_ID = '7bab5137428b4de29891fb8bd34a31cb';
+
 function makeClient() {
   return {
     getOptions: jest.fn(),
     captureEvent: jest.fn(),
-    captureException: jest.fn(),
+    captureException: jest.fn().mockReturnValue(MOCK_EVENT_ID),
     close: jest.fn(),
     flush: jest.fn(),
     getDsn: jest.fn(),
@@ -224,14 +226,12 @@ describe('Hub', () => {
       expect(args[0]).toBe('a');
     });
 
-    test('should set event_id in hint', () => {
+    test('should get event_id from client', () => {
       const testClient = makeClient();
       const hub = new Hub(testClient);
 
-      hub.captureException('a');
-      const args = getPassedArgs(testClient.captureException);
-
-      expect(args[1].event_id).toBeTruthy();
+      const id = hub.captureException('a');
+      expect(id).toBeDefined();
     });
 
     test('should keep event_id from hint', () => {
@@ -270,14 +270,12 @@ describe('Hub', () => {
       expect(args[0]).toBe('a');
     });
 
-    test('should set event_id in hint', () => {
+    test('should get event_id from client', () => {
       const testClient = makeClient();
       const hub = new Hub(testClient);
 
-      hub.captureMessage('a');
-      const args = getPassedArgs(testClient.captureMessage);
-
-      expect(args[2].event_id).toBeTruthy();
+      const id = hub.captureMessage('a');
+      expect(id).toBeDefined();
     });
 
     test('should keep event_id from hint', () => {
@@ -318,17 +316,15 @@ describe('Hub', () => {
       expect(args[0]).toBe(event);
     });
 
-    test('should set event_id in hint', () => {
+    test('should get event_id from client', () => {
       const event: Event = {
         extra: { b: 3 },
       };
       const testClient = makeClient();
       const hub = new Hub(testClient);
 
-      hub.captureEvent(event);
-      const args = getPassedArgs(testClient.captureEvent);
-
-      expect(args[1].event_id).toBeTruthy();
+      const id = hub.captureEvent(event);
+      expect(id).toBeDefined();
     });
 
     test('should keep event_id from hint', () => {
@@ -352,10 +348,8 @@ describe('Hub', () => {
       const testClient = makeClient();
       const hub = new Hub(testClient);
 
-      hub.captureEvent(event);
-      const args = getPassedArgs(testClient.captureEvent);
-
-      expect(args[1].event_id).toEqual(hub.lastEventId());
+      const id = hub.captureEvent(event);
+      expect(id).toEqual(hub.lastEventId());
     });
 
     test('transactions do not set lastEventId', () => {
@@ -366,10 +360,8 @@ describe('Hub', () => {
       const testClient = makeClient();
       const hub = new Hub(testClient);
 
-      hub.captureEvent(event);
-      const args = getPassedArgs(testClient.captureEvent);
-
-      expect(args[1].event_id).not.toEqual(hub.lastEventId());
+      const id = hub.captureEvent(event);
+      expect(id).not.toEqual(hub.lastEventId());
     });
   });