diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index 5dc7cc8fb7c7..b3a77e3319e8 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -84,34 +84,19 @@ export class Breadcrumbs implements Integration { */ public setupOnce(): void { if (this._options.console) { - addInstrumentationHandler({ - callback: _consoleBreadcrumb, - type: 'console', - }); + addInstrumentationHandler('console', _consoleBreadcrumb); } if (this._options.dom) { - addInstrumentationHandler({ - callback: _domBreadcrumb(this._options.dom), - type: 'dom', - }); + addInstrumentationHandler('dom', _domBreadcrumb(this._options.dom)); } if (this._options.xhr) { - addInstrumentationHandler({ - callback: _xhrBreadcrumb, - type: 'xhr', - }); + addInstrumentationHandler('xhr', _xhrBreadcrumb); } if (this._options.fetch) { - addInstrumentationHandler({ - callback: _fetchBreadcrumb, - type: 'fetch', - }); + addInstrumentationHandler('fetch', _fetchBreadcrumb); } if (this._options.history) { - addInstrumentationHandler({ - callback: _historyBreadcrumb, - type: 'history', - }); + addInstrumentationHandler('history', _historyBreadcrumb); } } } diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index ad69493676e9..34b37fc7632a 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -74,9 +74,10 @@ export class GlobalHandlers implements Integration { /** JSDoc */ function _installGlobalOnErrorHandler(): void { - addInstrumentationHandler({ + addInstrumentationHandler( + 'error', // eslint-disable-next-line @typescript-eslint/no-explicit-any - callback: (data: { msg: any; url: any; line: any; column: any; error: any }) => { + (data: { msg: any; url: any; line: any; column: any; error: any }) => { const [hub, attachStacktrace] = getHubAndAttachStacktrace(); if (!hub.getIntegration(GlobalHandlers)) { return; @@ -101,15 +102,15 @@ function _installGlobalOnErrorHandler(): void { addMechanismAndCapture(hub, error, event, 'onerror'); }, - type: 'error', - }); + ); } /** JSDoc */ function _installGlobalOnUnhandledRejectionHandler(): void { - addInstrumentationHandler({ + addInstrumentationHandler( + 'unhandledrejection', // eslint-disable-next-line @typescript-eslint/no-explicit-any - callback: (e: any) => { + (e: any) => { const [hub, attachStacktrace] = getHubAndAttachStacktrace(); if (!hub.getIntegration(GlobalHandlers)) { return; @@ -151,8 +152,7 @@ function _installGlobalOnUnhandledRejectionHandler(): void { addMechanismAndCapture(hub, error, event, 'onunhandledrejection'); return; }, - type: 'unhandledrejection', - }); + ); } /** diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 518052ac71ad..d729648dc8fb 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -226,15 +226,12 @@ function startSessionTracking(): void { hub.captureSession(); // We want to create a session for every navigation as well - addInstrumentationHandler({ - callback: ({ from, to }) => { - // Don't create an additional session for the initial route or if the location did not change - if (from === undefined || from === to) { - return; - } - hub.startSession({ ignoreDuration: true }); - hub.captureSession(); - }, - type: 'history', + addInstrumentationHandler('history', ({ from, to }) => { + // Don't create an additional session for the initial route or if the location did not change + if (from === undefined || from === to) { + return; + } + hub.startSession({ ignoreDuration: true }); + hub.captureSession(); }); } diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index db9bf8066314..c0a576ed8cc2 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -118,20 +118,14 @@ export function instrumentOutgoingRequests(_options?: Partial = {}; if (traceFetch) { - addInstrumentationHandler({ - callback: (handlerData: FetchData) => { - fetchCallback(handlerData, shouldCreateSpan, spans); - }, - type: 'fetch', + addInstrumentationHandler('fetch', (handlerData: FetchData) => { + fetchCallback(handlerData, shouldCreateSpan, spans); }); } if (traceXHR) { - addInstrumentationHandler({ - callback: (handlerData: XHRData) => { - xhrCallback(handlerData, shouldCreateSpan, spans); - }, - type: 'xhr', + addInstrumentationHandler('xhr', (handlerData: XHRData) => { + xhrCallback(handlerData, shouldCreateSpan, spans); }); } } diff --git a/packages/tracing/src/browser/router.ts b/packages/tracing/src/browser/router.ts index 172a11592279..f5b5fe5f8ea3 100644 --- a/packages/tracing/src/browser/router.ts +++ b/packages/tracing/src/browser/router.ts @@ -24,33 +24,30 @@ export function instrumentRoutingWithDefaults( } if (startTransactionOnLocationChange) { - addInstrumentationHandler({ - callback: ({ to, from }: { to: string; from?: string }) => { - /** - * This early return is there to account for some cases where a navigation transaction starts right after - * long-running pageload. We make sure that if `from` is undefined and a valid `startingURL` exists, we don't - * create an uneccessary navigation transaction. - * - * This was hard to duplicate, but this behavior stopped as soon as this fix was applied. This issue might also - * only be caused in certain development environments where the usage of a hot module reloader is causing - * errors. - */ - if (from === undefined && startingUrl && startingUrl.indexOf(to) !== -1) { - startingUrl = undefined; - return; - } + addInstrumentationHandler('history', ({ to, from }: { to: string; from?: string }) => { + /** + * This early return is there to account for some cases where a navigation transaction starts right after + * long-running pageload. We make sure that if `from` is undefined and a valid `startingURL` exists, we don't + * create an uneccessary navigation transaction. + * + * This was hard to duplicate, but this behavior stopped as soon as this fix was applied. This issue might also + * only be caused in certain development environments where the usage of a hot module reloader is causing + * errors. + */ + if (from === undefined && startingUrl && startingUrl.indexOf(to) !== -1) { + startingUrl = undefined; + return; + } - if (from !== to) { - startingUrl = undefined; - if (activeTransaction) { - logger.log(`[Tracing] Finishing current transaction with op: ${activeTransaction.op}`); - // If there's an open transaction on the scope, we need to finish it before creating an new one. - activeTransaction.finish(); - } - activeTransaction = customStartTransaction({ name: global.location.pathname, op: 'navigation' }); + if (from !== to) { + startingUrl = undefined; + if (activeTransaction) { + logger.log(`[Tracing] Finishing current transaction with op: ${activeTransaction.op}`); + // If there's an open transaction on the scope, we need to finish it before creating an new one. + activeTransaction.finish(); } - }, - type: 'history', + activeTransaction = customStartTransaction({ name: global.location.pathname, op: 'navigation' }); + } }); } } diff --git a/packages/tracing/src/errors.ts b/packages/tracing/src/errors.ts index eb15d592cb7c..ae7c40f5c721 100644 --- a/packages/tracing/src/errors.ts +++ b/packages/tracing/src/errors.ts @@ -7,14 +7,8 @@ import { getActiveTransaction } from './utils'; * Configures global error listeners */ export function registerErrorInstrumentation(): void { - addInstrumentationHandler({ - callback: errorCallback, - type: 'error', - }); - addInstrumentationHandler({ - callback: errorCallback, - type: 'unhandledrejection', - }); + addInstrumentationHandler('error', errorCallback); + addInstrumentationHandler('unhandledrejection', errorCallback); } /** diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index e8a123eeb93c..c76b72deb6d7 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -24,7 +24,7 @@ jest.mock('@sentry/utils', () => { const actual = jest.requireActual('@sentry/utils'); return { ...actual, - addInstrumentationHandler: ({ callback, type }: any): void => { + addInstrumentationHandler: (type, callback): void => { if (type === 'history') { // rather than actually add the navigation-change handler, grab a reference to it, so we can trigger it manually mockChangeHistory = callback; diff --git a/packages/tracing/test/browser/request.test.ts b/packages/tracing/test/browser/request.test.ts index 4c2a18788131..ee8369ab0edf 100644 --- a/packages/tracing/test/browser/request.test.ts +++ b/packages/tracing/test/browser/request.test.ts @@ -26,26 +26,20 @@ describe('instrumentOutgoingRequests', () => { it('instruments fetch and xhr requests', () => { instrumentOutgoingRequests(); - expect(addInstrumentationHandler).toHaveBeenCalledWith({ - callback: expect.any(Function), - type: 'fetch', - }); - expect(addInstrumentationHandler).toHaveBeenCalledWith({ - callback: expect.any(Function), - type: 'xhr', - }); + expect(addInstrumentationHandler).toHaveBeenCalledWith('fetch', expect.any(Function)); + expect(addInstrumentationHandler).toHaveBeenCalledWith('xhr', expect.any(Function)); }); it('does not instrument fetch requests if traceFetch is false', () => { instrumentOutgoingRequests({ traceFetch: false }); - expect(addInstrumentationHandler).not.toHaveBeenCalledWith({ callback: expect.any(Function), type: 'fetch' }); + expect(addInstrumentationHandler).not.toHaveBeenCalledWith('fetch', expect.any(Function)); }); it('does not instrument xhr requests if traceXHR is false', () => { instrumentOutgoingRequests({ traceXHR: false }); - expect(addInstrumentationHandler).not.toHaveBeenCalledWith({ callback: expect.any(Function), type: 'xhr' }); + expect(addInstrumentationHandler).not.toHaveBeenCalledWith('xhr', expect.any(Function)); }); }); diff --git a/packages/tracing/test/browser/router.test.ts b/packages/tracing/test/browser/router.test.ts index 6c565b26655b..e340a81222fe 100644 --- a/packages/tracing/test/browser/router.test.ts +++ b/packages/tracing/test/browser/router.test.ts @@ -8,7 +8,7 @@ jest.mock('@sentry/utils', () => { const actual = jest.requireActual('@sentry/utils'); return { ...actual, - addInstrumentationHandler: ({ callback, type }: any): void => { + addInstrumentationHandler: (type, callback): void => { addInstrumentationHandlerType = type; mockChangeHistory = callback; }, diff --git a/packages/tracing/test/errors.test.ts b/packages/tracing/test/errors.test.ts index 5a0477cb9d31..503890142fa5 100644 --- a/packages/tracing/test/errors.test.ts +++ b/packages/tracing/test/errors.test.ts @@ -12,7 +12,7 @@ jest.mock('@sentry/utils', () => { const actual = jest.requireActual('@sentry/utils'); return { ...actual, - addInstrumentationHandler: ({ callback, type }: any) => { + addInstrumentationHandler: (type, callback) => { if (type === 'error') { mockErrorCallback = callback; } @@ -20,7 +20,7 @@ jest.mock('@sentry/utils', () => { mockUnhandledRejectionCallback = callback; } if (typeof mockAddInstrumentationHandler === 'function') { - return mockAddInstrumentationHandler({ callback, type }); + return mockAddInstrumentationHandler(type, callback); } }, }; @@ -45,11 +45,8 @@ describe('registerErrorHandlers()', () => { it('registers error instrumentation', () => { registerErrorInstrumentation(); expect(mockAddInstrumentationHandler).toHaveBeenCalledTimes(2); - expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(1, { callback: expect.any(Function), type: 'error' }); - expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(2, { - callback: expect.any(Function), - type: 'unhandledrejection', - }); + expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(1, 'error', expect.any(Function)); + expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(2, 'unhandledrejection', expect.any(Function)); }); it('does not set status if transaction is not on scope', () => { diff --git a/packages/utils/src/instrument.ts b/packages/utils/src/instrument.ts index 18dce47726f9..5b96eea11a2b 100644 --- a/packages/utils/src/instrument.ts +++ b/packages/utils/src/instrument.ts @@ -12,11 +12,6 @@ import { supportsHistory, supportsNativeFetch } from './supports'; const global = getGlobalObject(); -/** Object describing handler that will be triggered for a given `type` of instrumentation */ -interface InstrumentHandler { - type: InstrumentHandlerType; - callback: InstrumentHandlerCallback; -} type InstrumentHandlerType = | 'console' | 'dom' @@ -82,13 +77,10 @@ function instrument(type: InstrumentHandlerType): void { * Use at your own risk, this might break without changelog notice, only used internally. * @hidden */ -export function addInstrumentationHandler(handler: InstrumentHandler): void { - if (!handler || typeof handler.type !== 'string' || typeof handler.callback !== 'function') { - return; - } - handlers[handler.type] = handlers[handler.type] || []; - (handlers[handler.type] as InstrumentHandlerCallback[]).push(handler.callback); - instrument(handler.type); +export function addInstrumentationHandler(type: InstrumentHandlerType, callback: InstrumentHandlerCallback): void { + handlers[type] = handlers[type] || []; + (handlers[type] as InstrumentHandlerCallback[]).push(callback); + instrument(type); } /** JSDoc */