diff --git a/CHANGELOG.md b/CHANGELOG.md index 811f659948ce..e35fad0ddb75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. + ## 6.15.0 - fix(browser): Capture stacktrace on `DOMExceptions`, if possible (#4160) diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index 2c1f6b297faf..6c5173d0d1c4 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -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; } @@ -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; } @@ -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; } @@ -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(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); } } diff --git a/packages/hub/test/hub.test.ts b/packages/hub/test/hub.test.ts index 4e865fa10412..becbcbffe591 100644 --- a/packages/hub/test/hub.test.ts +++ b/packages/hub/test/hub.test.ts @@ -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(); @@ -194,101 +187,94 @@ 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'); }); }); 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', () => { @@ -296,11 +282,9 @@ describe('Hub', () => { 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()); }); }); diff --git a/packages/minimal/src/index.ts b/packages/minimal/src/index.ts index 117c8a769b92..7325c95cb97d 100644 --- a/packages/minimal/src/index.ts +++ b/packages/minimal/src/index.ts @@ -179,21 +179,6 @@ export function withScope(callback: (scope: Scope) => void): void { callOnHub('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('_invokeClient', method, ...args); -} - /** * Starts a new `Transaction` and returns it. This is the entry point to manual tracing instrumentation. * diff --git a/packages/minimal/test/lib/minimal.test.ts b/packages/minimal/test/lib/minimal.test.ts index 9e25bb0a46df..c82a26faae74 100644 --- a/packages/minimal/test/lib/minimal.test.ts +++ b/packages/minimal/test/lib/minimal.test.ts @@ -2,7 +2,6 @@ import { getCurrentHub, getHubFromCarrier, Scope } from '@sentry/hub'; import { Severity } from '@sentry/types'; import { - _callOnClient, captureEvent, captureException, captureMessage, @@ -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(() => {