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.

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

ref(hub): Remove _invokeClient #4195

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

- ref(hub): Remove `_invokeClient` (#4195)
- **breaking** feat(minimal): Remove `_callOnClient` (#4195)

`_callOnClient` was a hidden API that was not meant for public use. If this breaks your set up, please write a GitHub issue describing your usage of `_callOnClient` and we can re-evaluate accordingly.
lobsterkatie marked this conversation as resolved.
Show resolved Hide resolved

## 6.15.0

- fix(browser): Capture stacktrace on `DOMExceptions`, if possible (#4160)
Expand Down
25 changes: 10 additions & 15 deletions packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,8 @@ export class Hub implements HubInterface {
};
}

this._invokeClient('captureException', exception, {
...finalHint,
event_id: eventId,
this._withClient((client, scope) => {
client.captureException(exception, { ...finalHint, event_id: eventId }, scope);
});
return eventId;
}
Expand Down Expand Up @@ -237,9 +236,8 @@ export class Hub implements HubInterface {
};
}

this._invokeClient('captureMessage', message, level, {
...finalHint,
event_id: eventId,
this._withClient((client, scope) => {
client.captureMessage(message, level, { ...finalHint, event_id: eventId }, scope);
});
return eventId;
}
Expand All @@ -253,9 +251,8 @@ export class Hub implements HubInterface {
this._lastEventId = eventId;
}

this._invokeClient('captureEvent', event, {
...hint,
event_id: eventId,
this._withClient((client, scope) => {
client.captureEvent(event, { ...hint, event_id: eventId }, scope);
});
return eventId;
}
Expand Down Expand Up @@ -476,15 +473,13 @@ export class Hub implements HubInterface {
/**
* Internal helper function to call a method on the top client if it exists.
*
* @param method The method to call on the client.
* @param args Arguments to pass to the client function.
* @param callback Callback that gets passed client and scope from stack
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private _invokeClient<M extends keyof Client>(method: M, ...args: any[]): void {
private _withClient(callback: (client: Client, scope: Scope | undefined) => void): void {
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);
if (client) {
callback(client, scope);
}
}

Expand Down
100 changes: 42 additions & 58 deletions packages/hub/test/hub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ describe('Hub', () => {
expect(hub.getStack()).toHaveLength(1);
});

test("don't invoke client sync with wrong func", () => {
const hub = new Hub(clientFn);
// @ts-ignore we want to able to call private method
hub._invokeClient('funca', true);
expect(clientFn).not.toHaveBeenCalled();
});

test('isOlderThan', () => {
const hub = new Hub();
expect(hub.isOlderThan(0)).toBeFalsy();
Expand Down Expand Up @@ -194,113 +187,104 @@ describe('Hub', () => {
});

describe('captureException', () => {
const mockClient: any = { captureException: jest.fn() };
beforeEach(() => {
mockClient.captureException.mockClear();
});

test('simple', () => {
const hub = new Hub();
const spy = jest.spyOn(hub as any, '_invokeClient');
const hub = new Hub(mockClient);
hub.captureException('a');
expect(spy).toHaveBeenCalled();
expect(spy.mock.calls[0][0]).toBe('captureException');
expect(spy.mock.calls[0][1]).toBe('a');
expect(mockClient.captureException).toHaveBeenCalled();
expect(mockClient.captureException.mock.calls[0][0]).toBe('a');
});

test('should set event_id in hint', () => {
const hub = new Hub();
const spy = jest.spyOn(hub as any, '_invokeClient');
const hub = new Hub(mockClient);
hub.captureException('a');
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][2].event_id).toBeTruthy();
expect(mockClient.captureException.mock.calls[0][1].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');
const hub = new Hub(mockClient);
const ex = new Error('foo');
hub.captureException(ex);
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][2].originalException).toBe(ex);
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][2].syntheticException).toBeInstanceOf(Error);
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][2].syntheticException.message).toBe('Sentry syntheticException');
expect(mockClient.captureException.mock.calls[0][1].originalException).toBe(ex);
expect(mockClient.captureException.mock.calls[0][1].syntheticException).toBeInstanceOf(Error);
expect(mockClient.captureException.mock.calls[0][1].syntheticException.message).toBe('Sentry syntheticException');
lobsterkatie marked this conversation as resolved.
Show resolved Hide resolved
});
});

describe('captureMessage', () => {
const mockClient: any = { captureMessage: jest.fn() };
beforeEach(() => {
mockClient.captureMessage.mockClear();
});

test('simple', () => {
const hub = new Hub();
const spy = jest.spyOn(hub as any, '_invokeClient');
const hub = new Hub(mockClient);
hub.captureMessage('a');
expect(spy).toHaveBeenCalled();
expect(spy.mock.calls[0][0]).toBe('captureMessage');
expect(spy.mock.calls[0][1]).toBe('a');
expect(mockClient.captureMessage).toHaveBeenCalled();
expect(mockClient.captureMessage.mock.calls[0][0]).toBe('a');
});

test('should set event_id in hint', () => {
const hub = new Hub();
const spy = jest.spyOn(hub as any, '_invokeClient');
const hub = new Hub(mockClient);
hub.captureMessage('a');
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][3].event_id).toBeTruthy();
expect(mockClient.captureMessage.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');
const hub = new Hub(mockClient);
hub.captureMessage('foo');
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][3].originalException).toBe('foo');
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][3].syntheticException).toBeInstanceOf(Error);
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][3].syntheticException.message).toBe('foo');
expect(mockClient.captureMessage.mock.calls[0][2].originalException).toBe('foo');
expect(mockClient.captureMessage.mock.calls[0][2].syntheticException).toBeInstanceOf(Error);
expect(mockClient.captureMessage.mock.calls[0][2].syntheticException.message).toBe('foo');
});
});

describe('captureEvent', () => {
const mockClient: any = { captureEvent: jest.fn() };
beforeEach(() => {
mockClient.captureEvent.mockClear();
});

test('simple', () => {
const event: Event = {
extra: { b: 3 },
};
const hub = new Hub();
const spy = jest.spyOn(hub as any, '_invokeClient');
const hub = new Hub(mockClient);
hub.captureEvent(event);
expect(spy).toHaveBeenCalled();
expect(spy.mock.calls[0][0]).toBe('captureEvent');
expect(spy.mock.calls[0][1]).toBe(event);
expect(mockClient.captureEvent).toHaveBeenCalled();
expect(mockClient.captureEvent.mock.calls[0][0]).toBe(event);
});

test('should set event_id in hint', () => {
const event: Event = {
extra: { b: 3 },
};
const hub = new Hub();
const spy = jest.spyOn(hub as any, '_invokeClient');
const hub = new Hub(mockClient);
hub.captureEvent(event);
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][2].event_id).toBeTruthy();
expect(mockClient.captureEvent.mock.calls[0][1].event_id).toBeTruthy();
});

test('sets lastEventId', () => {
const event: Event = {
extra: { b: 3 },
};
const hub = new Hub();
const spy = jest.spyOn(hub as any, '_invokeClient');
const hub = new Hub(mockClient);
hub.captureEvent(event);
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][2].event_id).toEqual(hub.lastEventId());
expect(mockClient.captureEvent.mock.calls[0][1].event_id).toEqual(hub.lastEventId());
});

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');
const hub = new Hub(mockClient);
hub.captureEvent(event);
// @ts-ignore Says mock object is type unknown
expect(spy.mock.calls[0][2].event_id).not.toEqual(hub.lastEventId());
expect(mockClient.captureEvent.mock.calls[0][1].event_id).not.toEqual(hub.lastEventId());
});
});

Expand Down
15 changes: 0 additions & 15 deletions packages/minimal/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,21 +179,6 @@ export function withScope(callback: (scope: Scope) => void): void {
callOnHub<void>('withScope', callback);
}

/**
* Calls a function on the latest client. Use this with caution, it's meant as
* in "internal" helper so we don't need to expose every possible function in
* the shim. It is not guaranteed that the client actually implements the
* function.
*
* @param method The method to call on the client/client.
* @param args Arguments to pass to the client/fontend.
* @hidden
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function _callOnClient(method: string, ...args: any[]): void {
callOnHub<void>('_invokeClient', method, ...args);
}

/**
* Starts a new `Transaction` and returns it. This is the entry point to manual tracing instrumentation.
*
Expand Down
12 changes: 0 additions & 12 deletions packages/minimal/test/lib/minimal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { getCurrentHub, getHubFromCarrier, Scope } from '@sentry/hub';
import { Severity } from '@sentry/types';

import {
_callOnClient,
captureEvent,
captureException,
captureMessage,
Expand Down Expand Up @@ -197,17 +196,6 @@ describe('Minimal', () => {
expect(getCurrentHub().getClient()).toBe(TestClient.instance);
});

test('Calls function on the client', done => {
const s = jest.spyOn(TestClient.prototype, 'mySecretPublicMethod');
getCurrentHub().withScope(() => {
getCurrentHub().bindClient(new TestClient({}) as any);
_callOnClient('mySecretPublicMethod', 'test');
expect(s.mock.calls[0][0]).toBe('test');
s.mockRestore();
done();
});
});

test('does not throw an error when pushing different clients', () => {
init({});
expect(() => {
Expand Down