From 62375f91883301513f7f45524fecce05d7fae5e2 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 2 Dec 2022 13:09:18 +0100 Subject: [PATCH] Revert "fix(core): Only generate eventIds in client (#6247)" This reverts commit 3fe5f7a7ab1cc2c09e30f1d3d7035049e7edaaa6. --- packages/core/src/baseclient.ts | 7 +-- packages/core/src/hub.ts | 78 +++++++++++++++++---------------- packages/hub/test/hub.test.ts | 40 ++++++++++------- 3 files changed, 68 insertions(+), 57 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index d4aeeca5fc8a..3d0a90faefbc 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -127,7 +127,8 @@ export abstract class BaseClient implements Client { return; } - let eventId: string | undefined; + let eventId: string | undefined = hint && hint.event_id; + this._process( this.eventFromException(exception, hint) .then(event => this._captureEvent(event, hint, scope)) @@ -149,7 +150,7 @@ export abstract class BaseClient implements Client { hint?: EventHint, scope?: Scope, ): string | undefined { - let eventId: string | undefined; + let eventId: string | undefined = hint && hint.event_id; const promisedEvent = isPrimitive(message) ? this.eventFromMessage(String(message), level, hint) @@ -176,7 +177,7 @@ export abstract class BaseClient implements Client { return; } - let eventId: string | undefined; + let eventId: string | undefined = hint && hint.event_id; this._process( this._captureEvent(event, hint, scope).then(result => { diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts index 1966269a9b47..5036158e08eb 100644 --- a/packages/core/src/hub.ts +++ b/packages/core/src/hub.ts @@ -27,13 +27,12 @@ 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. * @@ -185,20 +184,21 @@ 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._lastEventId = - this._withClient((client, scope) => { - return client.captureException( - exception, - { - originalException: exception, - syntheticException, - ...hint, - }, - scope, - ); - }) || NIL_EVENT_ID; - return this._lastEventId; + this._withClient((client, scope) => { + client.captureException( + exception, + { + originalException: exception, + syntheticException, + ...hint, + event_id: eventId, + }, + scope, + ); + }); + return eventId; } /** @@ -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._lastEventId = - this._withClient((client, scope) => { - return client.captureMessage( - message, - level, - { - originalException: message, - syntheticException, - ...hint, - }, - scope, - ); - }) || NIL_EVENT_ID; - return this._lastEventId; + this._withClient((client, scope) => { + client.captureMessage( + message, + level, + { + originalException: message, + syntheticException, + ...hint, + event_id: eventId, + }, + scope, + ); + }); + return eventId; } /** * @inheritDoc */ public captureEvent(event: Event, hint?: EventHint): string { - const clientId = - this._withClient((client, scope) => { - return client.captureEvent(event, { ...hint }, scope); - }) || NIL_EVENT_ID; - + const eventId = hint && hint.event_id ? hint.event_id : uuid4(); if (event.type !== 'transaction') { - this._lastEventId = clientId; + this._lastEventId = eventId; } - return clientId; + this._withClient((client, scope) => { + client.captureEvent(event, { ...hint, event_id: eventId }, scope); + }); + return eventId; } /** @@ -469,9 +469,11 @@ 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) => T): T | undefined { + private _withClient(callback: (client: Client, scope: Scope | undefined) => void): void { const { scope, client } = this.getStackTop(); - return client && callback(client, scope); + if (client) { + callback(client, scope); + } } /** diff --git a/packages/hub/test/hub.test.ts b/packages/hub/test/hub.test.ts index 2a1c9d4754a4..5ce521ead4c4 100644 --- a/packages/hub/test/hub.test.ts +++ b/packages/hub/test/hub.test.ts @@ -7,13 +7,11 @@ 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().mockReturnValue(MOCK_EVENT_ID), + captureException: jest.fn(), close: jest.fn(), flush: jest.fn(), getDsn: jest.fn(), @@ -226,12 +224,14 @@ describe('Hub', () => { expect(args[0]).toBe('a'); }); - test('should get event_id from client', () => { + test('should set event_id in hint', () => { const testClient = makeClient(); const hub = new Hub(testClient); - const id = hub.captureException('a'); - expect(id).toBeDefined(); + hub.captureException('a'); + const args = getPassedArgs(testClient.captureException); + + expect(args[1].event_id).toBeTruthy(); }); test('should keep event_id from hint', () => { @@ -270,12 +270,14 @@ describe('Hub', () => { expect(args[0]).toBe('a'); }); - test('should get event_id from client', () => { + test('should set event_id in hint', () => { const testClient = makeClient(); const hub = new Hub(testClient); - const id = hub.captureMessage('a'); - expect(id).toBeDefined(); + hub.captureMessage('a'); + const args = getPassedArgs(testClient.captureMessage); + + expect(args[2].event_id).toBeTruthy(); }); test('should keep event_id from hint', () => { @@ -316,15 +318,17 @@ describe('Hub', () => { expect(args[0]).toBe(event); }); - test('should get event_id from client', () => { + test('should set event_id in hint', () => { const event: Event = { extra: { b: 3 }, }; const testClient = makeClient(); const hub = new Hub(testClient); - const id = hub.captureEvent(event); - expect(id).toBeDefined(); + hub.captureEvent(event); + const args = getPassedArgs(testClient.captureEvent); + + expect(args[1].event_id).toBeTruthy(); }); test('should keep event_id from hint', () => { @@ -348,8 +352,10 @@ describe('Hub', () => { const testClient = makeClient(); const hub = new Hub(testClient); - const id = hub.captureEvent(event); - expect(id).toEqual(hub.lastEventId()); + hub.captureEvent(event); + const args = getPassedArgs(testClient.captureEvent); + + expect(args[1].event_id).toEqual(hub.lastEventId()); }); test('transactions do not set lastEventId', () => { @@ -360,8 +366,10 @@ describe('Hub', () => { const testClient = makeClient(); const hub = new Hub(testClient); - const id = hub.captureEvent(event); - expect(id).not.toEqual(hub.lastEventId()); + hub.captureEvent(event); + const args = getPassedArgs(testClient.captureEvent); + + expect(args[1].event_id).not.toEqual(hub.lastEventId()); }); });