From 64397e5bfe2d894d54db16fe1dd70b73f7cda8d9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 18 Dec 2023 17:53:35 +0100 Subject: [PATCH 1/2] ref(integrations): Refactor pluggable integrations to avoid `setupOnce` --- packages/integrations/src/captureconsole.ts | 57 +++++++++++-------- packages/integrations/src/debug.ts | 56 ++++++++++-------- packages/integrations/src/httpclient.ts | 52 ++++++++++------- .../integrations/src/reportingobserver.ts | 31 +++++----- 4 files changed, 114 insertions(+), 82 deletions(-) diff --git a/packages/integrations/src/captureconsole.ts b/packages/integrations/src/captureconsole.ts index 8d7d2540bc2c..09e6fc8d7ce1 100644 --- a/packages/integrations/src/captureconsole.ts +++ b/packages/integrations/src/captureconsole.ts @@ -1,4 +1,5 @@ -import type { EventProcessor, Hub, Integration } from '@sentry/types'; +import { captureException, captureMessage, getClient, withScope } from '@sentry/core'; +import type { CaptureContext, Client, EventProcessor, Hub, Integration } from '@sentry/types'; import { CONSOLE_LEVELS, GLOBAL_OBJ, @@ -36,7 +37,12 @@ export class CaptureConsole implements Integration { /** * @inheritDoc */ - public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + public setupOnce(_: (callback: EventProcessor) => void, _getCurrentHub: () => Hub): void { + // noop + } + + /** @inheritdoc */ + public client(client: Client): void { if (!('console' in GLOBAL_OBJ)) { return; } @@ -44,25 +50,24 @@ export class CaptureConsole implements Integration { const levels = this._levels; addConsoleInstrumentationHandler(({ args, level }) => { - if (!levels.includes(level)) { + if (getClient() !== client || !levels.includes(level)) { return; } - const hub = getCurrentHub(); - - if (!hub.getIntegration(CaptureConsole)) { - return; - } - - consoleHandler(hub, args, level); + consoleHandler(args, level); }); } } -function consoleHandler(hub: Hub, args: unknown[], level: string): void { - hub.withScope(scope => { - scope.setLevel(severityLevelFromString(level)); - scope.setExtra('arguments', args); +function consoleHandler(args: unknown[], level: string): void { + const captureContext: CaptureContext = { + level: severityLevelFromString(level), + extra: { + arguments: args, + }, + }; + + withScope(scope => { scope.addEventProcessor(event => { event.logger = 'console'; @@ -74,18 +79,20 @@ function consoleHandler(hub: Hub, args: unknown[], level: string): void { return event; }); - let message = safeJoin(args, ' '); + if (level === 'assert' && args[0] === false) { + const message = `Assertion failed: ${safeJoin(args.slice(1), ' ') || 'console.assert'}`; + scope.setExtra('arguments', args.slice(1)); + captureMessage(message, captureContext); + return; + } + const error = args.find(arg => arg instanceof Error); - if (level === 'assert') { - if (args[0] === false) { - message = `Assertion failed: ${safeJoin(args.slice(1), ' ') || 'console.assert'}`; - scope.setExtra('arguments', args.slice(1)); - hub.captureMessage(message); - } - } else if (level === 'error' && error) { - hub.captureException(error); - } else { - hub.captureMessage(message); + if (level === 'error' && error) { + captureException(error, captureContext); + return; } + + const message = safeJoin(args, ' '); + captureMessage(message, captureContext); }); } diff --git a/packages/integrations/src/debug.ts b/packages/integrations/src/debug.ts index 1c2459636510..bb8ed8924254 100644 --- a/packages/integrations/src/debug.ts +++ b/packages/integrations/src/debug.ts @@ -1,4 +1,4 @@ -import type { Event, EventHint, EventProcessor, Hub, Integration } from '@sentry/types'; +import type { Client, Event, EventHint, EventProcessor, Hub, Integration } from '@sentry/types'; import { consoleSandbox } from '@sentry/utils'; interface DebugOptions { @@ -38,32 +38,40 @@ export class Debug implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobalEventProcessor: (eventProcessor: EventProcessor) => void, getCurrentHub: () => Hub): void { - const client = getCurrentHub().getClient(); + public setupOnce( + _addGlobalEventProcessor: (eventProcessor: EventProcessor) => void, + _getCurrentHub: () => Hub, + ): void { + // noop + } - if (client && client.on) { - client.on('beforeSendEvent', (event: Event, hint?: EventHint) => { - if (this._options.debugger) { - // eslint-disable-next-line no-debugger - debugger; - } + /** @inheritdoc */ + public setup(client: Client): void { + if (!client.on) { + return; + } + + client.on('beforeSendEvent', (event: Event, hint?: EventHint) => { + if (this._options.debugger) { + // eslint-disable-next-line no-debugger + debugger; + } - /* eslint-disable no-console */ - consoleSandbox(() => { - if (this._options.stringify) { - console.log(JSON.stringify(event, null, 2)); - if (hint && Object.keys(hint).length) { - console.log(JSON.stringify(hint, null, 2)); - } - } else { - console.log(event); - if (hint && Object.keys(hint).length) { - console.log(hint); - } + /* eslint-disable no-console */ + consoleSandbox(() => { + if (this._options.stringify) { + console.log(JSON.stringify(event, null, 2)); + if (hint && Object.keys(hint).length) { + console.log(JSON.stringify(hint, null, 2)); } - }); - /* eslint-enable no-console */ + } else { + console.log(event); + if (hint && Object.keys(hint).length) { + console.log(hint); + } + } }); - } + /* eslint-enable no-console */ + }); } } diff --git a/packages/integrations/src/httpclient.ts b/packages/integrations/src/httpclient.ts index c03cd63e6840..bcb4429b7aeb 100644 --- a/packages/integrations/src/httpclient.ts +++ b/packages/integrations/src/httpclient.ts @@ -1,5 +1,6 @@ -import { getClient, isSentryRequestUrl } from '@sentry/core'; +import { captureEvent, getClient, isSentryRequestUrl } from '@sentry/core'; import type { + Client, Event as SentryEvent, EventProcessor, Hub, @@ -20,6 +21,7 @@ import { DEBUG_BUILD } from './debug-build'; export type HttpStatusCodeRange = [number, number] | number; export type HttpRequestTarget = string | RegExp; + interface HttpClientOptions { /** * HTTP status codes that should be considered failed. @@ -55,11 +57,6 @@ export class HttpClient implements Integration { private readonly _options: HttpClientOptions; - /** - * Returns current hub. - */ - private _getCurrentHub?: () => Hub; - /** * @inheritDoc * @@ -79,10 +76,14 @@ export class HttpClient implements Integration { * * @param options */ - public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { - this._getCurrentHub = getCurrentHub; - this._wrapFetch(); - this._wrapXHR(); + public setupOnce(_: (callback: EventProcessor) => void, _getCurrentHub: () => Hub): void { + // noop + } + + /** @inheritdoc */ + public setup(client: Client): void { + this._wrapFetch(client); + this._wrapXHR(client); } /** @@ -93,13 +94,12 @@ export class HttpClient implements Integration { * @param requestInit The request init object */ private _fetchResponseHandler(requestInfo: RequestInfo, response: Response, requestInit?: RequestInit): void { - if (this._getCurrentHub && this._shouldCaptureResponse(response.status, response.url)) { + if (this._shouldCaptureResponse(response.status, response.url)) { const request = _getRequest(requestInfo, requestInit); - const hub = this._getCurrentHub(); let requestHeaders, responseHeaders, requestCookies, responseCookies; - if (hub.shouldSendDefaultPii()) { + if (_shouldSendDefaultPii()) { [{ headers: requestHeaders, cookies: requestCookies }, { headers: responseHeaders, cookies: responseCookies }] = [ { cookieHeader: 'Cookie', obj: request }, @@ -135,7 +135,7 @@ export class HttpClient implements Integration { responseCookies, }); - hub.captureEvent(event); + captureEvent(event); } } @@ -147,11 +147,10 @@ export class HttpClient implements Integration { * @param headers The HTTP headers */ private _xhrResponseHandler(xhr: XMLHttpRequest, method: string, headers: Record): void { - if (this._getCurrentHub && this._shouldCaptureResponse(xhr.status, xhr.responseURL)) { + if (this._shouldCaptureResponse(xhr.status, xhr.responseURL)) { let requestHeaders, responseCookies, responseHeaders; - const hub = this._getCurrentHub(); - if (hub.shouldSendDefaultPii()) { + if (_shouldSendDefaultPii()) { try { const cookieString = xhr.getResponseHeader('Set-Cookie') || xhr.getResponseHeader('set-cookie') || undefined; @@ -181,7 +180,7 @@ export class HttpClient implements Integration { responseCookies, }); - hub.captureEvent(event); + captureEvent(event); } } @@ -296,12 +295,16 @@ export class HttpClient implements Integration { /** * Wraps `fetch` function to capture request and response data */ - private _wrapFetch(): void { + private _wrapFetch(client: Client): void { if (!supportsNativeFetch()) { return; } addFetchInstrumentationHandler(handlerData => { + if (getClient() !== client) { + return; + } + const { response, args } = handlerData; const [requestInfo, requestInit] = args as [RequestInfo, RequestInit | undefined]; @@ -316,12 +319,16 @@ export class HttpClient implements Integration { /** * Wraps XMLHttpRequest to capture request and response data */ - private _wrapXHR(): void { + private _wrapXHR(client: Client): void { if (!('XMLHttpRequest' in GLOBAL_OBJ)) { return; } addXhrInstrumentationHandler(handlerData => { + if (getClient() !== client) { + return; + } + const xhr = handlerData.xhr as SentryWrappedXMLHttpRequest & XMLHttpRequest; const sentryXhrData = xhr[SENTRY_XHR_DATA_KEY]; @@ -418,3 +425,8 @@ function _getRequest(requestInfo: RequestInfo, requestInit?: RequestInit): Reque return new Request(requestInfo, requestInit); } + +function _shouldSendDefaultPii(): boolean { + const client = getClient(); + return client ? Boolean(client.getOptions().sendDefaultPii) : false; +} diff --git a/packages/integrations/src/reportingobserver.ts b/packages/integrations/src/reportingobserver.ts index 65d3996963b1..dbcae7f014e2 100644 --- a/packages/integrations/src/reportingobserver.ts +++ b/packages/integrations/src/reportingobserver.ts @@ -1,4 +1,5 @@ -import type { EventProcessor, Hub, Integration } from '@sentry/types'; +import { captureMessage, getClient, withScope } from '@sentry/core'; +import type { Client, EventProcessor, Hub, Integration } from '@sentry/types'; import { GLOBAL_OBJ, supportsReportingObserver } from '@sentry/utils'; const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window; @@ -39,6 +40,8 @@ interface InterventionReportBody { columnNumber?: number; } +const SETUP_CLIENTS: Client[] = []; + /** Reporting API integration - https://w3c.github.io/reporting/ */ export class ReportingObserver implements Integration { /** @@ -51,11 +54,6 @@ export class ReportingObserver implements Integration { */ public readonly name: string; - /** - * Returns current hub. - */ - private _getCurrentHub?: () => Hub; - private readonly _types: ReportTypes[]; /** @@ -74,13 +72,11 @@ export class ReportingObserver implements Integration { /** * @inheritDoc */ - public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + public setupOnce(_: (callback: EventProcessor) => void, _getCurrentHub: () => Hub): void { if (!supportsReportingObserver()) { return; } - this._getCurrentHub = getCurrentHub; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any const observer = new (WINDOW as any).ReportingObserver(this.handler.bind(this), { buffered: true, @@ -91,16 +87,25 @@ export class ReportingObserver implements Integration { observer.observe(); } + /** @inheritdoc */ + public setup(client: Client): void { + if (!supportsReportingObserver()) { + return; + } + + SETUP_CLIENTS.push(client); + } + /** * @inheritDoc */ public handler(reports: Report[]): void { - const hub = this._getCurrentHub && this._getCurrentHub(); - if (!hub || !hub.getIntegration(ReportingObserver)) { + if (!SETUP_CLIENTS.includes(getClient() as Client)) { return; } + for (const report of reports) { - hub.withScope(scope => { + withScope(scope => { scope.setExtra('url', report.url); const label = `ReportingObserver [${report.type}]`; @@ -129,7 +134,7 @@ export class ReportingObserver implements Integration { } } - hub.captureMessage(`${label}: ${details}`); + captureMessage(`${label}: ${details}`); }); } } From faccf3a33e6fa96b0dbbca1ff5260043cd580f95 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 19 Dec 2023 15:24:58 +0100 Subject: [PATCH 2/2] fix & tests --- packages/integrations/src/captureconsole.ts | 2 +- .../integrations/test/captureconsole.test.ts | 278 ++++++------------ packages/integrations/test/debug.test.ts | 12 +- .../test/reportingobserver.test.ts | 112 ++++--- 4 files changed, 159 insertions(+), 245 deletions(-) diff --git a/packages/integrations/src/captureconsole.ts b/packages/integrations/src/captureconsole.ts index 09e6fc8d7ce1..a1792573c9b1 100644 --- a/packages/integrations/src/captureconsole.ts +++ b/packages/integrations/src/captureconsole.ts @@ -42,7 +42,7 @@ export class CaptureConsole implements Integration { } /** @inheritdoc */ - public client(client: Client): void { + public setup(client: Client): void { if (!('console' in GLOBAL_OBJ)) { return; } diff --git a/packages/integrations/test/captureconsole.test.ts b/packages/integrations/test/captureconsole.test.ts index 49770f0adf09..23a410f0bb33 100644 --- a/packages/integrations/test/captureconsole.test.ts +++ b/packages/integrations/test/captureconsole.test.ts @@ -1,5 +1,7 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import type { ConsoleLevel, Event, Hub, Integration } from '@sentry/types'; + +import * as SentryCore from '@sentry/core'; +import type { Client, ConsoleLevel, Event } from '@sentry/types'; import { CONSOLE_LEVELS, GLOBAL_OBJ, @@ -20,34 +22,32 @@ const mockConsole: { [key in ConsoleLevel]: jest.Mock } = { trace: jest.fn(), }; -function getMockHub(integration: Integration): Hub { +describe('CaptureConsole setup', () => { + // Ensure we've initialized the instrumentation so we can get the original one + addConsoleInstrumentationHandler(() => {}); + const _originalConsoleMethods = Object.assign({}, originalConsoleMethods); + + let mockClient: Client; + const mockScope = { - setLevel: jest.fn(), setExtra: jest.fn(), addEventProcessor: jest.fn(), }; - const mockHub = { - withScope: jest.fn(callback => { - callback(mockScope); - }), - captureMessage: jest.fn(), - captureException: jest.fn(), - getScope: jest.fn(() => mockScope), - }; + const captureMessage = jest.fn(); + const captureException = jest.fn(); + const withScope = jest.fn(callback => { + return callback(mockScope); + }); - return { - ...mockHub, - getIntegration: jest.fn(() => integration), - } as unknown as Hub; -} + beforeEach(() => { + mockClient = {} as Client; -describe('CaptureConsole setup', () => { - // Ensure we've initialized the instrumentation so we can get the original one - addConsoleInstrumentationHandler(() => {}); - const _originalConsoleMethods = Object.assign({}, originalConsoleMethods); + jest.spyOn(SentryCore, 'captureMessage').mockImplementation(captureMessage); + jest.spyOn(SentryCore, 'captureException').mockImplementation(captureException); + jest.spyOn(SentryCore, 'getClient').mockImplementation(() => mockClient); + jest.spyOn(SentryCore, 'withScope').mockImplementation(withScope); - beforeEach(() => { CONSOLE_LEVELS.forEach(key => { originalConsoleMethods[key] = mockConsole[key]; }); @@ -66,26 +66,18 @@ describe('CaptureConsole setup', () => { describe('monkeypatching', () => { it('should patch user-configured console levels', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['log', 'warn'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.error('msg 1'); GLOBAL_OBJ.console.log('msg 2'); GLOBAL_OBJ.console.warn('msg 3'); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(2); + expect(captureMessage).toHaveBeenCalledTimes(2); }); it('should fall back to default console levels if none are provided', () => { const captureConsoleIntegration = new CaptureConsole(); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); // Assert has a special handling (['debug', 'info', 'warn', 'error', 'log', 'trace'] as const).forEach(key => { @@ -94,22 +86,18 @@ describe('CaptureConsole setup', () => { GLOBAL_OBJ.console.assert(false); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(7); + expect(captureMessage).toHaveBeenCalledTimes(7); }); it('should not wrap any functions with an empty levels option', () => { const captureConsoleIntegration = new CaptureConsole({ levels: [] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); CONSOLE_LEVELS.forEach(key => { GLOBAL_OBJ.console[key]('msg'); }); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(0); + expect(captureMessage).toHaveBeenCalledTimes(0); }); }); @@ -119,76 +107,27 @@ describe('CaptureConsole setup', () => { delete GLOBAL_OBJ.console; const captureConsoleIntegration = new CaptureConsole(); - const mockHub = getMockHub(captureConsoleIntegration); expect(() => { - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); }).not.toThrow(); // reinstate initial console GLOBAL_OBJ.console = consoleRef; }); - it('should set a level in the scope when console function is called', () => { - const captureConsoleIntegration = new CaptureConsole({ levels: ['error'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); - - const mockScope = mockHub.getScope(); - - // call a wrapped function - GLOBAL_OBJ.console.error('some logging message'); - - expect(mockScope.setLevel).toHaveBeenCalledTimes(1); - expect(mockScope.setLevel).toHaveBeenCalledWith('error'); - }); - - it('should send arguments as extra data', () => { - const captureConsoleIntegration = new CaptureConsole({ levels: ['log'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); - - const mockScope = mockHub.getScope(); - - GLOBAL_OBJ.console.log('some arg 1', 'some arg 2'); - - expect(mockScope.setExtra).toHaveBeenCalledTimes(1); - expect(mockScope.setExtra).toHaveBeenCalledWith('arguments', ['some arg 1', 'some arg 2']); - }); - it('should send empty arguments as extra data', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['log'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); - - const mockScope = mockHub.getScope(); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.log(); - expect(mockScope.setExtra).toHaveBeenCalledTimes(1); - expect(mockScope.setExtra).toHaveBeenCalledWith('arguments', []); + expect(captureMessage).toHaveBeenCalledTimes(1); + expect(captureMessage).toHaveBeenCalledWith('', { extra: { arguments: [] }, level: 'log' }); }); it('should add an event processor that sets the `logger` field of events', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['log'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); - - const mockScope = mockHub.getScope(); + captureConsoleIntegration.setup(mockClient); // call a wrapped function GLOBAL_OBJ.console.log('some message'); @@ -204,135 +143,119 @@ describe('CaptureConsole setup', () => { it('should capture message on a failed assertion', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['assert'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); - - const mockScope = mockHub.getScope(); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.assert(1 + 1 === 3); expect(mockScope.setExtra).toHaveBeenLastCalledWith('arguments', []); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(1); - expect(mockHub.captureMessage).toHaveBeenCalledWith('Assertion failed: console.assert'); + expect(captureMessage).toHaveBeenCalledTimes(1); + expect(captureMessage).toHaveBeenCalledWith('Assertion failed: console.assert', { + extra: { arguments: [false] }, + level: 'log', + }); }); it('should capture correct message on a failed assertion with message', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['assert'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); - - const mockScope = mockHub.getScope(); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.assert(1 + 1 === 3, 'expression is false'); expect(mockScope.setExtra).toHaveBeenLastCalledWith('arguments', ['expression is false']); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(1); - expect(mockHub.captureMessage).toHaveBeenCalledWith('Assertion failed: expression is false'); + expect(captureMessage).toHaveBeenCalledTimes(1); + expect(captureMessage).toHaveBeenCalledWith('Assertion failed: expression is false', { + extra: { arguments: [false, 'expression is false'] }, + level: 'log', + }); }); it('should not capture message on a successful assertion', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['assert'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.assert(1 + 1 === 2); }); it('should capture exception when console logs an error object with level set to "error"', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['error'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); const someError = new Error('some error'); GLOBAL_OBJ.console.error(someError); - expect(mockHub.captureException).toHaveBeenCalledTimes(1); - expect(mockHub.captureException).toHaveBeenCalledWith(someError); + expect(captureException).toHaveBeenCalledTimes(1); + expect(captureException).toHaveBeenCalledWith(someError, { + extra: { arguments: [someError] }, + level: 'error', + }); }); it('should capture exception on `console.error` when no levels are provided in constructor', () => { const captureConsoleIntegration = new CaptureConsole(); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); const someError = new Error('some error'); GLOBAL_OBJ.console.error(someError); - expect(mockHub.captureException).toHaveBeenCalledTimes(1); - expect(mockHub.captureException).toHaveBeenCalledWith(someError); + expect(captureException).toHaveBeenCalledTimes(1); + expect(captureException).toHaveBeenCalledWith(someError, { + extra: { arguments: [someError] }, + level: 'error', + }); }); it('should capture exception when console logs an error object in any of the args when level set to "error"', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['error'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); const someError = new Error('some error'); GLOBAL_OBJ.console.error('Something went wrong', someError); - expect(mockHub.captureException).toHaveBeenCalledTimes(1); - expect(mockHub.captureException).toHaveBeenCalledWith(someError); + expect(captureException).toHaveBeenCalledTimes(1); + expect(captureException).toHaveBeenCalledWith(someError, { + extra: { arguments: ['Something went wrong', someError] }, + level: 'error', + }); }); it('should capture message on `console.log` when no levels are provided in constructor', () => { const captureConsoleIntegration = new CaptureConsole(); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.error('some message'); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(1); - expect(mockHub.captureMessage).toHaveBeenCalledWith('some message'); + expect(captureMessage).toHaveBeenCalledTimes(1); + expect(captureMessage).toHaveBeenCalledWith('some message', { + extra: { arguments: ['some message'] }, + level: 'error', + }); }); it('should capture message when console logs a non-error object with level set to "error"', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['error'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.error('some non-error message'); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(1); - expect(mockHub.captureMessage).toHaveBeenCalledWith('some non-error message'); - expect(mockHub.captureException).not.toHaveBeenCalled(); + expect(captureMessage).toHaveBeenCalledTimes(1); + expect(captureMessage).toHaveBeenCalledWith('some non-error message', { + extra: { arguments: ['some non-error message'] }, + level: 'error', + }); + expect(captureException).not.toHaveBeenCalled(); }); it('should capture a message for non-error log levels', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['info'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.info('some message'); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(1); - expect(mockHub.captureMessage).toHaveBeenCalledWith('some message'); + expect(captureMessage).toHaveBeenCalledTimes(1); + expect(captureMessage).toHaveBeenCalledWith('some message', { + extra: { arguments: ['some message'] }, + level: 'info', + }); }); it('should call the original console function when console members are called', () => { @@ -342,11 +265,7 @@ describe('CaptureConsole setup', () => { GLOBAL_OBJ.console.log = mockConsoleLog; const captureConsoleIntegration = new CaptureConsole({ levels: ['log'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); GLOBAL_OBJ.console.log('some message 1', 'some message 2'); @@ -359,11 +278,7 @@ describe('CaptureConsole setup', () => { it('should not wrap any levels that are not members of console', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['log', 'someNonExistingLevel', 'error'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); // The provided level should not be created expect((GLOBAL_OBJ.console as any)['someNonExistingLevel']).toBeUndefined(); @@ -371,26 +286,19 @@ describe('CaptureConsole setup', () => { it('should wrap the console when the client does not have a registered captureconsole integration, but not capture any messages', () => { const captureConsoleIntegration = new CaptureConsole({ levels: ['log', 'error'] }); - const mockHub = getMockHub(null as any); // simulate not having the integration registered - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + // when `setup` is not called on the current client, it will not trigger + captureConsoleIntegration.setup({} as Client); // Should not capture messages GLOBAL_OBJ.console.log('some message'); - expect(mockHub.captureMessage).not.toHaveBeenCalledWith(); + expect(captureMessage).not.toHaveBeenCalledWith(); }); it("should not crash when the original console methods don't exist at time of invocation", () => { originalConsoleMethods.log = undefined; const captureConsoleIntegration = new CaptureConsole({ levels: ['log'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); + captureConsoleIntegration.setup(mockClient); expect(() => { GLOBAL_OBJ.console.log('some message'); @@ -401,13 +309,7 @@ describe('CaptureConsole setup', () => { // const addExceptionMechanismSpy = jest.spyOn(utils, 'addExceptionMechanism'); const captureConsoleIntegration = new CaptureConsole({ levels: ['error'] }); - const mockHub = getMockHub(captureConsoleIntegration); - captureConsoleIntegration.setupOnce( - () => undefined, - () => mockHub, - ); - - const mockScope = mockHub.getScope(); + captureConsoleIntegration.setup(mockClient); const someError = new Error('some error'); GLOBAL_OBJ.console.error(someError); @@ -420,7 +322,7 @@ describe('CaptureConsole setup', () => { }; addedEventProcessor(someEvent); - expect(mockHub.captureException).toHaveBeenCalledTimes(1); + expect(captureException).toHaveBeenCalledTimes(1); expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1); expect(someEvent.exception?.values?.[0].mechanism).toEqual({ diff --git a/packages/integrations/test/debug.test.ts b/packages/integrations/test/debug.test.ts index 953fcdb0258e..eefd9c8b9240 100644 --- a/packages/integrations/test/debug.test.ts +++ b/packages/integrations/test/debug.test.ts @@ -2,7 +2,7 @@ import type { Client, Event, EventHint, Hub, Integration } from '@sentry/types'; import { Debug } from '../src/debug'; -function testEventLogged(integration: Integration, testEvent?: Event, testEventHint?: EventHint) { +function testEventLogged(integration: Debug, testEvent?: Event, testEventHint?: EventHint) { const callbacks: ((event: Event, hint?: EventHint) => void)[] = []; const client: Client = { @@ -12,15 +12,7 @@ function testEventLogged(integration: Integration, testEvent?: Event, testEventH }, } as Client; - function getCurrentHub() { - return { - getClient: jest.fn(() => { - return client; - }), - } as unknown as Hub; - } - - integration.setupOnce(() => {}, getCurrentHub); + integration.setup(client); expect(callbacks.length).toEqual(1); diff --git a/packages/integrations/test/reportingobserver.test.ts b/packages/integrations/test/reportingobserver.test.ts index 1a8759fda23e..6378a456c854 100644 --- a/packages/integrations/test/reportingobserver.test.ts +++ b/packages/integrations/test/reportingobserver.test.ts @@ -1,4 +1,5 @@ -import type { Hub, Integration } from '@sentry/types'; +import * as SentryCore from '@sentry/core'; +import type { Client, Hub } from '@sentry/types'; import { ReportingObserver } from '../src/reportingobserver'; @@ -6,18 +7,13 @@ const mockScope = { setExtra: jest.fn(), }; -const mockHub = { - withScope: jest.fn(callback => { - callback(mockScope); - }), - captureMessage: jest.fn(), -}; +const withScope = jest.fn(callback => { + return callback(mockScope); +}); + +const captureMessage = jest.fn(); -const getMockHubWithIntegration = (integration: Integration) => - ({ - ...mockHub, - getIntegration: jest.fn(() => integration), - }) as unknown as Hub; +const mockHub = {} as unknown as Hub; const mockReportingObserverConstructor = jest.fn(); const mockObserve = jest.fn(); @@ -31,8 +27,16 @@ class MockReportingObserver { } describe('ReportingObserver', () => { + let mockClient: Client; + beforeEach(() => { (global as any).ReportingObserver = MockReportingObserver; + + mockClient = {} as Client; + + jest.spyOn(SentryCore, 'captureMessage').mockImplementation(captureMessage); + jest.spyOn(SentryCore, 'getClient').mockImplementation(() => mockClient); + jest.spyOn(SentryCore, 'withScope').mockImplementation(withScope); }); afterEach(() => { @@ -50,7 +54,7 @@ describe('ReportingObserver', () => { expect(() => { reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(null as any), + () => mockHub, ); }).not.toThrow(); @@ -62,8 +66,9 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); expect(mockReportingObserverConstructor).toHaveBeenCalledTimes(1); expect(mockReportingObserverConstructor).toHaveBeenCalledWith( @@ -76,8 +81,9 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver({ types: ['crash'] }); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); expect(mockReportingObserverConstructor).toHaveBeenCalledTimes(1); expect(mockReportingObserverConstructor).toHaveBeenCalledWith( @@ -90,8 +96,9 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); expect(mockReportingObserverConstructor).toHaveBeenCalledTimes(1); expect(mockReportingObserverConstructor).toHaveBeenCalledWith( @@ -104,8 +111,9 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); expect(mockObserve).toHaveBeenCalledTimes(1); }); @@ -116,37 +124,40 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(null as any), + () => mockHub, ); + // without calling setup, the integration is not registered expect(() => { reportingObserverIntegration.handler([{ type: 'crash', url: 'some url' }]); }).not.toThrow(); - expect(mockHub.captureMessage).not.toHaveBeenCalled(); + expect(captureMessage).not.toHaveBeenCalled(); }); it('should capture messages', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); reportingObserverIntegration.handler([ { type: 'crash', url: 'some url' }, { type: 'deprecation', url: 'some url' }, ]); - expect(mockHub.captureMessage).toHaveBeenCalledTimes(2); + expect(captureMessage).toHaveBeenCalledTimes(2); }); it('should set extra including the url of a report', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); reportingObserverIntegration.handler([ { type: 'crash', url: 'some url 1' }, @@ -161,8 +172,9 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report1 = { type: 'crash', url: 'some url 1', body: { crashId: 'id1' } } as const; const report2 = { type: 'deprecation', url: 'some url 2', body: { id: 'id2', message: 'message' } } as const; @@ -177,8 +189,9 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); reportingObserverIntegration.handler([{ type: 'crash', url: 'some url' }]); @@ -189,8 +202,9 @@ describe('ReportingObserver', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report = { type: 'crash', @@ -199,17 +213,18 @@ describe('ReportingObserver', () => { } as const; reportingObserverIntegration.handler([report]); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.crashId)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.reason)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.crashId)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.reason)); }); it('should capture report message from body on deprecation report', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report = { type: 'deprecation', @@ -218,16 +233,17 @@ describe('ReportingObserver', () => { } as const; reportingObserverIntegration.handler([report]); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.message)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.message)); }); it('should capture report message from body on intervention report', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report = { type: 'intervention', @@ -236,16 +252,17 @@ describe('ReportingObserver', () => { } as const; reportingObserverIntegration.handler([report]); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.message)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.body.message)); }); it('should use fallback message when no body is available', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report = { type: 'intervention', @@ -253,30 +270,32 @@ describe('ReportingObserver', () => { } as const; reportingObserverIntegration.handler([report]); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); }); it('should use fallback message when no body details are available for crash report', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report = { type: 'crash', url: 'some url', body: { crashId: '', reason: '' } } as const; reportingObserverIntegration.handler([report]); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); }); it('should use fallback message when no body message is available for deprecation report', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report = { type: 'deprecation', @@ -285,16 +304,17 @@ describe('ReportingObserver', () => { } as const; reportingObserverIntegration.handler([report]); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); }); it('should use fallback message when no body message is available for intervention report', () => { const reportingObserverIntegration = new ReportingObserver(); reportingObserverIntegration.setupOnce( () => undefined, - () => getMockHubWithIntegration(reportingObserverIntegration), + () => mockHub, ); + reportingObserverIntegration.setup(mockClient); const report = { type: 'intervention', @@ -303,8 +323,8 @@ describe('ReportingObserver', () => { } as const; reportingObserverIntegration.handler([report]); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); - expect(mockHub.captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining(report.type)); + expect(captureMessage).toHaveBeenCalledWith(expect.stringContaining('No details available')); }); }); });