ref(hub): Mimic sentry-python's behaviour of creating and passing event_ids

.vscode/launch.json

@@ -30,6 +30,30 @@
       "internalConsoleOptions": "neverOpen", // since we're not using it, don't automatically switch to it
     },
 
+    // @sentry/hub - run a specific test file in watch mode
+    // must have file in currently active tab when hitting the play button
+    {
+      "type": "node",
+      "request": "launch",
+      "cwd": "${workspaceFolder}/packages/hub",
+      "name": "Debug @sentry/hub tests - just open file",
+      "program": "${workspaceFolder}/node_modules/.bin/jest",
+      "args": [
+        "--watch",
+        "--runInBand",
+        "--config",
+        "${workspaceFolder}/packages/hub/package.json",
+        "--coverage",
+        "false", // coverage messes up the source maps
+        "${relativeFile}" // remove this to run all package tests
+      ],
+      "disableOptimisticBPs": true,
+      "sourceMaps": true,
+      "smartStep": true,
+      "console": "integratedTerminal", // otherwise it goes to the VSCode debug terminal, which prints the test output or console logs (depending on "outputCapture" option here), but not both
+      "internalConsoleOptions": "neverOpen", // since we're not using it, don't automatically switch to it
+    },
+
     // @sentry/nextjs - run a specific test file in watch mode
     // must have file in currently active tab when hitting the play button
     {

packages/hub/src/hub.ts

@@ -21,7 +21,7 @@ import {
   TransactionContext,
   User,
 } from '@sentry/types';
-import { consoleSandbox, dateTimestampInSeconds, getGlobalObject, isNodeEnv, logger, uuid4 } from '@sentry/utils';
+import { consoleSandbox, dateTimestampInSeconds, getGlobalObject, isNodeEnv, logger } from '@sentry/utils';
 
 import { Scope } from './scope';
 import { Session } from './session';
@@ -185,8 +185,7 @@ export class Hub implements HubInterface {
    * @inheritDoc
    */
   // 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 = uuid4());
+  public captureException(exception: any, hint?: EventHint): string | undefined {
     let finalHint = hint;
 
     // If there's no explicit hint provided, mimic the same thing that would happen
@@ -206,18 +205,21 @@ export class Hub implements HubInterface {
       };
     }
 
-    this._invokeClient('captureException', exception, {
+    const eventId = this._invokeClient('captureException', exception, {
       ...finalHint,
-      event_id: eventId,
     });
+
+    if (eventId) {
+      this._lastEventId = eventId;
+    }
+
     return eventId;
   }
 
   /**
    * @inheritDoc
    */
-  public captureMessage(message: string, level?: Severity, hint?: EventHint): string {
-    const eventId = (this._lastEventId = uuid4());
+  public captureMessage(message: string, level?: Severity, hint?: EventHint): string | undefined {
     let finalHint = hint;
 
     // If there's no explicit hint provided, mimic the same thing that would happen
@@ -237,26 +239,29 @@ export class Hub implements HubInterface {
       };
     }
 
-    this._invokeClient('captureMessage', message, level, {
+    const eventId = this._invokeClient('captureMessage', message, level, {
       ...finalHint,
-      event_id: eventId,
     });
+
+    if (eventId) {
+      this._lastEventId = eventId;
+    }
+
     return eventId;
   }
 
   /**
    * @inheritDoc
    */
-  public captureEvent(event: Event, hint?: EventHint): string {
-    const eventId = uuid4();
+  public captureEvent(event: Event, hint?: EventHint): string | undefined {
+    const eventId = this._invokeClient('captureEvent', event, {
+      ...hint,
+    });
+
     if (event.type !== 'transaction') {
       this._lastEventId = eventId;
     }
 
-    this._invokeClient('captureEvent', event, {
-      ...hint,
-      event_id: eventId,
-    });
     return eventId;
   }
 
@@ -480,12 +485,14 @@ export class Hub implements HubInterface {
    * @param args Arguments to pass to the client function.
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  private _invokeClient<M extends keyof Client>(method: M, ...args: any[]): void {
+  private _invokeClient<M extends keyof Client>(method: M, ...args: any[]): string | undefined {
     const { scope, client } = this.getStackTop();
     if (client && client[method]) {
       // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
-      (client as any)[method](...args, scope);
+      return (client as any)[method](...args, scope);
     }
+
+    return undefined;
   }
 
   /**

packages/hub/test/hub.test.ts

@@ -1,8 +1,11 @@
+import { BrowserClient } from '@sentry/browser';
+import { NodeClient } from '@sentry/node';
 import { Event } from '@sentry/types';
 
 import { getCurrentHub, Hub, Scope } from '../src';
 
 const clientFn: any = jest.fn();
+const dsn = 'https://53039209a22b4ec1bcc296a3c9fdecd6@sentry.io/4291';
 
 describe('Hub', () => {
   afterEach(() => {
@@ -203,14 +206,6 @@ describe('Hub', () => {
       expect(spy.mock.calls[0][1]).toBe('a');
     });
 
-    test('should set event_id in hint', () => {
-      const hub = new Hub();
-      const spy = jest.spyOn(hub as any, '_invokeClient');
-      hub.captureException('a');
-      // @ts-ignore Says mock object is type unknown
-      expect(spy.mock.calls[0][2].event_id).toBeTruthy();
-    });
-
     test('should generate hint if not provided in the call', () => {
       const hub = new Hub();
       const spy = jest.spyOn(hub as any, '_invokeClient');
@@ -235,14 +230,6 @@ describe('Hub', () => {
       expect(spy.mock.calls[0][1]).toBe('a');
     });
 
-    test('should set event_id in hint', () => {
-      const hub = new Hub();
-      const spy = jest.spyOn(hub as any, '_invokeClient');
-      hub.captureMessage('a');
-      // @ts-ignore Says mock object is type unknown
-      expect(spy.mock.calls[0][3].event_id).toBeTruthy();
-    });
-
     test('should generate hint if not provided in the call', () => {
       const hub = new Hub();
       const spy = jest.spyOn(hub as any, '_invokeClient');
@@ -269,48 +256,107 @@ describe('Hub', () => {
       expect(spy.mock.calls[0][1]).toBe(event);
     });
 
-    test('should set event_id in hint', () => {
+    test('sets lastEventId', () => {
       const event: Event = {
         extra: { b: 3 },
       };
-      const hub = new Hub();
-      const spy = jest.spyOn(hub as any, '_invokeClient');
-      hub.captureEvent(event);
-      // @ts-ignore Says mock object is type unknown
-      expect(spy.mock.calls[0][2].event_id).toBeTruthy();
+
+      [BrowserClient, NodeClient].forEach(Client => {
+        const hub = new Hub(new Client({ dsn }));
+        const eventId = hub.captureEvent(event);
+
+        expect(eventId).toBeDefined();
+        expect(eventId).toEqual(hub.lastEventId());
+      });
     });
 
-    test('sets lastEventId', () => {
+    test('prefers to use eventId provided in the hint.', () => {
       const event: Event = {
         extra: { b: 3 },
       };
-      const hub = new Hub();
-      const spy = jest.spyOn(hub as any, '_invokeClient');
-      hub.captureEvent(event);
-      // @ts-ignore Says mock object is type unknown
-      expect(spy.mock.calls[0][2].event_id).toEqual(hub.lastEventId());
+
+      const customEventId = 'test_event_id';
+
+      [BrowserClient, NodeClient].forEach(Client => {
+        const hub = new Hub(new Client({ dsn }));
+        const eventId = hub.captureEvent(event, { event_id: customEventId });
+
+        expect(eventId).toBe(customEventId);
+      });
+    });
+
+    test(`doesn't return eventId if beforeSend drops the event`, () => {
+      const event: Event = {
+        extra: { b: 3 },
+      };
+
+      [BrowserClient, NodeClient].forEach(Client => {
+        const hub = new Hub(
+          new Client({
+            dsn,
+            beforeSend: (_event: any) => null,
+          }),
+        );
+        const eventId = hub.captureEvent(event);
+
+        expect(eventId).toBeUndefined();
+      });
+    });
+
+    test(`doesn't return eventId if SDK is disabled`, () => {
+      const event: Event = {
+        extra: { b: 3 },
+      };
+
+      [BrowserClient, NodeClient].forEach(Client => {
+        const hub = new Hub(new Client({ dsn, enabled: false }));
+        const eventId = hub.captureEvent(event);
+
+        expect(eventId).toBeUndefined();
+      });
+    });
+
+    test(`doesn't return eventId if sampling drops the event`, () => {
+      const event: Event = {
+        extra: { b: 3 },
+      };
+
+      [BrowserClient, NodeClient].forEach(Client => {
+        const hub = new Hub(new Client({ dsn, sampleRate: 0 }));
+        const eventId = hub.captureEvent(event);
+
+        expect(eventId).toBeUndefined();
+      });
     });
 
     test('transactions do not set lastEventId', () => {
       const event: Event = {
         extra: { b: 3 },
         type: 'transaction',
       };
-      const hub = new Hub();
-      const spy = jest.spyOn(hub as any, '_invokeClient');
-      hub.captureEvent(event);
-      // @ts-ignore Says mock object is type unknown
-      expect(spy.mock.calls[0][2].event_id).not.toEqual(hub.lastEventId());
+
+      [BrowserClient, NodeClient].forEach(Client => {
+        const hub = new Hub(new Client({ dsn }));
+        const eventId = hub.captureEvent(event);
+
+        expect(eventId).toBeDefined();
+        expect(eventId).not.toEqual(hub.lastEventId());
+      });
     });
   });
 
   test('lastEventId should be the same as last created', () => {
     const event: Event = {
       extra: { b: 3 },
     };
-    const hub = new Hub();
-    const eventId = hub.captureEvent(event);
-    expect(eventId).toBe(hub.lastEventId());
+
+    [BrowserClient, NodeClient].forEach(Client => {
+      const hub = new Hub(new Client({ dsn }));
+      const eventId = hub.captureEvent(event);
+
+      expect(eventId).toBeDefined();
+      expect(eventId).toBe(hub.lastEventId());
+    });
   });
 
   describe('run', () => {

packages/types/src/hub.ts

@@ -78,7 +78,7 @@ export interface Hub {
    * @param hint May contain additional information about the original exception.
    * @returns The generated eventId.
    */
-  captureException(exception: any, hint?: EventHint): string;
+  captureException(exception: any, hint?: EventHint): string | undefined;
 
   /**
    * Captures a message event and sends it to Sentry.
@@ -88,15 +88,15 @@ export interface Hub {
    * @param hint May contain additional information about the original exception.
    * @returns The generated eventId.
    */
-  captureMessage(message: string, level?: Severity, hint?: EventHint): string;
+  captureMessage(message: string, level?: Severity, hint?: EventHint): string | undefined;
 
   /**
    * Captures a manually created event and sends it to Sentry.
    *
    * @param event The event to send to Sentry.
    * @param hint May contain additional information about the original exception.
    */
-  captureEvent(event: Event, hint?: EventHint): string;
+  captureEvent(event: Event, hint?: EventHint): string | undefined;
 
   /**
    * This is the getter for lastEventId.