Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "fix(core): Only generate eventIds in client" #6392

Merged
merged 1 commit into from
Dec 2, 2022
Merged

Revert "fix(core): Only generate eventIds in client" #6392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/core/src/baseclient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
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))
Expand All @@ -149,7 +150,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
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)
Expand All @@ -176,7 +177,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
return;
}

let eventId: string | undefined;
let eventId: string | undefined = hint && hint.event_id;

this._process(
this._captureEvent(event, hint, scope).then(result => {
Expand Down
78 changes: 40 additions & 38 deletions packages/core/src/hub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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<T>(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);
}
}

/**
Expand Down
40 changes: 24 additions & 16 deletions packages/hub/test/hub.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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());
});
});

Expand Down