From 81399fc326641bbbafc14b0534ca27a07a64e60a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 13 Nov 2023 09:50:45 +0100 Subject: [PATCH 1/9] feat(utils): Refactor `addInstrumentationHandler` to dedicated methods This is mostly an internal utility, but changes a bit. This improves the actual type safety a lot, because currently we've been duplicating the types everywhere, sometimes in slightly differing ways, leading to potential issues. --- .../browser/src/integrations/breadcrumbs.ts | 50 +- .../src/integrations/globalhandlers.ts | 154 ++-- packages/browser/src/sdk.ts | 11 +- packages/core/src/tracing/errors.ts | 10 +- packages/core/test/lib/tracing/errors.test.ts | 41 +- packages/integrations/src/captureconsole.ts | 4 +- packages/integrations/src/httpclient.ts | 44 +- .../integrations/test/captureconsole.test.ts | 7 +- .../appRouterRoutingInstrumentation.ts | 6 +- .../appRouterInstrumentation.test.ts | 12 +- packages/node/src/integrations/console.ts | 4 +- packages/replay/src/coreHandlers/handleDom.ts | 14 +- .../replay/src/coreHandlers/handleFetch.ts | 2 +- .../replay/src/coreHandlers/handleHistory.ts | 15 +- .../coreHandlers/handleNetworkBreadcrumbs.ts | 6 +- packages/replay/src/coreHandlers/handleXhr.ts | 2 +- .../replay/src/coreHandlers/util/domUtils.ts | 8 +- packages/replay/src/types/performance.ts | 2 +- .../replay/src/util/addGlobalListeners.ts | 6 +- .../beforeAddRecordingEvent.test.ts | 10 +- .../test/integration/errorSampleRate.test.ts | 7 + .../replay/test/integration/flush.test.ts | 14 +- .../test/integration/sendReplayEvent.test.ts | 14 +- .../replay/test/integration/session.test.ts | 6 +- packages/replay/test/integration/stop.test.ts | 16 +- packages/replay/test/mocks/resetSdkMock.ts | 6 +- packages/replay/test/types.ts | 4 +- .../test/unit/coreHandlers/handleDom.test.ts | 13 +- .../handleNetworkBreadcrumbs.test.ts | 2 + .../tracing-internal/src/browser/request.ts | 27 +- .../tracing-internal/src/browser/router.ts | 4 +- .../test/browser/browsertracing.test.ts | 13 +- .../test/browser/request.test.ts | 36 +- .../test/browser/router.test.ts | 34 +- packages/types/src/index.ts | 13 +- packages/types/src/instrument.ts | 40 +- packages/utils/src/error.ts | 2 +- packages/utils/src/instrument.ts | 690 ------------------ packages/utils/src/instrument/_handlers.ts | 54 ++ packages/utils/src/instrument/console.ts | 44 ++ packages/utils/src/instrument/dom.ts | 260 +++++++ packages/utils/src/instrument/fetch.ts | 125 ++++ packages/utils/src/instrument/globalError.ts | 48 ++ .../instrument/globalUnhandledRejection.ts | 40 + packages/utils/src/instrument/history.ts | 71 ++ packages/utils/src/instrument/index.ts | 67 ++ packages/utils/src/instrument/xhr.ts | 158 ++++ packages/utils/src/logger.ts | 13 +- packages/utils/src/worldwide.ts | 4 +- packages/utils/test/instrument.test.ts | 51 -- packages/utils/test/instrument/dom.test.ts | 18 + packages/utils/test/instrument/fetch.test.ts | 29 + packages/utils/test/instrument/xhr.test.ts | 18 + 53 files changed, 1312 insertions(+), 1037 deletions(-) delete mode 100644 packages/utils/src/instrument.ts create mode 100644 packages/utils/src/instrument/_handlers.ts create mode 100644 packages/utils/src/instrument/console.ts create mode 100644 packages/utils/src/instrument/dom.ts create mode 100644 packages/utils/src/instrument/fetch.ts create mode 100644 packages/utils/src/instrument/globalError.ts create mode 100644 packages/utils/src/instrument/globalUnhandledRejection.ts create mode 100644 packages/utils/src/instrument/history.ts create mode 100644 packages/utils/src/instrument/index.ts create mode 100644 packages/utils/src/instrument/xhr.ts delete mode 100644 packages/utils/test/instrument.test.ts create mode 100644 packages/utils/test/instrument/dom.test.ts create mode 100644 packages/utils/test/instrument/fetch.test.ts create mode 100644 packages/utils/test/instrument/xhr.test.ts diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index f71361b7d96e..85fd489c7a68 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -1,7 +1,14 @@ -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ /* eslint-disable max-lines */ import { getCurrentHub } from '@sentry/core'; -import type { Event as SentryEvent, HandlerDataFetch, HandlerDataXhr, Integration } from '@sentry/types'; +import type { + Event as SentryEvent, + HandlerDataConsole, + HandlerDataDom, + HandlerDataFetch, + HandlerDataHistory, + HandlerDataXhr, + Integration, +} from '@sentry/types'; import type { FetchBreadcrumbData, FetchBreadcrumbHint, @@ -9,7 +16,11 @@ import type { XhrBreadcrumbHint, } from '@sentry/types/build/types/breadcrumb'; import { - addInstrumentationHandler, + addClickKeypressInstrumentationHandler, + addConsoleInstrumentationHandler, + addFetchInstrumentationHandler, + addHistoryInstrumentationHandler, + addXhrInstrumentationHandler, getEventDescription, htmlTreeAsString, logger, @@ -21,8 +32,6 @@ import { import { WINDOW } from '../helpers'; -type HandlerData = Record; - /** JSDoc */ interface BreadcrumbsOptions { console: boolean; @@ -88,19 +97,19 @@ export class Breadcrumbs implements Integration { */ public setupOnce(): void { if (this.options.console) { - addInstrumentationHandler('console', _consoleBreadcrumb); + addConsoleInstrumentationHandler(_consoleBreadcrumb); } if (this.options.dom) { - addInstrumentationHandler('dom', _domBreadcrumb(this.options.dom)); + addClickKeypressInstrumentationHandler(_domBreadcrumb(this.options.dom)); } if (this.options.xhr) { - addInstrumentationHandler('xhr', _xhrBreadcrumb); + addXhrInstrumentationHandler(_xhrBreadcrumb); } if (this.options.fetch) { - addInstrumentationHandler('fetch', _fetchBreadcrumb); + addFetchInstrumentationHandler(_fetchBreadcrumb); } if (this.options.history) { - addInstrumentationHandler('history', _historyBreadcrumb); + addHistoryInstrumentationHandler(_historyBreadcrumb); } if (this.options.sentry) { const client = getCurrentHub().getClient(); @@ -130,8 +139,8 @@ function addSentryBreadcrumb(event: SentryEvent): void { * A HOC that creaes a function that creates breadcrumbs from DOM API calls. * This is a HOC so that we get access to dom options in the closure. */ -function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: HandlerData) => void { - function _innerDomBreadcrumb(handlerData: HandlerData): void { +function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: HandlerDataDom) => void { + function _innerDomBreadcrumb(handlerData: HandlerDataDom): void { let target; let keyAttrs = typeof dom === 'object' ? dom.serializeAttribute : undefined; @@ -182,7 +191,7 @@ function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: HandlerDa /** * Creates breadcrumbs from console API calls */ -function _consoleBreadcrumb(handlerData: HandlerData & { args: unknown[]; level: string }): void { +function _consoleBreadcrumb(handlerData: HandlerDataConsole): void { const breadcrumb = { category: 'console', data: { @@ -212,7 +221,7 @@ function _consoleBreadcrumb(handlerData: HandlerData & { args: unknown[]; level: /** * Creates breadcrumbs from XHR API calls */ -function _xhrBreadcrumb(handlerData: HandlerData & HandlerDataXhr): void { +function _xhrBreadcrumb(handlerData: HandlerDataXhr): void { const { startTimestamp, endTimestamp } = handlerData; const sentryXhrData = handlerData.xhr[SENTRY_XHR_DATA_KEY]; @@ -250,7 +259,7 @@ function _xhrBreadcrumb(handlerData: HandlerData & HandlerDataXhr): void { /** * Creates breadcrumbs from fetch API calls */ -function _fetchBreadcrumb(handlerData: HandlerData & HandlerDataFetch & { response?: Response }): void { +function _fetchBreadcrumb(handlerData: HandlerDataFetch): void { const { startTimestamp, endTimestamp } = handlerData; // We only capture complete fetch requests @@ -282,13 +291,14 @@ function _fetchBreadcrumb(handlerData: HandlerData & HandlerDataFetch & { respon hint, ); } else { + const response = handlerData.response as Response | undefined; const data: FetchBreadcrumbData = { ...handlerData.fetchData, - status_code: handlerData.response && handlerData.response.status, + status_code: response && response.status, }; const hint: FetchBreadcrumbHint = { input: handlerData.args, - response: handlerData.response, + response, startTimestamp, endTimestamp, }; @@ -306,15 +316,15 @@ function _fetchBreadcrumb(handlerData: HandlerData & HandlerDataFetch & { respon /** * Creates breadcrumbs from history API calls */ -function _historyBreadcrumb(handlerData: HandlerData & { from: string; to: string }): void { +function _historyBreadcrumb(handlerData: HandlerDataHistory): void { let from: string | undefined = handlerData.from; let to: string | undefined = handlerData.to; const parsedLoc = parseUrl(WINDOW.location.href); - let parsedFrom = parseUrl(from); + let parsedFrom = from ? parseUrl(from) : undefined; const parsedTo = parseUrl(to); // Initial pushState doesn't provide `from` information - if (!parsedFrom.path) { + if (!parsedFrom || !parsedFrom.path) { parsedFrom = parsedLoc; } diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index af2f917daf96..0da5d934cb6d 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -1,7 +1,24 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ import { getCurrentHub } from '@sentry/core'; -import type { Event, Hub, Integration, Primitive, StackParser } from '@sentry/types'; -import { addInstrumentationHandler, getLocationHref, isErrorEvent, isPrimitive, isString, logger } from '@sentry/utils'; +import type { + Event, + EventHint, + HandlerDataUnhandledRejection, + Hub, + Integration, + Primitive, + StackParser, +} from '@sentry/types'; +import { + addExceptionMechanism, + addGlobalErrorInstrumentationHandler, + addGlobalUnhandledRejectionInstrumentationHandler, + getLocationHref, + isErrorEvent, + isPrimitive, + isString, + logger, +} from '@sentry/utils'; import type { BrowserClient } from '../client'; import { eventFromUnknownInput } from '../eventbuilder'; @@ -70,94 +87,73 @@ export class GlobalHandlers implements Integration { /** JSDoc */ function _installGlobalOnErrorHandler(): void { - addInstrumentationHandler( - 'error', - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (data: { msg: any; url: any; line: any; column: any; error: any }) => { - const [hub, stackParser, attachStacktrace] = getHubAndOptions(); - if (!hub.getIntegration(GlobalHandlers)) { - return; - } - const { msg, url, line, column, error } = data; - if (shouldIgnoreOnError() || (error && error.__sentry_own_request__)) { - return; - } + addGlobalErrorInstrumentationHandler(data => { + const [hub, stackParser, attachStacktrace] = getHubAndOptions(); + if (!hub.getIntegration(GlobalHandlers)) { + return; + } + const { msg, url, line, column, error } = data; + if (shouldIgnoreOnError()) { + return; + } - const event = - error === undefined && isString(msg) - ? _eventFromIncompleteOnError(msg, url, line, column) - : _enhanceEventWithInitialFrame( - eventFromUnknownInput(stackParser, error || msg, undefined, attachStacktrace, false), - url, - line, - column, - ); - - event.level = 'error'; - - hub.captureEvent(event, { - originalException: error, - mechanism: { - handled: false, - type: 'onerror', - }, - }); - }, - ); + const event = + error === undefined && isString(msg) + ? _eventFromIncompleteOnError(msg, url, line, column) + : _enhanceEventWithInitialFrame( + eventFromUnknownInput(stackParser, error || msg, undefined, attachStacktrace, false), + url, + line, + column, + ); + + event.level = 'error'; + + addMechanismAndCapture(hub, error, event, 'onerror'); + }); } /** JSDoc */ function _installGlobalOnUnhandledRejectionHandler(): void { - addInstrumentationHandler( - 'unhandledrejection', - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (e: any) => { - const [hub, stackParser, attachStacktrace] = getHubAndOptions(); - if (!hub.getIntegration(GlobalHandlers)) { - return; + addGlobalUnhandledRejectionInstrumentationHandler(e => { + const [hub, stackParser, attachStacktrace] = getHubAndOptions(); + if (!hub.getIntegration(GlobalHandlers)) { + return; + } + let error: Error | HandlerDataUnhandledRejection = e; + + // dig the object of the rejection out of known event types + try { + // PromiseRejectionEvents store the object of the rejection under 'reason' + // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent + if ('reason' in e && e.reason) { + error = e.reason; } - let error = e; - - // dig the object of the rejection out of known event types - try { - // PromiseRejectionEvents store the object of the rejection under 'reason' - // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent - if ('reason' in e) { - error = e.reason; - } - // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents - // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into - // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec - // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and - // https://github.com/getsentry/sentry-javascript/issues/2380 - else if ('detail' in e && 'reason' in e.detail) { - error = e.detail.reason; - } - } catch (_oO) { - // no-empty - } - - if (shouldIgnoreOnError() || (error && error.__sentry_own_request__)) { - return true; + // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents + // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into + // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec + // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and + // https://github.com/getsentry/sentry-javascript/issues/2380 + else if ('detail' in e && e.detail && 'reason' in e.detail && e.detail.reason) { + error = e.detail.reason; } + } catch (_oO) { + // no-empty + } - const event = isPrimitive(error) - ? _eventFromRejectionWithPrimitive(error) - : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, true); + if (shouldIgnoreOnError()) { + return true; + } - event.level = 'error'; + const event = isPrimitive(error) + ? _eventFromRejectionWithPrimitive(error) + : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, true); - hub.captureEvent(event, { - originalException: error, - mechanism: { - handled: false, - type: 'onunhandledrejection', - }, - }); + event.level = 'error'; - return; - }, - ); + addMechanismAndCapture(hub, error, event, 'onunhandledrejection'); + return; + }); } /** diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index de352d9db154..e52db79033e5 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -7,7 +7,12 @@ import { Integrations as CoreIntegrations, } from '@sentry/core'; import type { UserFeedback } from '@sentry/types'; -import { addInstrumentationHandler, logger, stackParserFromStackParserOptions, supportsFetch } from '@sentry/utils'; +import { + addHistoryInstrumentationHandler, + logger, + stackParserFromStackParserOptions, + supportsFetch, +} from '@sentry/utils'; import type { BrowserClientOptions, BrowserOptions } from './client'; import { BrowserClient } from './client'; @@ -240,9 +245,9 @@ function startSessionTracking(): void { startSessionOnHub(hub); // We want to create a session for every navigation as well - addInstrumentationHandler('history', ({ from, to }) => { + addHistoryInstrumentationHandler(({ from, to }) => { // Don't create an additional session for the initial route or if the location did not change - if (!(from === undefined || from === to)) { + if (from !== undefined && from !== to) { startSessionOnHub(getCurrentHub()); } }); diff --git a/packages/core/src/tracing/errors.ts b/packages/core/src/tracing/errors.ts index 8785bd86d448..477629cb7f9e 100644 --- a/packages/core/src/tracing/errors.ts +++ b/packages/core/src/tracing/errors.ts @@ -1,4 +1,8 @@ -import { addInstrumentationHandler, logger } from '@sentry/utils'; +import { + addGlobalErrorInstrumentationHandler, + addGlobalUnhandledRejectionInstrumentationHandler, + logger, +} from '@sentry/utils'; import type { SpanStatusType } from './span'; import { getActiveTransaction } from './utils'; @@ -14,8 +18,8 @@ export function registerErrorInstrumentation(): void { } errorsInstrumented = true; - addInstrumentationHandler('error', errorCallback); - addInstrumentationHandler('unhandledrejection', errorCallback); + addGlobalErrorInstrumentationHandler(errorCallback); + addGlobalUnhandledRejectionInstrumentationHandler(errorCallback); } /** diff --git a/packages/core/test/lib/tracing/errors.test.ts b/packages/core/test/lib/tracing/errors.test.ts index 8bf12f675032..4f72e7f74658 100644 --- a/packages/core/test/lib/tracing/errors.test.ts +++ b/packages/core/test/lib/tracing/errors.test.ts @@ -1,27 +1,26 @@ import { BrowserClient } from '@sentry/browser'; import { addTracingExtensions, Hub, makeMain } from '@sentry/core'; -import type { InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils'; +import type { HandlerDataError, HandlerDataUnhandledRejection } from '@sentry/types'; import { getDefaultBrowserClientOptions } from '../../../../tracing/test/testutils'; import { registerErrorInstrumentation } from '../../../src/tracing/errors'; -const mockAddInstrumentationHandler = jest.fn(); -let mockErrorCallback: InstrumentHandlerCallback = () => undefined; -let mockUnhandledRejectionCallback: InstrumentHandlerCallback = () => undefined; +const mockAddGlobalErrorInstrumentationHandler = jest.fn(); +const mockAddGlobalUnhandledRejectionInstrumentationHandler = jest.fn(); +let mockErrorCallback: (data: HandlerDataError) => void = () => {}; +let mockUnhandledRejectionCallback: (data: HandlerDataUnhandledRejection) => void = () => {}; jest.mock('@sentry/utils', () => { const actual = jest.requireActual('@sentry/utils'); return { ...actual, - addInstrumentationHandler: (type: InstrumentHandlerType, callback: InstrumentHandlerCallback) => { - if (type === 'error') { - mockErrorCallback = callback; - } - if (type === 'unhandledrejection') { - mockUnhandledRejectionCallback = callback; - } - if (typeof mockAddInstrumentationHandler === 'function') { - return mockAddInstrumentationHandler(type, callback); - } + addGlobalErrorInstrumentationHandler: (callback: () => void) => { + mockErrorCallback = callback; + + return mockAddGlobalErrorInstrumentationHandler(callback); + }, + addGlobalUnhandledRejectionInstrumentationHandler: (callback: () => void) => { + mockUnhandledRejectionCallback = callback; + return mockAddGlobalUnhandledRejectionInstrumentationHandler(callback); }, }; }); @@ -33,7 +32,8 @@ beforeAll(() => { describe('registerErrorHandlers()', () => { let hub: Hub; beforeEach(() => { - mockAddInstrumentationHandler.mockClear(); + mockAddGlobalErrorInstrumentationHandler.mockClear(); + mockAddGlobalUnhandledRejectionInstrumentationHandler.mockClear(); const options = getDefaultBrowserClientOptions(); hub = new Hub(new BrowserClient(options)); makeMain(hub); @@ -45,9 +45,10 @@ describe('registerErrorHandlers()', () => { it('registers error instrumentation', () => { registerErrorInstrumentation(); - expect(mockAddInstrumentationHandler).toHaveBeenCalledTimes(2); - expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(1, 'error', expect.any(Function)); - expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(2, 'unhandledrejection', expect.any(Function)); + expect(mockAddGlobalErrorInstrumentationHandler).toHaveBeenCalledTimes(1); + expect(mockAddGlobalUnhandledRejectionInstrumentationHandler).toHaveBeenCalledTimes(1); + expect(mockAddGlobalErrorInstrumentationHandler).toHaveBeenCalledWith(expect.any(Function)); + expect(mockAddGlobalUnhandledRejectionInstrumentationHandler).toHaveBeenCalledWith(expect.any(Function)); }); it('does not set status if transaction is not on scope', () => { @@ -55,7 +56,7 @@ describe('registerErrorHandlers()', () => { const transaction = hub.startTransaction({ name: 'test' }); expect(transaction.status).toBe(undefined); - mockErrorCallback({}); + mockErrorCallback({} as HandlerDataError); expect(transaction.status).toBe(undefined); mockUnhandledRejectionCallback({}); @@ -68,7 +69,7 @@ describe('registerErrorHandlers()', () => { const transaction = hub.startTransaction({ name: 'test' }); hub.configureScope(scope => scope.setSpan(transaction)); - mockErrorCallback({}); + mockErrorCallback({} as HandlerDataError); expect(transaction.status).toBe('internal_error'); transaction.finish(); diff --git a/packages/integrations/src/captureconsole.ts b/packages/integrations/src/captureconsole.ts index 124985662dd5..82dcfcde4886 100644 --- a/packages/integrations/src/captureconsole.ts +++ b/packages/integrations/src/captureconsole.ts @@ -1,7 +1,7 @@ import type { EventProcessor, Hub, Integration } from '@sentry/types'; import { + addConsoleInstrumentationHandler, addExceptionMechanism, - addInstrumentationHandler, CONSOLE_LEVELS, GLOBAL_OBJ, safeJoin, @@ -43,7 +43,7 @@ export class CaptureConsole implements Integration { const levels = this._levels; - addInstrumentationHandler('console', ({ args, level }: { args: unknown[]; level: string }) => { + addConsoleInstrumentationHandler(({ args, level }) => { if (!levels.includes(level)) { return; } diff --git a/packages/integrations/src/httpclient.ts b/packages/integrations/src/httpclient.ts index 6ceb832fc1ff..623b216e22fa 100644 --- a/packages/integrations/src/httpclient.ts +++ b/packages/integrations/src/httpclient.ts @@ -2,15 +2,14 @@ import { getCurrentHub, isSentryRequestUrl } from '@sentry/core'; import type { Event as SentryEvent, EventProcessor, - HandlerDataFetch, - HandlerDataXhr, Hub, Integration, SentryWrappedXMLHttpRequest, } from '@sentry/types'; import { addExceptionMechanism, - addInstrumentationHandler, + addFetchInstrumentationHandler, + addXhrInstrumentationHandler, GLOBAL_OBJ, logger, SENTRY_XHR_DATA_KEY, @@ -172,7 +171,7 @@ export class HttpClient implements Integration { const event = this._createEvent({ url: xhr.responseURL, - method: method, + method, status: xhr.status, requestHeaders, // Can't access request cookies from XHR @@ -300,7 +299,7 @@ export class HttpClient implements Integration { return; } - addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch & { response?: Response }) => { + addFetchInstrumentationHandler(handlerData => { const { response, args } = handlerData; const [requestInfo, requestInit] = args as [RequestInfo, RequestInit | undefined]; @@ -308,7 +307,7 @@ export class HttpClient implements Integration { return; } - this._fetchResponseHandler(requestInfo, response, requestInit); + this._fetchResponseHandler(requestInfo, response as Response, requestInit); }); } @@ -320,30 +319,23 @@ export class HttpClient implements Integration { return; } - addInstrumentationHandler( - 'xhr', - (handlerData: HandlerDataXhr & { xhr: SentryWrappedXMLHttpRequest & XMLHttpRequest }) => { - const { xhr } = handlerData; + addXhrInstrumentationHandler(handlerData => { + const xhr = handlerData.xhr as SentryWrappedXMLHttpRequest & XMLHttpRequest; - const sentryXhrData = xhr[SENTRY_XHR_DATA_KEY]; + const sentryXhrData = xhr[SENTRY_XHR_DATA_KEY]; - if (!sentryXhrData) { - return; - } - - const { method, request_headers: headers } = sentryXhrData; + if (!sentryXhrData) { + return; + } - if (!method) { - return; - } + const { method, request_headers: headers } = sentryXhrData; - try { - this._xhrResponseHandler(xhr, method, headers); - } catch (e) { - __DEBUG_BUILD__ && logger.warn('Error while extracting response event form XHR response', e); - } - }, - ); + try { + this._xhrResponseHandler(xhr, method, headers); + } catch (e) { + __DEBUG_BUILD__ && logger.warn('Error while extracting response event form XHR response', e); + } + }); } /** diff --git a/packages/integrations/test/captureconsole.test.ts b/packages/integrations/test/captureconsole.test.ts index bddf7ef603dd..3a75547592aa 100644 --- a/packages/integrations/test/captureconsole.test.ts +++ b/packages/integrations/test/captureconsole.test.ts @@ -1,8 +1,7 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import type { Event, Hub, Integration } from '@sentry/types'; -import type { ConsoleLevel } from '@sentry/utils'; +import type { ConsoleLevel, Event, Hub, Integration } from '@sentry/types'; import { - addInstrumentationHandler, + addConsoleInstrumentationHandler, CONSOLE_LEVELS, GLOBAL_OBJ, originalConsoleMethods, @@ -45,7 +44,7 @@ function getMockHub(integration: Integration): Hub { describe('CaptureConsole setup', () => { // Ensure we've initialized the instrumentation so we can get the original one - addInstrumentationHandler('console', () => {}); + addConsoleInstrumentationHandler(() => {}); const _originalConsoleMethods = Object.assign({}, originalConsoleMethods); beforeEach(() => { diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index 5d7ab3596c8e..8d45f4b9ddb3 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -1,6 +1,6 @@ import { WINDOW } from '@sentry/react'; -import type { HandlerDataFetch, Primitive, Transaction, TransactionContext } from '@sentry/types'; -import { addInstrumentationHandler, browserPerformanceTimeOrigin } from '@sentry/utils'; +import type { Primitive, Transaction, TransactionContext } from '@sentry/types'; +import { addFetchInstrumentationHandler, browserPerformanceTimeOrigin } from '@sentry/utils'; type StartTransactionCb = (context: TransactionContext) => Transaction | undefined; @@ -36,7 +36,7 @@ export function appRouterInstrumentation( } if (startTransactionOnLocationChange) { - addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { + addFetchInstrumentationHandler(handlerData => { // The instrumentation handler is invoked twice - once for starting a request and once when the req finishes // We can use the existence of the end-timestamp to filter out "finishing"-events. if (handlerData.endTimestamp !== undefined) { diff --git a/packages/nextjs/test/performance/appRouterInstrumentation.test.ts b/packages/nextjs/test/performance/appRouterInstrumentation.test.ts index bebb0cbf8ab6..3337b99ab9a9 100644 --- a/packages/nextjs/test/performance/appRouterInstrumentation.test.ts +++ b/packages/nextjs/test/performance/appRouterInstrumentation.test.ts @@ -5,7 +5,7 @@ import { JSDOM } from 'jsdom'; import { appRouterInstrumentation } from '../../src/client/routing/appRouterRoutingInstrumentation'; -const addInstrumentationHandlerSpy = jest.spyOn(sentryUtils, 'addInstrumentationHandler'); +const addFetchInstrumentationHandlerSpy = jest.spyOn(sentryUtils, 'addFetchInstrumentationHandler'); function setUpPage(url: string) { const dom = new JSDOM('

nothingness

', { url }); @@ -55,7 +55,7 @@ describe('appRouterInstrumentation', () => { setUpPage('https://example.com/some/page?someParam=foobar'); let fetchInstrumentationHandlerCallback: (arg: HandlerDataFetch) => void; - addInstrumentationHandlerSpy.mockImplementationOnce((_type, callback) => { + addFetchInstrumentationHandlerSpy.mockImplementationOnce(callback => { fetchInstrumentationHandlerCallback = callback; }); @@ -138,7 +138,7 @@ describe('appRouterInstrumentation', () => { setUpPage('https://example.com/some/page?someParam=foobar'); let fetchInstrumentationHandlerCallback: (arg: HandlerDataFetch) => void; - addInstrumentationHandlerSpy.mockImplementationOnce((_type, callback) => { + addFetchInstrumentationHandlerSpy.mockImplementationOnce(callback => { fetchInstrumentationHandlerCallback = callback; }); @@ -151,11 +151,11 @@ describe('appRouterInstrumentation', () => { it('should not create navigation transactions when `startTransactionOnLocationChange` is false', () => { setUpPage('https://example.com/some/page?someParam=foobar'); - const addInstrumentationHandlerImpl = jest.fn(); + const addFetchInstrumentationHandlerImpl = jest.fn(); const startTransactionCallbackFn = jest.fn(); - addInstrumentationHandlerSpy.mockImplementationOnce(addInstrumentationHandlerImpl); + addFetchInstrumentationHandlerSpy.mockImplementationOnce(addFetchInstrumentationHandlerImpl); appRouterInstrumentation(startTransactionCallbackFn, false, false); - expect(addInstrumentationHandlerImpl).not.toHaveBeenCalled(); + expect(addFetchInstrumentationHandlerImpl).not.toHaveBeenCalled(); }); }); diff --git a/packages/node/src/integrations/console.ts b/packages/node/src/integrations/console.ts index eb8a38980a64..9f39b41fe0b1 100644 --- a/packages/node/src/integrations/console.ts +++ b/packages/node/src/integrations/console.ts @@ -1,6 +1,6 @@ import { getCurrentHub } from '@sentry/core'; import type { Integration } from '@sentry/types'; -import { addInstrumentationHandler, severityLevelFromString } from '@sentry/utils'; +import { addConsoleInstrumentationHandler, severityLevelFromString } from '@sentry/utils'; import * as util from 'util'; /** Console module integration */ @@ -19,7 +19,7 @@ export class Console implements Integration { * @inheritDoc */ public setupOnce(): void { - addInstrumentationHandler('console', ({ args, level }: { args: unknown[]; level: string }) => { + addConsoleInstrumentationHandler(({ args, level }) => { const hub = getCurrentHub(); if (!hub.getIntegration(Console)) { diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 60220f0a5a66..53a92575b027 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -1,21 +1,20 @@ import { record } from '@sentry-internal/rrweb'; import type { serializedElementNodeWithId, serializedNodeWithId } from '@sentry-internal/rrweb-snapshot'; import { NodeType } from '@sentry-internal/rrweb-snapshot'; -import type { Breadcrumb } from '@sentry/types'; +import type { Breadcrumb, HandlerDataDom } from '@sentry/types'; import { htmlTreeAsString } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; import { handleClick } from './handleClick'; import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; -import type { DomHandlerData } from './util/domUtils'; import { getClickTargetNode, getTargetNode } from './util/domUtils'; import { getAttributesToRecord } from './util/getAttributesToRecord'; -export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHandlerData) => void = ( +export const handleDomListener: (replay: ReplayContainer) => (handlerData: HandlerDataDom) => void = ( replay: ReplayContainer, ) => { - return (handlerData: DomHandlerData): void => { + return (handlerData: HandlerDataDom): void => { if (!replay.isEnabled()) { return; } @@ -27,12 +26,13 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa } const isClick = handlerData.name === 'click'; - const event = isClick && (handlerData.event as PointerEvent); + const event = isClick ? (handlerData.event as PointerEvent) : undefined; // Ignore clicks if ctrl/alt/meta/shift keys are held down as they alter behavior of clicks (e.g. open in new tab) if ( isClick && replay.clickDetector && event && + event.target && !event.altKey && !event.metaKey && !event.ctrlKey && @@ -80,7 +80,7 @@ export function getBaseDomBreadcrumb(target: Node | null, message: string): Brea * An event handler to react to DOM events. * Exported for tests. */ -export function handleDom(handlerData: DomHandlerData): Breadcrumb | null { +export function handleDom(handlerData: HandlerDataDom): Breadcrumb | null { const { target, message } = getDomTarget(handlerData); return createBreadcrumb({ @@ -89,7 +89,7 @@ export function handleDom(handlerData: DomHandlerData): Breadcrumb | null { }); } -function getDomTarget(handlerData: DomHandlerData): { target: Node | null; message: string } { +function getDomTarget(handlerData: HandlerDataDom): { target: Node | null; message: string } { const isClick = handlerData.name === 'click'; let message: string | undefined; diff --git a/packages/replay/src/coreHandlers/handleFetch.ts b/packages/replay/src/coreHandlers/handleFetch.ts index 36ec538f1b28..54681c2871c0 100644 --- a/packages/replay/src/coreHandlers/handleFetch.ts +++ b/packages/replay/src/coreHandlers/handleFetch.ts @@ -27,7 +27,7 @@ export function handleFetch(handlerData: HandlerDataFetch): null | ReplayPerform } /** - * Returns a listener to be added to `addInstrumentationHandler('fetch', listener)`. + * Returns a listener to be added to `addFetchInstrumentationHandler(listener)`. */ export function handleFetchSpanListener(replay: ReplayContainer): (handlerData: HandlerDataFetch) => void { return (handlerData: HandlerDataFetch) => { diff --git a/packages/replay/src/coreHandlers/handleHistory.ts b/packages/replay/src/coreHandlers/handleHistory.ts index a72618a2203f..79167114c135 100644 --- a/packages/replay/src/coreHandlers/handleHistory.ts +++ b/packages/replay/src/coreHandlers/handleHistory.ts @@ -1,12 +1,9 @@ +import type { HandlerDataHistory } from '@sentry/types'; + import type { HistoryData, ReplayContainer, ReplayPerformanceEntry } from '../types'; import { createPerformanceSpans } from '../util/createPerformanceSpans'; -interface HistoryHandlerData { - from: string; - to: string; -} - -function handleHistory(handlerData: HistoryHandlerData): ReplayPerformanceEntry { +function handleHistory(handlerData: HandlerDataHistory): ReplayPerformanceEntry { const { from, to } = handlerData; const now = Date.now() / 1000; @@ -23,10 +20,10 @@ function handleHistory(handlerData: HistoryHandlerData): ReplayPerformanceEntry< } /** - * Returns a listener to be added to `addInstrumentationHandler('history', listener)`. + * Returns a listener to be added to `addHistoryInstrumentationHandler(listener)`. */ -export function handleHistorySpanListener(replay: ReplayContainer): (handlerData: HistoryHandlerData) => void { - return (handlerData: HistoryHandlerData) => { +export function handleHistorySpanListener(replay: ReplayContainer): (handlerData: HandlerDataHistory) => void { + return (handlerData: HandlerDataHistory) => { if (!replay.isEnabled()) { return; } diff --git a/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts b/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts index 20fa00a633eb..8ce5a818cd07 100644 --- a/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts +++ b/packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts @@ -6,7 +6,7 @@ import type { TextEncoderInternal, XhrBreadcrumbData, } from '@sentry/types'; -import { addInstrumentationHandler, logger } from '@sentry/utils'; +import { addFetchInstrumentationHandler, addXhrInstrumentationHandler, logger } from '@sentry/utils'; import type { FetchHint, ReplayContainer, ReplayNetworkOptions, XhrHint } from '../types'; import { handleFetchSpanListener } from './handleFetch'; @@ -53,8 +53,8 @@ export function handleNetworkBreadcrumbs(replay: ReplayContainer): void { client.on('beforeAddBreadcrumb', (breadcrumb, hint) => beforeAddNetworkBreadcrumb(options, breadcrumb, hint)); } else { // Fallback behavior - addInstrumentationHandler('fetch', handleFetchSpanListener(replay)); - addInstrumentationHandler('xhr', handleXhrSpanListener(replay)); + addFetchInstrumentationHandler(handleFetchSpanListener(replay)); + addXhrInstrumentationHandler(handleXhrSpanListener(replay)); } } catch { // Do nothing diff --git a/packages/replay/src/coreHandlers/handleXhr.ts b/packages/replay/src/coreHandlers/handleXhr.ts index d3ec1736427a..afe76411bdcf 100644 --- a/packages/replay/src/coreHandlers/handleXhr.ts +++ b/packages/replay/src/coreHandlers/handleXhr.ts @@ -34,7 +34,7 @@ export function handleXhr(handlerData: HandlerDataXhr): ReplayPerformanceEntry void { return (handlerData: HandlerDataXhr) => { diff --git a/packages/replay/src/coreHandlers/util/domUtils.ts b/packages/replay/src/coreHandlers/util/domUtils.ts index 5f45bfbd367d..83f34dc19f31 100644 --- a/packages/replay/src/coreHandlers/util/domUtils.ts +++ b/packages/replay/src/coreHandlers/util/domUtils.ts @@ -1,9 +1,5 @@ import type { INode } from '@sentry-internal/rrweb-snapshot'; - -export interface DomHandlerData { - name: string; - event: Node | { target: EventTarget }; -} +import type { HandlerDataDom } from '@sentry/types'; const INTERACTIVE_SELECTOR = 'button,a'; @@ -19,7 +15,7 @@ export function getClosestInteractive(element: Element): Element { * This is useful because if you click on the image in , * The target will be the image, not the button, which we don't want here */ -export function getClickTargetNode(event: DomHandlerData['event'] | MouseEvent): Node | INode | null { +export function getClickTargetNode(event: HandlerDataDom['event'] | MouseEvent | Node): Node | INode | null { const target = getTargetNode(event); if (!target || !(target instanceof Element)) { diff --git a/packages/replay/src/types/performance.ts b/packages/replay/src/types/performance.ts index 49590a361cdb..2fe87d24a9c8 100644 --- a/packages/replay/src/types/performance.ts +++ b/packages/replay/src/types/performance.ts @@ -131,7 +131,7 @@ export interface NetworkRequestData { } export interface HistoryData { - previous: string; + previous: string | undefined; } export type AllEntryData = AllPerformanceEntryData | MemoryData | NetworkRequestData | HistoryData; diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index bf133c3e2130..2549236317df 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -1,7 +1,7 @@ import type { BaseClient } from '@sentry/core'; import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; import type { Client, DynamicSamplingContext } from '@sentry/types'; -import { addInstrumentationHandler } from '@sentry/utils'; +import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler } from '@sentry/utils'; import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent'; import { handleDomListener } from '../coreHandlers/handleDom'; @@ -20,8 +20,8 @@ export function addGlobalListeners(replay: ReplayContainer): void { const client = getCurrentHub().getClient(); scope.addScopeListener(handleScopeListener(replay)); - addInstrumentationHandler('dom', handleDomListener(replay)); - addInstrumentationHandler('history', handleHistorySpanListener(replay)); + addClickKeypressInstrumentationHandler(handleDomListener(replay)); + addHistoryInstrumentationHandler(handleHistorySpanListener(replay)); handleNetworkBreadcrumbs(replay); // Tag all (non replay) events that get sent to Sentry with the current diff --git a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts index 4059b71fe195..cfa0e80e17f6 100644 --- a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts +++ b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts @@ -9,6 +9,7 @@ import { createPerformanceEntries } from '../../src/util/createPerformanceEntrie import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; import * as SendReplayRequest from '../../src/util/sendReplayRequest'; import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; +import type { DomHandler } from '../types'; import { useFakeTimers } from '../utils/use-fake-timers'; useFakeTimers(); @@ -25,15 +26,13 @@ describe('Integration | beforeAddRecordingEvent', () => { let integration: Replay; let mockTransportSend: MockTransportSend; let mockSendReplayRequest: jest.SpyInstance; - let domHandler: (args: any) => any; + let domHandler: DomHandler; const { record: mockRecord } = mockRrweb(); beforeAll(async () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); - jest.spyOn(SentryUtils, 'addInstrumentationHandler').mockImplementation((type, handler: (args: any) => any) => { - if (type === 'dom') { - domHandler = handler; - } + jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + domHandler = handler; }); ({ replay, integration } = await mockSdk({ @@ -104,6 +103,7 @@ describe('Integration | beforeAddRecordingEvent', () => { it('changes click breadcrumbs message', async () => { domHandler({ name: 'click', + event: new Event('click'), }); await advanceTimers(5000); diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index fadcaa568f25..2cbc6ac35af7 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -70,6 +70,7 @@ describe('Integration | errorSampleRate', () => { // Does not capture on mouse click domHandler({ name: 'click', + event: new Event('click'), }); jest.runAllTimers(); await new Promise(process.nextTick); @@ -121,6 +122,7 @@ describe('Integration | errorSampleRate', () => { // Check that click will get captured domHandler({ name: 'click', + event: new Event('click'), }); await waitForFlush(); @@ -156,6 +158,7 @@ describe('Integration | errorSampleRate', () => { // Does not capture on mouse click domHandler({ name: 'click', + event: new Event('click'), }); jest.runAllTimers(); await new Promise(process.nextTick); @@ -195,6 +198,7 @@ describe('Integration | errorSampleRate', () => { // Check that click will not get captured domHandler({ name: 'click', + event: new Event('click'), }); await waitForFlush(); @@ -239,6 +243,7 @@ describe('Integration | errorSampleRate', () => { // Does not capture on mouse click domHandler({ name: 'click', + event: new Event('click'), }); jest.runAllTimers(); await new Promise(process.nextTick); @@ -279,6 +284,7 @@ describe('Integration | errorSampleRate', () => { // Check that click will not get captured domHandler({ name: 'click', + event: new Event('click'), }); await waitForFlush(); @@ -517,6 +523,7 @@ describe('Integration | errorSampleRate', () => { domHandler({ name: 'click', + event: new Event('click'), }); await waitForFlush(); diff --git a/packages/replay/test/integration/flush.test.ts b/packages/replay/test/integration/flush.test.ts index d7c8b0033561..b40d6c78906d 100644 --- a/packages/replay/test/integration/flush.test.ts +++ b/packages/replay/test/integration/flush.test.ts @@ -9,6 +9,7 @@ import { createPerformanceEntries } from '../../src/util/createPerformanceEntrie import { createPerformanceSpans } from '../../src/util/createPerformanceSpans'; import * as SendReplay from '../../src/util/sendReplay'; import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; +import type { DomHandler } from '../types'; import { getTestEventCheckout } from '../utils/getTestEvent'; import { useFakeTimers } from '../utils/use-fake-timers'; @@ -30,7 +31,7 @@ const prevLocation = WINDOW.location; const prevBrowserPerformanceTimeOrigin = SentryUtils.browserPerformanceTimeOrigin; describe('Integration | flush', () => { - let domHandler: (args: any) => any; + let domHandler: DomHandler; const { record: mockRecord } = mockRrweb(); @@ -43,10 +44,8 @@ describe('Integration | flush', () => { let mockAddPerformanceEntries: MockAddPerformanceEntries; beforeAll(async () => { - jest.spyOn(SentryUtils, 'addInstrumentationHandler').mockImplementation((type, handler: (args: any) => any) => { - if (type === 'dom') { - domHandler = handler; - } + jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + domHandler = handler; }); ({ replay } = await mockSdk()); @@ -162,6 +161,7 @@ describe('Integration | flush', () => { // This will attempt to flush in 5 seconds (flushMinDelay) domHandler({ name: 'click', + event: new Event('click'), }); await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); // flush #2 @ t=5s - due to click @@ -259,6 +259,7 @@ describe('Integration | flush', () => { // click happens first domHandler({ name: 'click', + event: new Event('click'), }); // checkout @@ -284,6 +285,7 @@ describe('Integration | flush', () => { // click happens first domHandler({ name: 'click', + event: new Event('click'), }); // checkout @@ -324,6 +326,7 @@ describe('Integration | flush', () => { // click happens first domHandler({ name: 'click', + event: new Event('click'), }); // checkout @@ -355,6 +358,7 @@ describe('Integration | flush', () => { // click happens first domHandler({ name: 'click', + event: new Event('click'), }); // no checkout! diff --git a/packages/replay/test/integration/sendReplayEvent.test.ts b/packages/replay/test/integration/sendReplayEvent.test.ts index 4aef1ee94776..8ca71236bb41 100644 --- a/packages/replay/test/integration/sendReplayEvent.test.ts +++ b/packages/replay/test/integration/sendReplayEvent.test.ts @@ -8,6 +8,7 @@ import { clearSession } from '../../src/session/clearSession'; import { addEvent } from '../../src/util/addEvent'; import * as SendReplayRequest from '../../src/util/sendReplayRequest'; import { BASE_TIMESTAMP, mockRrweb, mockSdk } from '../index'; +import type { DomHandler } from '../types'; import { getTestEventCheckout, getTestEventIncremental } from '../utils/getTestEvent'; import { useFakeTimers } from '../utils/use-fake-timers'; @@ -24,15 +25,13 @@ describe('Integration | sendReplayEvent', () => { let replay: ReplayContainer; let mockTransportSend: MockTransportSend; let mockSendReplayRequest: jest.SpyInstance; - let domHandler: (args: any) => any; + let domHandler: DomHandler; const { record: mockRecord } = mockRrweb(); beforeAll(async () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); - jest.spyOn(SentryUtils, 'addInstrumentationHandler').mockImplementation((type, handler: (args: any) => any) => { - if (type === 'dom') { - domHandler = handler; - } + jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + domHandler = handler; }); ({ replay } = await mockSdk({ @@ -115,6 +114,7 @@ describe('Integration | sendReplayEvent', () => { domHandler({ name: 'click', + event: new Event('click'), }); expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP); @@ -125,6 +125,7 @@ describe('Integration | sendReplayEvent', () => { domHandler({ name: 'click', + event: new Event('click'), }); expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED); @@ -135,6 +136,7 @@ describe('Integration | sendReplayEvent', () => { domHandler({ name: 'input', + event: new Event('keypress'), }); expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP); @@ -145,6 +147,7 @@ describe('Integration | sendReplayEvent', () => { domHandler({ name: 'input', + event: new Event('keypress'), }); expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP + ELAPSED); @@ -273,6 +276,7 @@ describe('Integration | sendReplayEvent', () => { it('uploads a dom breadcrumb 5 seconds after listener receives an event', async () => { domHandler({ name: 'click', + event: new Event('click'), }); // Pretend 5 seconds have passed diff --git a/packages/replay/test/integration/session.test.ts b/packages/replay/test/integration/session.test.ts index 3716c8b33bc7..86e1fa16cc9e 100644 --- a/packages/replay/test/integration/session.test.ts +++ b/packages/replay/test/integration/session.test.ts @@ -18,6 +18,7 @@ import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; import { BASE_TIMESTAMP } from '../index'; import type { RecordMock } from '../mocks/mockRrweb'; import { resetSdkMock } from '../mocks/resetSdkMock'; +import type { DomHandler } from '../types'; import { getTestEventCheckout, getTestEventIncremental } from '../utils/getTestEvent'; import { useFakeTimers } from '../utils/use-fake-timers'; @@ -32,7 +33,7 @@ const prevLocation = WINDOW.location; describe('Integration | session', () => { let replay: ReplayContainer; - let domHandler: (args: any) => any; + let domHandler: DomHandler; let mockRecord: RecordMock; beforeEach(async () => { @@ -185,6 +186,7 @@ describe('Integration | session', () => { // Now do a click which will create a new session and start recording again domHandler({ name: 'click', + event: new Event('click'), }); const optionsEvent = createOptionsEvent(replay); @@ -294,6 +296,7 @@ describe('Integration | session', () => { // Now do a click which will create a new session and start recording again domHandler({ name: 'click', + event: new Event('click'), }); // Should be same session @@ -367,6 +370,7 @@ describe('Integration | session', () => { // Now do a click domHandler({ name: 'click', + event: new Event('click'), }); // 20 is for the process.nextTick diff --git a/packages/replay/test/integration/stop.test.ts b/packages/replay/test/integration/stop.test.ts index 04c85a9dedde..043dd76bcddc 100644 --- a/packages/replay/test/integration/stop.test.ts +++ b/packages/replay/test/integration/stop.test.ts @@ -20,18 +20,14 @@ describe('Integration | stop', () => { let integration: Replay; const prevLocation = WINDOW.location; - type MockAddInstrumentationHandler = jest.MockedFunction; const { record: mockRecord } = mockRrweb(); - let mockAddInstrumentationHandler: MockAddInstrumentationHandler; + let mockAddDomInstrumentationHandler: jest.SpyInstance; let mockRunFlush: MockRunFlush; beforeAll(async () => { jest.setSystemTime(new Date(BASE_TIMESTAMP)); - mockAddInstrumentationHandler = jest.spyOn( - SentryUtils, - 'addInstrumentationHandler', - ) as MockAddInstrumentationHandler; + mockAddDomInstrumentationHandler = jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler'); ({ replay, integration } = await mockSdk()); @@ -56,7 +52,7 @@ describe('Integration | stop', () => { replay['_initializeSessionForSampling'](); replay.setInitialState(); mockRecord.takeFullSnapshot.mockClear(); - mockAddInstrumentationHandler.mockClear(); + mockAddDomInstrumentationHandler.mockClear(); Object.defineProperty(WINDOW, 'location', { value: prevLocation, writable: true, @@ -166,13 +162,13 @@ describe('Integration | stop', () => { }); }); - it('does not call core SDK `addInstrumentationHandler` after initial setup', async function () { - // NOTE: We clear addInstrumentationHandler mock after every test + it('does not call core SDK `addClickKeypressInstrumentationHandler` after initial setup', async function () { + // NOTE: We clear mockAddDomInstrumentationHandler after every test await integration.stop(); integration.start(); await integration.stop(); integration.start(); - expect(mockAddInstrumentationHandler).not.toHaveBeenCalled(); + expect(mockAddDomInstrumentationHandler).not.toHaveBeenCalled(); }); }); diff --git a/packages/replay/test/mocks/resetSdkMock.ts b/packages/replay/test/mocks/resetSdkMock.ts index aa92cfead4a8..7b3fb7a169b1 100644 --- a/packages/replay/test/mocks/resetSdkMock.ts +++ b/packages/replay/test/mocks/resetSdkMock.ts @@ -26,10 +26,8 @@ export async function resetSdkMock({ replayOptions, sentryOptions, autoStart }: getGlobalSingleton('globalEventProcessors', () => []).length = 0; const SentryUtils = await import('@sentry/utils'); - jest.spyOn(SentryUtils, 'addInstrumentationHandler').mockImplementation((type, handler: (args: any) => any) => { - if (type === 'dom') { - domHandler = handler; - } + jest.spyOn(SentryUtils, 'addClickKeypressInstrumentationHandler').mockImplementation(handler => { + domHandler = handler; }); const { mockRrweb } = await import('./mockRrweb'); const { record: mockRecord } = mockRrweb(); diff --git a/packages/replay/test/types.ts b/packages/replay/test/types.ts index 80a3423dd553..336f11b7bed9 100644 --- a/packages/replay/test/types.ts +++ b/packages/replay/test/types.ts @@ -1 +1,3 @@ -export type DomHandler = (args: any) => any; +import type { HandlerDataDom } from '@sentry/types'; + +export type DomHandler = (data: HandlerDataDom) => void; diff --git a/packages/replay/test/unit/coreHandlers/handleDom.test.ts b/packages/replay/test/unit/coreHandlers/handleDom.test.ts index 99fa5dc1e367..29f39a6c0a42 100644 --- a/packages/replay/test/unit/coreHandlers/handleDom.test.ts +++ b/packages/replay/test/unit/coreHandlers/handleDom.test.ts @@ -1,5 +1,6 @@ +import type { HandlerDataDom } from '@sentry/types'; + import { handleDom } from '../../../src/coreHandlers/handleDom'; -import type { DomHandlerData } from '../../../src/coreHandlers/util/domUtils'; describe('Unit | coreHandlers | handleDom', () => { test('it works with a basic click event on a div', () => { @@ -8,7 +9,7 @@ describe('Unit | coreHandlers | handleDom', () => { target.classList.add('my-class', 'other-class'); parent.appendChild(target); - const handlerData: DomHandlerData = { + const handlerData: HandlerDataDom = { name: 'click', event: { target, @@ -30,7 +31,7 @@ describe('Unit | coreHandlers | handleDom', () => { target.classList.add('my-class', 'other-class'); parent.appendChild(target); - const handlerData: DomHandlerData = { + const handlerData: HandlerDataDom = { name: 'click', event: { target, @@ -55,7 +56,7 @@ describe('Unit | coreHandlers | handleDom', () => { const target = document.createElement('span'); interactive.appendChild(target); - const handlerData: DomHandlerData = { + const handlerData: HandlerDataDom = { name: 'click', event: { target, @@ -80,7 +81,7 @@ describe('Unit | coreHandlers | handleDom', () => { const target = document.createElement('span'); interactive.appendChild(target); - const handlerData: DomHandlerData = { + const handlerData: HandlerDataDom = { name: 'click', event: { target, @@ -111,7 +112,7 @@ describe('Unit | coreHandlers | handleDom', () => { target.classList.add('my-class', 'other-class'); current.appendChild(target); - const handlerData: DomHandlerData = { + const handlerData: HandlerDataDom = { name: 'click', event: { target, diff --git a/packages/replay/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts b/packages/replay/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts index 99b377ba7d17..2d012bd936de 100644 --- a/packages/replay/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts +++ b/packages/replay/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts @@ -118,6 +118,8 @@ describe('Unit | coreHandlers | handleNetworkBreadcrumbs', () => { value: 'test response', }); xhr[SENTRY_XHR_DATA_KEY] = { + method: 'GET', + url: 'https://example.com', request_headers: { 'content-type': 'text/plain', 'other-header': 'test', diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 06d60f219530..2584eb736211 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -1,8 +1,9 @@ /* eslint-disable max-lines */ import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core'; -import type { HandlerDataFetch, Span } from '@sentry/types'; +import type { SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; import { - addInstrumentationHandler, + addFetchInstrumentationHandler, + addXhrInstrumentationHandler, BAGGAGE_HEADER_NAME, browserPerformanceTimeOrigin, dynamicSamplingContextToSentryBaggageHeader, @@ -14,6 +15,13 @@ import { import { instrumentFetchRequest } from '../common/fetch'; import { addPerformanceInstrumentationHandler } from './instrument'; +/** Exported only for tests. */ +export type ExtendedSentryWrappedXMLHttpRequest = SentryWrappedXMLHttpRequest & { + __sentry_xhr_span_id__?: string; + setRequestHeader?: (key: string, val: string) => void; + getRequestHeader?: (key: string) => string; +}; + export const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\/(?!\/)/]; /** Options for Request Instrumentation */ @@ -81,8 +89,6 @@ export interface XHRData { getRequestHeader?: (key: string) => string; __sentry_own_request__?: boolean; }; - startTimestamp: number; - endTimestamp?: number; } export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = { @@ -123,7 +129,7 @@ export function instrumentOutgoingRequests(_options?: Partial = {}; if (traceFetch) { - addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { + addFetchInstrumentationHandler(handlerData => { const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); if (enableHTTPTimings && createdSpan) { addHTTPTimings(createdSpan); @@ -132,7 +138,7 @@ export function instrumentOutgoingRequests(_options?: Partial { + addXhrInstrumentationHandler(handlerData => { const createdSpan = xhrCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); if (enableHTTPTimings && createdSpan) { addHTTPTimings(createdSpan); @@ -252,12 +258,12 @@ export function shouldAttachHeaders(url: string, tracePropagationTargets: (strin */ // eslint-disable-next-line complexity export function xhrCallback( - handlerData: XHRData, + handlerData: HandlerDataXhr, shouldCreateSpan: (url: string) => boolean, shouldAttachHeaders: (url: string) => boolean, spans: Record, ): Span | undefined { - const xhr = handlerData.xhr; + const xhr = handlerData.xhr as ExtendedSentryWrappedXMLHttpRequest; const sentryXhrData = xhr && xhr[SENTRY_XHR_DATA_KEY]; if (!hasTracingEnabled() || (xhr && xhr.__sentry_own_request__) || !xhr || !sentryXhrData) { @@ -272,7 +278,7 @@ export function xhrCallback( if (!spanId) return; const span = spans[spanId]; - if (span) { + if (span && sentryXhrData.status_code !== undefined) { span.setHttpStatus(sentryXhrData.status_code); span.finish(); @@ -290,7 +296,6 @@ export function xhrCallback( shouldCreateSpanResult && parentSpan ? parentSpan.startChild({ data: { - ...sentryXhrData.data, type: 'xhr', 'http.method': sentryXhrData.method, url: sentryXhrData.url, @@ -327,7 +332,7 @@ export function xhrCallback( } function setHeaderOnXhr( - xhr: NonNullable, + xhr: ExtendedSentryWrappedXMLHttpRequest, sentryTraceHeader: string, sentryBaggageHeader: string | undefined, ): void { diff --git a/packages/tracing-internal/src/browser/router.ts b/packages/tracing-internal/src/browser/router.ts index c796e44d3783..98656d4d3f2a 100644 --- a/packages/tracing-internal/src/browser/router.ts +++ b/packages/tracing-internal/src/browser/router.ts @@ -1,5 +1,5 @@ import type { Transaction, TransactionContext } from '@sentry/types'; -import { addInstrumentationHandler, browserPerformanceTimeOrigin, logger } from '@sentry/utils'; +import { addHistoryInstrumentationHandler, browserPerformanceTimeOrigin, logger } from '@sentry/utils'; import { WINDOW } from './types'; @@ -31,7 +31,7 @@ export function instrumentRoutingWithDefaults( } if (startTransactionOnLocationChange) { - addInstrumentationHandler('history', ({ to, from }: { to: string; from?: string }) => { + addHistoryInstrumentationHandler(({ to, from }) => { /** * 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 diff --git a/packages/tracing-internal/test/browser/browsertracing.test.ts b/packages/tracing-internal/test/browser/browsertracing.test.ts index 8a65f6cf8fbe..d1ce55644e6c 100644 --- a/packages/tracing-internal/test/browser/browsertracing.test.ts +++ b/packages/tracing-internal/test/browser/browsertracing.test.ts @@ -1,8 +1,7 @@ /* eslint-disable deprecation/deprecation */ import { Hub, makeMain, TRACING_DEFAULTS } from '@sentry/core'; import * as hubExtensions from '@sentry/core'; -import type { BaseTransportOptions, ClientOptions, DsnComponents } from '@sentry/types'; -import type { InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils'; +import type { BaseTransportOptions, ClientOptions, DsnComponents, HandlerDataHistory } from '@sentry/types'; import { JSDOM } from 'jsdom'; import type { IdleTransaction } from '../../../tracing/src'; @@ -15,17 +14,15 @@ import { instrumentRoutingWithDefaults } from '../../src/browser/router'; import { WINDOW } from '../../src/browser/types'; import { TestClient } from '../utils/TestClient'; -let mockChangeHistory: ({ to, from }: { to: string; from?: string }) => void = () => undefined; +let mockChangeHistory: (data: HandlerDataHistory) => void = () => {}; jest.mock('@sentry/utils', () => { const actual = jest.requireActual('@sentry/utils'); return { ...actual, - addInstrumentationHandler: (type: InstrumentHandlerType, callback: InstrumentHandlerCallback): 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; - } + + addHistoryInstrumentationHandler: (callback: (data: HandlerDataHistory) => void): void => { + mockChangeHistory = callback; }, }; }); diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index b1e46bd05abe..3aa629cc4e7b 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -7,7 +7,7 @@ import { SENTRY_XHR_DATA_KEY } from '@sentry/utils'; import type { Transaction } from '../../../tracing/src'; import { addExtensionMethods, Span, spanStatusfromHttpCode } from '../../../tracing/src'; import { getDefaultBrowserClientOptions } from '../../../tracing/test/testutils'; -import type { XHRData } from '../../src/browser/request'; +import type { ExtendedSentryWrappedXMLHttpRequest } from '../../src/browser/request'; import { extractNetworkProtocol, instrumentOutgoingRequests, @@ -25,7 +25,6 @@ beforeAll(() => { }); const hasTracingEnabled = jest.spyOn(sentryCore, 'hasTracingEnabled'); -const addInstrumentationHandler = jest.spyOn(utils, 'addInstrumentationHandler'); const setRequestHeader = jest.fn(); describe('instrumentOutgoingRequests', () => { @@ -34,22 +33,29 @@ describe('instrumentOutgoingRequests', () => { }); it('instruments fetch and xhr requests', () => { + const addFetchSpy = jest.spyOn(utils, 'addFetchInstrumentationHandler'); + const addXhrSpy = jest.spyOn(utils, 'addXhrInstrumentationHandler'); + instrumentOutgoingRequests(); - expect(addInstrumentationHandler).toHaveBeenCalledWith('fetch', expect.any(Function)); - expect(addInstrumentationHandler).toHaveBeenCalledWith('xhr', expect.any(Function)); + expect(addFetchSpy).toHaveBeenCalledWith(expect.any(Function)); + expect(addXhrSpy).toHaveBeenCalledWith(expect.any(Function)); }); it('does not instrument fetch requests if traceFetch is false', () => { + const addFetchSpy = jest.spyOn(utils, 'addFetchInstrumentationHandler'); + instrumentOutgoingRequests({ traceFetch: false }); - expect(addInstrumentationHandler).not.toHaveBeenCalledWith('fetch', expect.any(Function)); + expect(addFetchSpy).not.toHaveBeenCalled(); }); it('does not instrument xhr requests if traceXHR is false', () => { + const addXhrSpy = jest.spyOn(utils, 'addXhrInstrumentationHandler'); + instrumentOutgoingRequests({ traceXHR: false }); - expect(addInstrumentationHandler).not.toHaveBeenCalledWith('xhr', expect.any(Function)); + expect(addXhrSpy).not.toHaveBeenCalled(); }); }); @@ -155,7 +161,8 @@ describe('callbacks', () => { }); expect(newSpan.description).toBe('GET http://dogs.are.great/'); expect(newSpan.op).toBe('http.client'); - expect(fetchHandlerData.fetchData?.__span).toBeDefined(); + const spanId = fetchHandlerData.fetchData?.__span; + expect(spanId).toBeDefined(); const postRequestFetchHandlerData = { ...fetchHandlerData, @@ -261,7 +268,7 @@ describe('callbacks', () => { }); describe('xhrCallback()', () => { - let xhrHandlerData: XHRData; + let xhrHandlerData: HandlerDataXhr; const xhrSpan = { data: { @@ -279,16 +286,17 @@ describe('callbacks', () => { beforeEach(() => { xhrHandlerData = { + args: ['GET', 'http://dogs.are.great/'], xhr: { [SENTRY_XHR_DATA_KEY]: { method: 'GET', url: 'http://dogs.are.great/', status_code: 200, - data: {}, + request_headers: {}, }, __sentry_xhr_span_id__: '1231201211212012', setRequestHeader, - }, + } as ExtendedSentryWrappedXMLHttpRequest, startTimestamp, }; }); @@ -344,8 +352,9 @@ describe('callbacks', () => { }); expect(newSpan.description).toBe('GET http://dogs.are.great/'); expect(newSpan.op).toBe('http.client'); - expect(xhrHandlerData.xhr?.__sentry_xhr_span_id__).toBeDefined(); - expect(xhrHandlerData.xhr?.__sentry_xhr_span_id__).toEqual(newSpan?.spanId); + const spanId = (xhrHandlerData.xhr as ExtendedSentryWrappedXMLHttpRequest | undefined)?.__sentry_xhr_span_id__; + expect(spanId).toBeDefined(); + expect(spanId).toEqual(newSpan?.spanId); const postRequestXHRHandlerData = { ...xhrHandlerData, @@ -383,12 +392,13 @@ describe('callbacks', () => { it('ignores response with no associated span', () => { // the request might be missed somehow. E.g. if it was sent before tracing gets enabled. - const postRequestXHRHandlerData = { + const postRequestXHRHandlerData: HandlerDataXhr = { ...{ xhr: { [SENTRY_XHR_DATA_KEY]: xhrHandlerData.xhr?.[SENTRY_XHR_DATA_KEY], }, }, + args: ['GET', 'http://dogs.are.great/'], startTimestamp, endTimestamp, }; diff --git a/packages/tracing-internal/test/browser/router.test.ts b/packages/tracing-internal/test/browser/router.test.ts index 2b57ad4bd57f..a78d9e5631bd 100644 --- a/packages/tracing-internal/test/browser/router.test.ts +++ b/packages/tracing-internal/test/browser/router.test.ts @@ -1,17 +1,15 @@ -import type { InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils'; +import type { HandlerDataHistory } from '@sentry/types'; import { JSDOM } from 'jsdom'; import { conditionalTest } from '../../../tracing/test/testutils'; import { instrumentRoutingWithDefaults } from '../../src/browser/router'; -let mockChangeHistory: ({ to, from }: { to: string; from?: string }) => void = () => undefined; -let addInstrumentationHandlerType: string = ''; +let mockChangeHistory: undefined | ((data: HandlerDataHistory) => void); jest.mock('@sentry/utils', () => { const actual = jest.requireActual('@sentry/utils'); return { ...actual, - addInstrumentationHandler: (type: InstrumentHandlerType, callback: InstrumentHandlerCallback): void => { - addInstrumentationHandlerType = type; + addHistoryInstrumentationHandler: (callback: (data: HandlerDataHistory) => void): void => { mockChangeHistory = callback; }, }; @@ -59,8 +57,7 @@ conditionalTest({ min: 16 })('instrumentRoutingWithDefaults', () => { describe('navigation transaction', () => { beforeEach(() => { - mockChangeHistory = () => undefined; - addInstrumentationHandlerType = ''; + mockChangeHistory = undefined; }); it('it is not created automatically', () => { @@ -75,8 +72,8 @@ conditionalTest({ min: 16 })('instrumentRoutingWithDefaults', () => { it('is created on location change', () => { instrumentRoutingWithDefaults(customStartTransaction); - mockChangeHistory({ to: 'here', from: 'there' }); - expect(addInstrumentationHandlerType).toBe('history'); + expect(mockChangeHistory).toBeDefined(); + mockChangeHistory!({ to: 'here', from: 'there' }); expect(customStartTransaction).toHaveBeenCalledTimes(2); expect(customStartTransaction).toHaveBeenLastCalledWith({ @@ -89,8 +86,7 @@ conditionalTest({ min: 16 })('instrumentRoutingWithDefaults', () => { it('is not created if startTransactionOnLocationChange is false', () => { instrumentRoutingWithDefaults(customStartTransaction, true, false); - mockChangeHistory({ to: 'here', from: 'there' }); - expect(addInstrumentationHandlerType).toBe(''); + expect(mockChangeHistory).toBeUndefined(); expect(customStartTransaction).toHaveBeenCalledTimes(1); }); @@ -98,27 +94,31 @@ conditionalTest({ min: 16 })('instrumentRoutingWithDefaults', () => { it('finishes the last active transaction', () => { instrumentRoutingWithDefaults(customStartTransaction); + expect(mockChangeHistory).toBeDefined(); + expect(mockFinish).toHaveBeenCalledTimes(0); - mockChangeHistory({ to: 'here', from: 'there' }); + mockChangeHistory!({ to: 'here', from: 'there' }); expect(mockFinish).toHaveBeenCalledTimes(1); }); it('will finish active transaction multiple times', () => { instrumentRoutingWithDefaults(customStartTransaction); + expect(mockChangeHistory).toBeDefined(); + expect(mockFinish).toHaveBeenCalledTimes(0); - mockChangeHistory({ to: 'here', from: 'there' }); + mockChangeHistory!({ to: 'here', from: 'there' }); expect(mockFinish).toHaveBeenCalledTimes(1); - mockChangeHistory({ to: 'over/there', from: 'here' }); + mockChangeHistory!({ to: 'over/there', from: 'here' }); expect(mockFinish).toHaveBeenCalledTimes(2); - mockChangeHistory({ to: 'nowhere', from: 'over/there' }); + mockChangeHistory!({ to: 'nowhere', from: 'over/there' }); expect(mockFinish).toHaveBeenCalledTimes(3); }); it('not created if `from` is equal to `to`', () => { instrumentRoutingWithDefaults(customStartTransaction); - mockChangeHistory({ to: 'first/path', from: 'first/path' }); - expect(addInstrumentationHandlerType).toBe('history'); + expect(mockChangeHistory).toBeDefined(); + mockChangeHistory!({ to: 'first/path', from: 'first/path' }); expect(customStartTransaction).toHaveBeenCalledTimes(1); expect(customStartTransaction).not.toHaveBeenLastCalledWith('navigation'); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 27068f8080f8..4c648f35363e 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -124,7 +124,18 @@ export type { export type { User, UserFeedback } from './user'; export type { WrappedFunction } from './wrappedfunction'; export type { Instrumenter } from './instrumenter'; -export type { HandlerDataFetch, HandlerDataXhr, SentryXhrData, SentryWrappedXMLHttpRequest } from './instrument'; +export type { + HandlerDataFetch, + HandlerDataXhr, + HandlerDataDom, + HandlerDataConsole, + HandlerDataHistory, + HandlerDataError, + HandlerDataUnhandledRejection, + ConsoleLevel, + SentryXhrData, + SentryWrappedXMLHttpRequest, +} from './instrument'; export type { BrowserClientReplayOptions, BrowserClientProfilingOptions } from './browseroptions'; export type { CheckIn, MonitorConfig, FinishedCheckIn, InProgressCheckIn, SerializedCheckIn } from './checkin'; diff --git a/packages/types/src/instrument.ts b/packages/types/src/instrument.ts index d3581d694312..70215f1b968c 100644 --- a/packages/types/src/instrument.ts +++ b/packages/types/src/instrument.ts @@ -3,15 +3,17 @@ // Make sure to cast it where needed! type XHRSendInput = unknown; +export type ConsoleLevel = 'debug' | 'info' | 'warn' | 'error' | 'log' | 'assert' | 'trace'; + export interface SentryWrappedXMLHttpRequest { - __sentry_xhr_v2__?: SentryXhrData; + __sentry_xhr_v3__?: SentryXhrData; __sentry_own_request__?: boolean; } // WARNING: When the shape of this type is changed bump the version in `SentryWrappedXMLHttpRequest` export interface SentryXhrData { - method?: string; - url?: string; + method: string; + url: string; status_code?: number; body?: XHRSendInput; request_body_size?: number; @@ -20,6 +22,9 @@ export interface SentryXhrData { } export interface HandlerDataXhr { + /** + * @deprecated This property will be removed in v8. + */ args: [string, string]; xhr: SentryWrappedXMLHttpRequest; startTimestamp?: number; @@ -55,3 +60,32 @@ export interface HandlerDataFetch { }; error?: unknown; } + +export interface HandlerDataDom { + event: Event | { target: EventTarget }; + name: string; + global?: boolean; +} + +export interface HandlerDataConsole { + level: ConsoleLevel; + args: any[]; +} + +export interface HandlerDataHistory { + from: string | undefined; + to: string; +} + +export interface HandlerDataError { + column?: number; + error?: Error; + line?: number; + msg: string | Event; + url?: string; +} + +export interface HandlerDataUnhandledRejection { + reason?: Error; + detail?: { reason?: Error }; +} diff --git a/packages/utils/src/error.ts b/packages/utils/src/error.ts index ae64f09a3643..af0f80e9dce7 100644 --- a/packages/utils/src/error.ts +++ b/packages/utils/src/error.ts @@ -1,4 +1,4 @@ -import type { ConsoleLevel } from './logger'; +import type { ConsoleLevel } from '@sentry/types'; /** An error emitted by Sentry SDKs and related utilities. */ export class SentryError extends Error { diff --git a/packages/utils/src/instrument.ts b/packages/utils/src/instrument.ts deleted file mode 100644 index 5d5ce0f7c616..000000000000 --- a/packages/utils/src/instrument.ts +++ /dev/null @@ -1,690 +0,0 @@ -/* eslint-disable max-lines */ -/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable @typescript-eslint/ban-types */ -import type { - HandlerDataFetch, - HandlerDataXhr, - SentryWrappedXMLHttpRequest, - SentryXhrData, - WrappedFunction, -} from '@sentry/types'; - -import { isString } from './is'; -import type { ConsoleLevel } from './logger'; -import { CONSOLE_LEVELS, logger, originalConsoleMethods } from './logger'; -import { uuid4 } from './misc'; -import { addNonEnumerableProperty, fill } from './object'; -import { getFunctionName } from './stacktrace'; -import { supportsHistory, supportsNativeFetch } from './supports'; -import { getGlobalObject, GLOBAL_OBJ } from './worldwide'; - -// eslint-disable-next-line deprecation/deprecation -const WINDOW = getGlobalObject(); - -export const SENTRY_XHR_DATA_KEY = '__sentry_xhr_v2__'; - -export type InstrumentHandlerType = - | 'console' - | 'dom' - | 'fetch' - | 'history' - | 'sentry' - | 'xhr' - | 'error' - | 'unhandledrejection'; -export type InstrumentHandlerCallback = (data: any) => void; - -/** - * Instrument native APIs to call handlers that can be used to create breadcrumbs, APM spans etc. - * - Console API - * - Fetch API - * - XHR API - * - History API - * - DOM API (click/typing) - * - Error API - * - UnhandledRejection API - */ - -const handlers: { [key in InstrumentHandlerType]?: InstrumentHandlerCallback[] } = {}; -const instrumented: { [key in InstrumentHandlerType]?: boolean } = {}; - -/** Instruments given API */ -function instrument(type: InstrumentHandlerType): void { - if (instrumented[type]) { - return; - } - - instrumented[type] = true; - - switch (type) { - case 'console': - instrumentConsole(); - break; - case 'dom': - instrumentDOM(); - break; - case 'xhr': - instrumentXHR(); - break; - case 'fetch': - instrumentFetch(); - break; - case 'history': - instrumentHistory(); - break; - case 'error': - instrumentError(); - break; - case 'unhandledrejection': - instrumentUnhandledRejection(); - break; - default: - __DEBUG_BUILD__ && logger.warn('unknown instrumentation type:', type); - return; - } -} - -/** - * Add handler that will be called when given type of instrumentation triggers. - * Use at your own risk, this might break without changelog notice, only used internally. - * @hidden - */ -export function addInstrumentationHandler(type: InstrumentHandlerType, callback: InstrumentHandlerCallback): void { - handlers[type] = handlers[type] || []; - (handlers[type] as InstrumentHandlerCallback[]).push(callback); - instrument(type); -} - -/** - * Reset all instrumentation handlers. - * This can be used by tests to ensure we have a clean slate of instrumentation handlers. - */ -export function resetInstrumentationHandlers(): void { - Object.keys(handlers).forEach(key => { - handlers[key as InstrumentHandlerType] = undefined; - }); -} - -/** JSDoc */ -function triggerHandlers(type: InstrumentHandlerType, data: any): void { - if (!type || !handlers[type]) { - return; - } - - for (const handler of handlers[type] || []) { - try { - handler(data); - } catch (e) { - __DEBUG_BUILD__ && - logger.error( - `Error while triggering instrumentation handler.\nType: ${type}\nName: ${getFunctionName(handler)}\nError:`, - e, - ); - } - } -} - -/** JSDoc */ -function instrumentConsole(): void { - if (!('console' in GLOBAL_OBJ)) { - return; - } - - CONSOLE_LEVELS.forEach(function (level: ConsoleLevel): void { - if (!(level in GLOBAL_OBJ.console)) { - return; - } - - fill(GLOBAL_OBJ.console, level, function (originalConsoleMethod: () => any): Function { - originalConsoleMethods[level] = originalConsoleMethod; - - return function (...args: any[]): void { - triggerHandlers('console', { args, level }); - - const log = originalConsoleMethods[level]; - log && log.apply(GLOBAL_OBJ.console, args); - }; - }); - }); -} - -/** JSDoc */ -function instrumentFetch(): void { - if (!supportsNativeFetch()) { - return; - } - - fill(GLOBAL_OBJ, 'fetch', function (originalFetch: () => void): () => void { - return function (...args: any[]): void { - const { method, url } = parseFetchArgs(args); - - const handlerData: HandlerDataFetch = { - args, - fetchData: { - method, - url, - }, - startTimestamp: Date.now(), - }; - - triggerHandlers('fetch', { - ...handlerData, - }); - - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return originalFetch.apply(GLOBAL_OBJ, args).then( - (response: Response) => { - triggerHandlers('fetch', { - ...handlerData, - endTimestamp: Date.now(), - response, - }); - return response; - }, - (error: Error) => { - triggerHandlers('fetch', { - ...handlerData, - endTimestamp: Date.now(), - error, - }); - // NOTE: If you are a Sentry user, and you are seeing this stack frame, - // it means the sentry.javascript SDK caught an error invoking your application code. - // This is expected behavior and NOT indicative of a bug with sentry.javascript. - throw error; - }, - ); - }; - }); -} - -function hasProp(obj: unknown, prop: T): obj is Record { - return !!obj && typeof obj === 'object' && !!(obj as Record)[prop]; -} - -type FetchResource = string | { toString(): string } | { url: string }; - -function getUrlFromResource(resource: FetchResource): string { - if (typeof resource === 'string') { - return resource; - } - - if (!resource) { - return ''; - } - - if (hasProp(resource, 'url')) { - return resource.url; - } - - if (resource.toString) { - return resource.toString(); - } - - return ''; -} - -/** - * Parses the fetch arguments to find the used Http method and the url of the request - */ -export function parseFetchArgs(fetchArgs: unknown[]): { method: string; url: string } { - if (fetchArgs.length === 0) { - return { method: 'GET', url: '' }; - } - - if (fetchArgs.length === 2) { - const [url, options] = fetchArgs as [FetchResource, object]; - - return { - url: getUrlFromResource(url), - method: hasProp(options, 'method') ? String(options.method).toUpperCase() : 'GET', - }; - } - - const arg = fetchArgs[0]; - return { - url: getUrlFromResource(arg as FetchResource), - method: hasProp(arg, 'method') ? String(arg.method).toUpperCase() : 'GET', - }; -} - -/** JSDoc */ -export function instrumentXHR(): void { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (!(WINDOW as any).XMLHttpRequest) { - return; - } - - const xhrproto = XMLHttpRequest.prototype; - - fill(xhrproto, 'open', function (originalOpen: () => void): () => void { - return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: any[]): void { - const startTimestamp = Date.now(); - - const url = args[1]; - const xhrInfo: SentryXhrData = (this[SENTRY_XHR_DATA_KEY] = { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - method: isString(args[0]) ? args[0].toUpperCase() : args[0], - url: args[1], - request_headers: {}, - }); - - // if Sentry key appears in URL, don't capture it as a request - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (isString(url) && xhrInfo.method === 'POST' && url.match(/sentry_key/)) { - this.__sentry_own_request__ = true; - } - - const onreadystatechangeHandler: () => void = () => { - // For whatever reason, this is not the same instance here as from the outer method - const xhrInfo = this[SENTRY_XHR_DATA_KEY]; - - if (!xhrInfo) { - return; - } - - if (this.readyState === 4) { - try { - // touching statusCode in some platforms throws - // an exception - xhrInfo.status_code = this.status; - } catch (e) { - /* do nothing */ - } - - triggerHandlers('xhr', { - args: args as [string, string], - endTimestamp: Date.now(), - startTimestamp, - xhr: this, - } as HandlerDataXhr); - } - }; - - if ('onreadystatechange' in this && typeof this.onreadystatechange === 'function') { - fill(this, 'onreadystatechange', function (original: WrappedFunction): Function { - return function (this: SentryWrappedXMLHttpRequest, ...readyStateArgs: any[]): void { - onreadystatechangeHandler(); - return original.apply(this, readyStateArgs); - }; - }); - } else { - this.addEventListener('readystatechange', onreadystatechangeHandler); - } - - // Intercepting `setRequestHeader` to access the request headers of XHR instance. - // This will only work for user/library defined headers, not for the default/browser-assigned headers. - // Request cookies are also unavailable for XHR, as `Cookie` header can't be defined by `setRequestHeader`. - fill(this, 'setRequestHeader', function (original: WrappedFunction): Function { - return function (this: SentryWrappedXMLHttpRequest, ...setRequestHeaderArgs: unknown[]): void { - const [header, value] = setRequestHeaderArgs as [string, string]; - - const xhrInfo = this[SENTRY_XHR_DATA_KEY]; - - if (xhrInfo) { - xhrInfo.request_headers[header.toLowerCase()] = value; - } - - return original.apply(this, setRequestHeaderArgs); - }; - }); - - return originalOpen.apply(this, args); - }; - }); - - fill(xhrproto, 'send', function (originalSend: () => void): () => void { - return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: any[]): void { - const sentryXhrData = this[SENTRY_XHR_DATA_KEY]; - if (sentryXhrData && args[0] !== undefined) { - sentryXhrData.body = args[0]; - } - - triggerHandlers('xhr', { - args, - startTimestamp: Date.now(), - xhr: this, - }); - - return originalSend.apply(this, args); - }; - }); -} - -let lastHref: string; - -/** JSDoc */ -function instrumentHistory(): void { - if (!supportsHistory()) { - return; - } - - const oldOnPopState = WINDOW.onpopstate; - WINDOW.onpopstate = function (this: WindowEventHandlers, ...args: any[]): any { - const to = WINDOW.location.href; - // keep track of the current URL state, as we always receive only the updated state - const from = lastHref; - lastHref = to; - triggerHandlers('history', { - from, - to, - }); - if (oldOnPopState) { - // Apparently this can throw in Firefox when incorrectly implemented plugin is installed. - // https://github.com/getsentry/sentry-javascript/issues/3344 - // https://github.com/bugsnag/bugsnag-js/issues/469 - try { - return oldOnPopState.apply(this, args); - } catch (_oO) { - // no-empty - } - } - }; - - /** @hidden */ - function historyReplacementFunction(originalHistoryFunction: () => void): () => void { - return function (this: History, ...args: any[]): void { - const url = args.length > 2 ? args[2] : undefined; - if (url) { - // coerce to string (this is what pushState does) - const from = lastHref; - const to = String(url); - // keep track of the current URL state, as we always receive only the updated state - lastHref = to; - triggerHandlers('history', { - from, - to, - }); - } - return originalHistoryFunction.apply(this, args); - }; - } - - fill(WINDOW.history, 'pushState', historyReplacementFunction); - fill(WINDOW.history, 'replaceState', historyReplacementFunction); -} - -const DEBOUNCE_DURATION = 1000; -let debounceTimerID: number | undefined; -let lastCapturedEventType: string | undefined; -let lastCapturedEventTargetId: string | undefined; - -type SentryWrappedTarget = HTMLElement & { _sentryId?: string }; - -/** - * Check whether the event is similar to the last captured one. For example, two click events on the same button. - */ -function isSimilarToLastCapturedEvent(event: Event): boolean { - // If both events have different type, then user definitely performed two separate actions. e.g. click + keypress. - if (event.type !== lastCapturedEventType) { - return false; - } - - try { - // If both events have the same type, it's still possible that actions were performed on different targets. - // e.g. 2 clicks on different buttons. - if (!event.target || (event.target as SentryWrappedTarget)._sentryId !== lastCapturedEventTargetId) { - return false; - } - } catch (e) { - // just accessing `target` property can throw an exception in some rare circumstances - // see: https://github.com/getsentry/sentry-javascript/issues/838 - } - - // If both events have the same type _and_ same `target` (an element which triggered an event, _not necessarily_ - // to which an event listener was attached), we treat them as the same action, as we want to capture - // only one breadcrumb. e.g. multiple clicks on the same button, or typing inside a user input box. - return true; -} - -/** - * Decide whether an event should be captured. - * @param event event to be captured - */ -function shouldSkipDOMEvent(eventType: string, target: SentryWrappedTarget | null): boolean { - // We are only interested in filtering `keypress` events for now. - if (eventType !== 'keypress') { - return false; - } - - if (!target || !target.tagName) { - return true; - } - - // Only consider keypress events on actual input elements. This will disregard keypresses targeting body - // e.g.tabbing through elements, hotkeys, etc. - if (target.tagName === 'INPUT' || target.tagName === 'TEXTAREA' || target.isContentEditable) { - return false; - } - - return true; -} - -function getEventTarget(event: Event): SentryWrappedTarget | null { - try { - return event.target as SentryWrappedTarget | null; - } catch (e) { - // just accessing `target` property can throw an exception in some rare circumstances - // see: https://github.com/getsentry/sentry-javascript/issues/838 - return null; - } -} - -/** - * Wraps addEventListener to capture UI breadcrumbs - * @param handler function that will be triggered - * @param globalListener indicates whether event was captured by the global event listener - * @returns wrapped breadcrumb events handler - * @hidden - */ -function makeDOMEventHandler(handler: Function, globalListener: boolean = false): (event: Event) => void { - return (event: Event & { _sentryCaptured?: true }): void => { - // It's possible this handler might trigger multiple times for the same - // event (e.g. event propagation through node ancestors). - // Ignore if we've already captured that event. - if (!event || event['_sentryCaptured']) { - return; - } - - const target = getEventTarget(event); - - // We always want to skip _some_ events. - if (shouldSkipDOMEvent(event.type, target)) { - return; - } - - // Mark event as "seen" - addNonEnumerableProperty(event, '_sentryCaptured', true); - - if (target && !target._sentryId) { - // Add UUID to event target so we can identify if - addNonEnumerableProperty(target, '_sentryId', uuid4()); - } - - const name = event.type === 'keypress' ? 'input' : event.type; - - // If there is no last captured event, it means that we can safely capture the new event and store it for future comparisons. - // If there is a last captured event, see if the new event is different enough to treat it as a unique one. - // If that's the case, emit the previous event and store locally the newly-captured DOM event. - if (!isSimilarToLastCapturedEvent(event)) { - handler({ - event: event, - name, - global: globalListener, - }); - lastCapturedEventType = event.type; - lastCapturedEventTargetId = target ? target._sentryId : undefined; - } - - // Start a new debounce timer that will prevent us from capturing multiple events that should be grouped together. - clearTimeout(debounceTimerID); - debounceTimerID = WINDOW.setTimeout(() => { - lastCapturedEventTargetId = undefined; - lastCapturedEventType = undefined; - }, DEBOUNCE_DURATION); - }; -} - -type AddEventListener = ( - type: string, - listener: EventListenerOrEventListenerObject, - options?: boolean | AddEventListenerOptions, -) => void; -type RemoveEventListener = ( - type: string, - listener: EventListenerOrEventListenerObject, - options?: boolean | EventListenerOptions, -) => void; - -type InstrumentedElement = Element & { - __sentry_instrumentation_handlers__?: { - [key in 'click' | 'keypress']?: { - handler?: Function; - /** The number of custom listeners attached to this element */ - refCount: number; - }; - }; -}; - -/** JSDoc */ -export function instrumentDOM(): void { - if (!WINDOW.document) { - return; - } - - // Make it so that any click or keypress that is unhandled / bubbled up all the way to the document triggers our dom - // handlers. (Normally we have only one, which captures a breadcrumb for each click or keypress.) Do this before - // we instrument `addEventListener` so that we don't end up attaching this handler twice. - const triggerDOMHandler = triggerHandlers.bind(null, 'dom'); - const globalDOMEventHandler = makeDOMEventHandler(triggerDOMHandler, true); - WINDOW.document.addEventListener('click', globalDOMEventHandler, false); - WINDOW.document.addEventListener('keypress', globalDOMEventHandler, false); - - // After hooking into click and keypress events bubbled up to `document`, we also hook into user-handled - // clicks & keypresses, by adding an event listener of our own to any element to which they add a listener. That - // way, whenever one of their handlers is triggered, ours will be, too. (This is needed because their handler - // could potentially prevent the event from bubbling up to our global listeners. This way, our handler are still - // guaranteed to fire at least once.) - ['EventTarget', 'Node'].forEach((target: string) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const proto = (WINDOW as any)[target] && (WINDOW as any)[target].prototype; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, no-prototype-builtins - if (!proto || !proto.hasOwnProperty || !proto.hasOwnProperty('addEventListener')) { - return; - } - - fill(proto, 'addEventListener', function (originalAddEventListener: AddEventListener): AddEventListener { - return function ( - this: Element, - type: string, - listener: EventListenerOrEventListenerObject, - options?: boolean | AddEventListenerOptions, - ): AddEventListener { - if (type === 'click' || type == 'keypress') { - try { - const el = this as InstrumentedElement; - const handlers = (el.__sentry_instrumentation_handlers__ = el.__sentry_instrumentation_handlers__ || {}); - const handlerForType = (handlers[type] = handlers[type] || { refCount: 0 }); - - if (!handlerForType.handler) { - const handler = makeDOMEventHandler(triggerDOMHandler); - handlerForType.handler = handler; - originalAddEventListener.call(this, type, handler, options); - } - - handlerForType.refCount++; - } catch (e) { - // Accessing dom properties is always fragile. - // Also allows us to skip `addEventListenrs` calls with no proper `this` context. - } - } - - return originalAddEventListener.call(this, type, listener, options); - }; - }); - - fill( - proto, - 'removeEventListener', - function (originalRemoveEventListener: RemoveEventListener): RemoveEventListener { - return function ( - this: Element, - type: string, - listener: EventListenerOrEventListenerObject, - options?: boolean | EventListenerOptions, - ): () => void { - if (type === 'click' || type == 'keypress') { - try { - const el = this as InstrumentedElement; - const handlers = el.__sentry_instrumentation_handlers__ || {}; - const handlerForType = handlers[type]; - - if (handlerForType) { - handlerForType.refCount--; - // If there are no longer any custom handlers of the current type on this element, we can remove ours, too. - if (handlerForType.refCount <= 0) { - originalRemoveEventListener.call(this, type, handlerForType.handler, options); - handlerForType.handler = undefined; - delete handlers[type]; // eslint-disable-line @typescript-eslint/no-dynamic-delete - } - - // If there are no longer any custom handlers of any type on this element, cleanup everything. - if (Object.keys(handlers).length === 0) { - delete el.__sentry_instrumentation_handlers__; - } - } - } catch (e) { - // Accessing dom properties is always fragile. - // Also allows us to skip `addEventListenrs` calls with no proper `this` context. - } - } - - return originalRemoveEventListener.call(this, type, listener, options); - }; - }, - ); - }); -} - -let _oldOnErrorHandler: (typeof WINDOW)['onerror'] | null = null; -/** JSDoc */ -function instrumentError(): void { - _oldOnErrorHandler = WINDOW.onerror; - - WINDOW.onerror = function (msg: unknown, url: unknown, line: unknown, column: unknown, error: unknown): boolean { - triggerHandlers('error', { - column, - error, - line, - msg, - url, - }); - - if (_oldOnErrorHandler && !_oldOnErrorHandler.__SENTRY_LOADER__) { - // eslint-disable-next-line prefer-rest-params - return _oldOnErrorHandler.apply(this, arguments); - } - - return false; - }; - - WINDOW.onerror.__SENTRY_INSTRUMENTED__ = true; -} - -let _oldOnUnhandledRejectionHandler: (typeof WINDOW)['onunhandledrejection'] | null = null; -/** JSDoc */ -function instrumentUnhandledRejection(): void { - _oldOnUnhandledRejectionHandler = WINDOW.onunhandledrejection; - - WINDOW.onunhandledrejection = function (e: any): boolean { - triggerHandlers('unhandledrejection', e); - - if (_oldOnUnhandledRejectionHandler && !_oldOnUnhandledRejectionHandler.__SENTRY_LOADER__) { - // eslint-disable-next-line prefer-rest-params - return _oldOnUnhandledRejectionHandler.apply(this, arguments); - } - - return true; - }; - - WINDOW.onunhandledrejection.__SENTRY_INSTRUMENTED__ = true; -} diff --git a/packages/utils/src/instrument/_handlers.ts b/packages/utils/src/instrument/_handlers.ts new file mode 100644 index 000000000000..e86ab677ed39 --- /dev/null +++ b/packages/utils/src/instrument/_handlers.ts @@ -0,0 +1,54 @@ +import { logger } from '../logger'; +import { getFunctionName } from '../stacktrace'; + +export type InstrumentHandlerType = 'console' | 'dom' | 'fetch' | 'history' | 'xhr' | 'error' | 'unhandledrejection'; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type InstrumentHandlerCallback = (data: any) => void; + +// We keep the handlers globally +const handlers: { [key in InstrumentHandlerType]?: InstrumentHandlerCallback[] } = {}; +const instrumented: { [key in InstrumentHandlerType]?: boolean } = {}; + +/** Add a handler function. */ +export function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallback): void { + handlers[type] = handlers[type] || []; + (handlers[type] as InstrumentHandlerCallback[]).push(handler); +} + +/** + * Reset all instrumentation handlers. + * This can be used by tests to ensure we have a clean slate of instrumentation handlers. + */ +export function resetInstrumentationHandlers(): void { + Object.keys(handlers).forEach(key => { + handlers[key as InstrumentHandlerType] = undefined; + }); +} + +/** Maybe run an instrumentation function, unless it was already called. */ +export function maybeInstrument(type: InstrumentHandlerType, instrumentFn: () => void): void { + if (!instrumented[type]) { + instrumentFn(); + instrumented[type] = true; + } +} + +/** Trigger handlers for a given instrumentation type. */ +export function triggerHandlers(type: InstrumentHandlerType, data: unknown): void { + const typeHandlers = type && handlers[type]; + if (!typeHandlers) { + return; + } + + for (const handler of typeHandlers) { + try { + handler(data); + } catch (e) { + __DEBUG_BUILD__ && + logger.error( + `Error while triggering instrumentation handler.\nType: ${type}\nName: ${getFunctionName(handler)}\nError:`, + e, + ); + } + } +} diff --git a/packages/utils/src/instrument/console.ts b/packages/utils/src/instrument/console.ts new file mode 100644 index 000000000000..7570f58a55dc --- /dev/null +++ b/packages/utils/src/instrument/console.ts @@ -0,0 +1,44 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/ban-types */ +import type { ConsoleLevel, HandlerDataConsole } from '@sentry/types'; + +import { CONSOLE_LEVELS, originalConsoleMethods } from '../logger'; +import { fill } from '../object'; +import { GLOBAL_OBJ } from '../worldwide'; +import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; + +/** + * Add an instrumentation handler for when a console.xxx method is called. + * + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + */ +export function addConsoleInstrumentationHandler(handler: (data: HandlerDataConsole) => void): void { + const type = 'console'; + addHandler(type, handler); + maybeInstrument(type, instrumentConsole); +} + +function instrumentConsole(): void { + if (!('console' in GLOBAL_OBJ)) { + return; + } + + CONSOLE_LEVELS.forEach(function (level: ConsoleLevel): void { + if (!(level in GLOBAL_OBJ.console)) { + return; + } + + fill(GLOBAL_OBJ.console, level, function (originalConsoleMethod: () => any): Function { + originalConsoleMethods[level] = originalConsoleMethod; + + return function (...args: any[]): void { + const handlerData: HandlerDataConsole = { args, level }; + triggerHandlers('console', handlerData); + + const log = originalConsoleMethods[level]; + log && log.apply(GLOBAL_OBJ.console, args); + }; + }); + }); +} diff --git a/packages/utils/src/instrument/dom.ts b/packages/utils/src/instrument/dom.ts new file mode 100644 index 000000000000..71f64e43ebc5 --- /dev/null +++ b/packages/utils/src/instrument/dom.ts @@ -0,0 +1,260 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/ban-types */ +import type { HandlerDataDom } from '@sentry/types'; + +import { uuid4 } from '../misc'; +import { addNonEnumerableProperty, fill } from '../object'; +import { GLOBAL_OBJ } from '../worldwide'; +import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; + +type SentryWrappedTarget = HTMLElement & { _sentryId?: string }; + +type AddEventListener = ( + type: string, + listener: EventListenerOrEventListenerObject, + options?: boolean | AddEventListenerOptions, +) => void; +type RemoveEventListener = ( + type: string, + listener: EventListenerOrEventListenerObject, + options?: boolean | EventListenerOptions, +) => void; + +type InstrumentedElement = Element & { + __sentry_instrumentation_handlers__?: { + [key in 'click' | 'keypress']?: { + handler?: Function; + /** The number of custom listeners attached to this element */ + refCount: number; + }; + }; +}; + +const WINDOW = GLOBAL_OBJ as unknown as Window; +const DEBOUNCE_DURATION = 1000; + +let debounceTimerID: number | undefined; +let lastCapturedEventType: string | undefined; +let lastCapturedEventTargetId: string | undefined; + +/** + * Add an instrumentation handler for when a click or a keypress happens. + * + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + */ +export function addClickKeypressInstrumentationHandler(handler: (data: HandlerDataDom) => void): void { + const type = 'dom'; + addHandler(type, handler); + maybeInstrument(type, instrumentDOM); +} + +/** Exported for tests only. */ +export function instrumentDOM(): void { + if (!WINDOW.document) { + return; + } + + // Make it so that any click or keypress that is unhandled / bubbled up all the way to the document triggers our dom + // handlers. (Normally we have only one, which captures a breadcrumb for each click or keypress.) Do this before + // we instrument `addEventListener` so that we don't end up attaching this handler twice. + const triggerDOMHandler = triggerHandlers.bind(null, 'dom'); + const globalDOMEventHandler = makeDOMEventHandler(triggerDOMHandler, true); + WINDOW.document.addEventListener('click', globalDOMEventHandler, false); + WINDOW.document.addEventListener('keypress', globalDOMEventHandler, false); + + // After hooking into click and keypress events bubbled up to `document`, we also hook into user-handled + // clicks & keypresses, by adding an event listener of our own to any element to which they add a listener. That + // way, whenever one of their handlers is triggered, ours will be, too. (This is needed because their handler + // could potentially prevent the event from bubbling up to our global listeners. This way, our handler are still + // guaranteed to fire at least once.) + ['EventTarget', 'Node'].forEach((target: string) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const proto = (WINDOW as any)[target] && (WINDOW as any)[target].prototype; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, no-prototype-builtins + if (!proto || !proto.hasOwnProperty || !proto.hasOwnProperty('addEventListener')) { + return; + } + + fill(proto, 'addEventListener', function (originalAddEventListener: AddEventListener): AddEventListener { + return function ( + this: Element, + type: string, + listener: EventListenerOrEventListenerObject, + options?: boolean | AddEventListenerOptions, + ): AddEventListener { + if (type === 'click' || type == 'keypress') { + try { + const el = this as InstrumentedElement; + const handlers = (el.__sentry_instrumentation_handlers__ = el.__sentry_instrumentation_handlers__ || {}); + const handlerForType = (handlers[type] = handlers[type] || { refCount: 0 }); + + if (!handlerForType.handler) { + const handler = makeDOMEventHandler(triggerDOMHandler); + handlerForType.handler = handler; + originalAddEventListener.call(this, type, handler, options); + } + + handlerForType.refCount++; + } catch (e) { + // Accessing dom properties is always fragile. + // Also allows us to skip `addEventListenrs` calls with no proper `this` context. + } + } + + return originalAddEventListener.call(this, type, listener, options); + }; + }); + + fill( + proto, + 'removeEventListener', + function (originalRemoveEventListener: RemoveEventListener): RemoveEventListener { + return function ( + this: Element, + type: string, + listener: EventListenerOrEventListenerObject, + options?: boolean | EventListenerOptions, + ): () => void { + if (type === 'click' || type == 'keypress') { + try { + const el = this as InstrumentedElement; + const handlers = el.__sentry_instrumentation_handlers__ || {}; + const handlerForType = handlers[type]; + + if (handlerForType) { + handlerForType.refCount--; + // If there are no longer any custom handlers of the current type on this element, we can remove ours, too. + if (handlerForType.refCount <= 0) { + originalRemoveEventListener.call(this, type, handlerForType.handler, options); + handlerForType.handler = undefined; + delete handlers[type]; // eslint-disable-line @typescript-eslint/no-dynamic-delete + } + + // If there are no longer any custom handlers of any type on this element, cleanup everything. + if (Object.keys(handlers).length === 0) { + delete el.__sentry_instrumentation_handlers__; + } + } + } catch (e) { + // Accessing dom properties is always fragile. + // Also allows us to skip `addEventListenrs` calls with no proper `this` context. + } + } + + return originalRemoveEventListener.call(this, type, listener, options); + }; + }, + ); + }); +} + +/** + * Check whether the event is similar to the last captured one. For example, two click events on the same button. + */ +function isSimilarToLastCapturedEvent(event: Event): boolean { + // If both events have different type, then user definitely performed two separate actions. e.g. click + keypress. + if (event.type !== lastCapturedEventType) { + return false; + } + + try { + // If both events have the same type, it's still possible that actions were performed on different targets. + // e.g. 2 clicks on different buttons. + if (!event.target || (event.target as SentryWrappedTarget)._sentryId !== lastCapturedEventTargetId) { + return false; + } + } catch (e) { + // just accessing `target` property can throw an exception in some rare circumstances + // see: https://github.com/getsentry/sentry-javascript/issues/838 + } + + // If both events have the same type _and_ same `target` (an element which triggered an event, _not necessarily_ + // to which an event listener was attached), we treat them as the same action, as we want to capture + // only one breadcrumb. e.g. multiple clicks on the same button, or typing inside a user input box. + return true; +} + +/** + * Decide whether an event should be captured. + * @param event event to be captured + */ +function shouldSkipDOMEvent(eventType: string, target: SentryWrappedTarget | null): boolean { + // We are only interested in filtering `keypress` events for now. + if (eventType !== 'keypress') { + return false; + } + + if (!target || !target.tagName) { + return true; + } + + // Only consider keypress events on actual input elements. This will disregard keypresses targeting body + // e.g.tabbing through elements, hotkeys, etc. + if (target.tagName === 'INPUT' || target.tagName === 'TEXTAREA' || target.isContentEditable) { + return false; + } + + return true; +} + +/** + * Wraps addEventListener to capture UI breadcrumbs + */ +function makeDOMEventHandler( + handler: (data: HandlerDataDom) => void, + globalListener: boolean = false, +): (event: Event) => void { + return (event: Event & { _sentryCaptured?: true }): void => { + // It's possible this handler might trigger multiple times for the same + // event (e.g. event propagation through node ancestors). + // Ignore if we've already captured that event. + if (!event || event['_sentryCaptured']) { + return; + } + + const target = getEventTarget(event); + + // We always want to skip _some_ events. + if (shouldSkipDOMEvent(event.type, target)) { + return; + } + + // Mark event as "seen" + addNonEnumerableProperty(event, '_sentryCaptured', true); + + if (target && !target._sentryId) { + // Add UUID to event target so we can identify if + addNonEnumerableProperty(target, '_sentryId', uuid4()); + } + + const name = event.type === 'keypress' ? 'input' : event.type; + + // If there is no last captured event, it means that we can safely capture the new event and store it for future comparisons. + // If there is a last captured event, see if the new event is different enough to treat it as a unique one. + // If that's the case, emit the previous event and store locally the newly-captured DOM event. + if (!isSimilarToLastCapturedEvent(event)) { + const handlerData: HandlerDataDom = { event, name, global: globalListener }; + handler(handlerData); + lastCapturedEventType = event.type; + lastCapturedEventTargetId = target ? target._sentryId : undefined; + } + + // Start a new debounce timer that will prevent us from capturing multiple events that should be grouped together. + clearTimeout(debounceTimerID); + debounceTimerID = WINDOW.setTimeout(() => { + lastCapturedEventTargetId = undefined; + lastCapturedEventType = undefined; + }, DEBOUNCE_DURATION); + }; +} + +function getEventTarget(event: Event): SentryWrappedTarget | null { + try { + return event.target as SentryWrappedTarget | null; + } catch (e) { + // just accessing `target` property can throw an exception in some rare circumstances + // see: https://github.com/getsentry/sentry-javascript/issues/838 + return null; + } +} diff --git a/packages/utils/src/instrument/fetch.ts b/packages/utils/src/instrument/fetch.ts new file mode 100644 index 000000000000..a02dc5db42ab --- /dev/null +++ b/packages/utils/src/instrument/fetch.ts @@ -0,0 +1,125 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/ban-types */ +import type { HandlerDataFetch } from '@sentry/types'; + +import { fill } from '../object'; +import { supportsNativeFetch } from '../supports'; +import { GLOBAL_OBJ } from '../worldwide'; +import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; + +type FetchResource = string | { toString(): string } | { url: string }; + +/** + * Add an instrumentation handler for when a fetch request happens. + * The handler function is called once when the request starts and once when it ends, + * which can be identified by checking if it has an `endTimestamp`. + * + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + */ +export function addFetchInstrumentationHandler(handler: (data: HandlerDataFetch) => void): void { + const type = 'fetch'; + addHandler(type, handler); + maybeInstrument(type, instrumentFetch); +} + +function instrumentFetch(): void { + if (!supportsNativeFetch()) { + return; + } + + fill(GLOBAL_OBJ, 'fetch', function (originalFetch: () => void): () => void { + return function (...args: any[]): void { + const { method, url } = parseFetchArgs(args); + + const handlerData: HandlerDataFetch = { + args, + fetchData: { + method, + url, + }, + startTimestamp: Date.now(), + }; + + triggerHandlers('fetch', { + ...handlerData, + }); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + return originalFetch.apply(GLOBAL_OBJ, args).then( + (response: Response) => { + const finishedHandlerData: HandlerDataFetch = { + ...handlerData, + endTimestamp: Date.now(), + response, + }; + + triggerHandlers('fetch', finishedHandlerData); + return response; + }, + (error: Error) => { + const erroredHandlerData: HandlerDataFetch = { + ...handlerData, + endTimestamp: Date.now(), + error, + }; + + triggerHandlers('fetch', erroredHandlerData); + // NOTE: If you are a Sentry user, and you are seeing this stack frame, + // it means the sentry.javascript SDK caught an error invoking your application code. + // This is expected behavior and NOT indicative of a bug with sentry.javascript. + throw error; + }, + ); + }; + }); +} + +function hasProp(obj: unknown, prop: T): obj is Record { + return !!obj && typeof obj === 'object' && !!(obj as Record)[prop]; +} + +function getUrlFromResource(resource: FetchResource): string { + if (typeof resource === 'string') { + return resource; + } + + if (!resource) { + return ''; + } + + if (hasProp(resource, 'url')) { + return resource.url; + } + + if (resource.toString) { + return resource.toString(); + } + + return ''; +} + +/** + * Parses the fetch arguments to find the used Http method and the url of the request. + * Exported for tests only. + */ +export function parseFetchArgs(fetchArgs: unknown[]): { method: string; url: string } { + if (fetchArgs.length === 0) { + return { method: 'GET', url: '' }; + } + + if (fetchArgs.length === 2) { + const [url, options] = fetchArgs as [FetchResource, object]; + + return { + url: getUrlFromResource(url), + method: hasProp(options, 'method') ? String(options.method).toUpperCase() : 'GET', + }; + } + + const arg = fetchArgs[0]; + return { + url: getUrlFromResource(arg as FetchResource), + method: hasProp(arg, 'method') ? String(arg.method).toUpperCase() : 'GET', + }; +} diff --git a/packages/utils/src/instrument/globalError.ts b/packages/utils/src/instrument/globalError.ts new file mode 100644 index 000000000000..df7ff21438cc --- /dev/null +++ b/packages/utils/src/instrument/globalError.ts @@ -0,0 +1,48 @@ +import type { HandlerDataError } from '@sentry/types'; + +import { GLOBAL_OBJ } from '../worldwide'; +import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; + +let _oldOnErrorHandler: (typeof GLOBAL_OBJ)['onerror'] | null = null; + +/** + * Add an instrumentation handler for when an error is captured by the global error handler. + * + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + */ +export function addGlobalErrorInstrumentationHandler(handler: (data: HandlerDataError) => void): void { + const type = 'error'; + addHandler(type, handler); + maybeInstrument(type, instrumentError); +} + +function instrumentError(): void { + _oldOnErrorHandler = GLOBAL_OBJ.onerror; + + GLOBAL_OBJ.onerror = function ( + msg: string | Event, + url?: string, + line?: number, + column?: number, + error?: Error, + ): boolean { + const handlerData: HandlerDataError = { + column, + error, + line, + msg, + url, + }; + triggerHandlers('error', handlerData); + + if (_oldOnErrorHandler && !_oldOnErrorHandler.__SENTRY_LOADER__) { + // eslint-disable-next-line prefer-rest-params + return _oldOnErrorHandler.apply(this, arguments); + } + + return false; + }; + + GLOBAL_OBJ.onerror.__SENTRY_INSTRUMENTED__ = true; +} diff --git a/packages/utils/src/instrument/globalUnhandledRejection.ts b/packages/utils/src/instrument/globalUnhandledRejection.ts new file mode 100644 index 000000000000..c92f4d723b0d --- /dev/null +++ b/packages/utils/src/instrument/globalUnhandledRejection.ts @@ -0,0 +1,40 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ + +import type { HandlerDataUnhandledRejection } from '@sentry/types'; + +import { GLOBAL_OBJ } from '../worldwide'; +import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; + +let _oldOnUnhandledRejectionHandler: (typeof GLOBAL_OBJ)['onunhandledrejection'] | null = null; + +/** + * Add an instrumentation handler for when an unhandled promise rejection is captured. + * + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + */ +export function addGlobalUnhandledRejectionInstrumentationHandler( + handler: (data: HandlerDataUnhandledRejection) => void, +): void { + const type = 'unhandledrejection'; + addHandler(type, handler); + maybeInstrument(type, instrumentUnhandledRejection); +} + +function instrumentUnhandledRejection(): void { + _oldOnUnhandledRejectionHandler = GLOBAL_OBJ.onunhandledrejection; + + GLOBAL_OBJ.onunhandledrejection = function (e: any): boolean { + const handlerData: HandlerDataUnhandledRejection = e; + triggerHandlers('unhandledrejection', handlerData); + + if (_oldOnUnhandledRejectionHandler && !_oldOnUnhandledRejectionHandler.__SENTRY_LOADER__) { + // eslint-disable-next-line prefer-rest-params + return _oldOnUnhandledRejectionHandler.apply(this, arguments); + } + + return true; + }; + + GLOBAL_OBJ.onunhandledrejection.__SENTRY_INSTRUMENTED__ = true; +} diff --git a/packages/utils/src/instrument/history.ts b/packages/utils/src/instrument/history.ts new file mode 100644 index 000000000000..b13eabf99e3b --- /dev/null +++ b/packages/utils/src/instrument/history.ts @@ -0,0 +1,71 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/ban-types */ +import type { HandlerDataHistory } from '@sentry/types'; + +import { fill } from '../object'; +import { supportsHistory } from '../supports'; +import { GLOBAL_OBJ } from '../worldwide'; +import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; + +const WINDOW = GLOBAL_OBJ as unknown as Window; + +let lastHref: string | undefined; + +/** + * Add an instrumentation handler for when a fetch request happens. + * The handler function is called once when the request starts and once when it ends, + * which can be identified by checking if it has an `endTimestamp`. + * + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + */ +export function addHistoryInstrumentationHandler(handler: (data: HandlerDataHistory) => void): void { + const type = 'history'; + addHandler(type, handler); + maybeInstrument(type, instrumentHistory); +} + +function instrumentHistory(): void { + if (!supportsHistory()) { + return; + } + + const oldOnPopState = WINDOW.onpopstate; + WINDOW.onpopstate = function (this: WindowEventHandlers, ...args: any[]): any { + const to = WINDOW.location.href; + // keep track of the current URL state, as we always receive only the updated state + const from = lastHref; + lastHref = to; + const handlerData: HandlerDataHistory = { from, to }; + triggerHandlers('history', handlerData); + if (oldOnPopState) { + // Apparently this can throw in Firefox when incorrectly implemented plugin is installed. + // https://github.com/getsentry/sentry-javascript/issues/3344 + // https://github.com/bugsnag/bugsnag-js/issues/469 + try { + return oldOnPopState.apply(this, args); + } catch (_oO) { + // no-empty + } + } + }; + + function historyReplacementFunction(originalHistoryFunction: () => void): () => void { + return function (this: History, ...args: any[]): void { + const url = args.length > 2 ? args[2] : undefined; + if (url) { + // coerce to string (this is what pushState does) + const from = lastHref; + const to = String(url); + // keep track of the current URL state, as we always receive only the updated state + lastHref = to; + const handlerData: HandlerDataHistory = { from, to }; + triggerHandlers('history', handlerData); + } + return originalHistoryFunction.apply(this, args); + }; + } + + fill(WINDOW.history, 'pushState', historyReplacementFunction); + fill(WINDOW.history, 'replaceState', historyReplacementFunction); +} diff --git a/packages/utils/src/instrument/index.ts b/packages/utils/src/instrument/index.ts new file mode 100644 index 000000000000..800f2ec6540d --- /dev/null +++ b/packages/utils/src/instrument/index.ts @@ -0,0 +1,67 @@ +import type { + InstrumentHandlerCallback as _InstrumentHandlerCallback, + InstrumentHandlerType as _InstrumentHandlerType, +} from './_handlers'; +import { resetInstrumentationHandlers } from './_handlers'; +import { logger } from './../logger'; +import { addConsoleInstrumentationHandler } from './console'; +import { addClickKeypressInstrumentationHandler } from './dom'; +import { addFetchInstrumentationHandler } from './fetch'; +import { addGlobalErrorInstrumentationHandler } from './globalError'; +import { addGlobalUnhandledRejectionInstrumentationHandler } from './globalUnhandledRejection'; +import { addHistoryInstrumentationHandler } from './history'; +import { addXhrInstrumentationHandler, SENTRY_XHR_DATA_KEY } from './xhr'; + +/** + * Add handler that will be called when given type of instrumentation triggers. + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + * @deprecated Use the proper function per instrumentation type instead! + */ +export function addInstrumentationHandler(type: _InstrumentHandlerType, callback: _InstrumentHandlerCallback): void { + switch (type) { + case 'console': + return addConsoleInstrumentationHandler(callback); + case 'dom': + return addClickKeypressInstrumentationHandler(callback); + case 'xhr': + return addXhrInstrumentationHandler(callback); + case 'fetch': + return addFetchInstrumentationHandler(callback); + case 'history': + return addHistoryInstrumentationHandler(callback); + case 'error': + return addGlobalErrorInstrumentationHandler(callback); + case 'unhandledrejection': + return addGlobalUnhandledRejectionInstrumentationHandler(callback); + default: + __DEBUG_BUILD__ && logger.warn('unknown instrumentation type:', type); + } +} + +/** + * @deprecated Use the specific handler data types from @sentry/types instead, e.g. HandlerDataFetch, HandlerDataConsole, ... + */ +type InstrumentHandlerCallback = _InstrumentHandlerCallback; + +/** + * @deprecated Use the specific handler functions instead, e.g. addConsoleInstrumentationHandler, ... + */ +type InstrumentHandlerType = _InstrumentHandlerType; + +// eslint-disable-next-line deprecation/deprecation +export type { InstrumentHandlerCallback, InstrumentHandlerType }; + +export { + addConsoleInstrumentationHandler, + addClickKeypressInstrumentationHandler, + addXhrInstrumentationHandler, + addFetchInstrumentationHandler, + addHistoryInstrumentationHandler, + addGlobalErrorInstrumentationHandler, + addGlobalUnhandledRejectionInstrumentationHandler, + SENTRY_XHR_DATA_KEY, + + // Only exported for tests + resetInstrumentationHandlers, +}; diff --git a/packages/utils/src/instrument/xhr.ts b/packages/utils/src/instrument/xhr.ts new file mode 100644 index 000000000000..d0245dcdd2ee --- /dev/null +++ b/packages/utils/src/instrument/xhr.ts @@ -0,0 +1,158 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/ban-types */ +import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, WrappedFunction } from '@sentry/types'; + +import { isString } from '../is'; +import { fill } from '../object'; +import { GLOBAL_OBJ } from '../worldwide'; +import { addHandler, maybeInstrument, triggerHandlers } from './_handlers'; + +const WINDOW = GLOBAL_OBJ as unknown as Window; + +export const SENTRY_XHR_DATA_KEY = '__sentry_xhr_v3__'; + +/** + * Add an instrumentation handler for when an XHR request happens. + * The handler function is called once when the request starts and once when it ends, + * which can be identified by checking if it has an `endTimestamp`. + * + * Use at your own risk, this might break without changelog notice, only used internally. + * @hidden + */ +export function addXhrInstrumentationHandler(handler: (data: HandlerDataXhr) => void): void { + const type = 'xhr'; + addHandler(type, handler); + maybeInstrument(type, instrumentXHR); +} + +/** Exported only for tests. */ +export function instrumentXHR(): void { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (!(WINDOW as any).XMLHttpRequest) { + return; + } + + const xhrproto = XMLHttpRequest.prototype; + + fill(xhrproto, 'open', function (originalOpen: () => void): () => void { + return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: any[]): void { + const startTimestamp = Date.now(); + + // open() should always be called with two or more arguments + // But to be on the safe side, we actually validate this and bail out if we don't have a method & url + const method = isString(args[0]) ? args[0].toUpperCase() : undefined; + const url = parseUrl(args[1]); + + if (!method || !url) { + return; + } + + this[SENTRY_XHR_DATA_KEY] = { + method, + url, + request_headers: {}, + }; + + // if Sentry key appears in URL, don't capture it as a request + if (method === 'POST' && url.match(/sentry_key/)) { + this.__sentry_own_request__ = true; + } + + const onreadystatechangeHandler: () => void = () => { + // For whatever reason, this is not the same instance here as from the outer method + const xhrInfo = this[SENTRY_XHR_DATA_KEY]; + + if (!xhrInfo) { + return; + } + + if (this.readyState === 4) { + try { + // touching statusCode in some platforms throws + // an exception + xhrInfo.status_code = this.status; + } catch (e) { + /* do nothing */ + } + + const handlerData: HandlerDataXhr = { + args: [method, url], + endTimestamp: Date.now(), + startTimestamp, + xhr: this, + }; + triggerHandlers('xhr', handlerData); + } + }; + + if ('onreadystatechange' in this && typeof this.onreadystatechange === 'function') { + fill(this, 'onreadystatechange', function (original: WrappedFunction): Function { + return function (this: SentryWrappedXMLHttpRequest, ...readyStateArgs: any[]): void { + onreadystatechangeHandler(); + return original.apply(this, readyStateArgs); + }; + }); + } else { + this.addEventListener('readystatechange', onreadystatechangeHandler); + } + + // Intercepting `setRequestHeader` to access the request headers of XHR instance. + // This will only work for user/library defined headers, not for the default/browser-assigned headers. + // Request cookies are also unavailable for XHR, as `Cookie` header can't be defined by `setRequestHeader`. + fill(this, 'setRequestHeader', function (original: WrappedFunction): Function { + return function (this: SentryWrappedXMLHttpRequest, ...setRequestHeaderArgs: unknown[]): void { + const [header, value] = setRequestHeaderArgs; + + const xhrInfo = this[SENTRY_XHR_DATA_KEY]; + + if (xhrInfo && isString(header) && isString(value)) { + xhrInfo.request_headers[header.toLowerCase()] = value; + } + + return original.apply(this, setRequestHeaderArgs); + }; + }); + + return originalOpen.apply(this, args); + }; + }); + + fill(xhrproto, 'send', function (originalSend: () => void): () => void { + return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: any[]): void { + const sentryXhrData = this[SENTRY_XHR_DATA_KEY]; + + if (!sentryXhrData) { + return; + } + + if (args[0] !== undefined) { + sentryXhrData.body = args[0]; + } + + const handlerData: HandlerDataXhr = { + args: [sentryXhrData.method, sentryXhrData.url], + startTimestamp: Date.now(), + xhr: this, + }; + triggerHandlers('xhr', handlerData); + + return originalSend.apply(this, args); + }; + }); +} + +function parseUrl(url: string | unknown): string | undefined { + if (isString(url)) { + return url; + } + + try { + // url can be a string or URL + // but since URL is not available in IE11, we do not check for it, + // but simply assume it is an URL and return `toString()` from it (which returns the full URL) + // If that fails, we just return undefined + return (url as URL).toString(); + } catch {} // eslint-disable-line no-empty + + return undefined; +} diff --git a/packages/utils/src/logger.ts b/packages/utils/src/logger.ts index 9d37855e9114..1784aa179bb1 100644 --- a/packages/utils/src/logger.ts +++ b/packages/utils/src/logger.ts @@ -1,10 +1,19 @@ +import type { ConsoleLevel } from '@sentry/types'; + import { GLOBAL_OBJ } from './worldwide'; /** Prefix for logging strings */ const PREFIX = 'Sentry Logger '; -export const CONSOLE_LEVELS = ['debug', 'info', 'warn', 'error', 'log', 'assert', 'trace'] as const; -export type ConsoleLevel = (typeof CONSOLE_LEVELS)[number]; +export const CONSOLE_LEVELS: readonly ConsoleLevel[] = [ + 'debug', + 'info', + 'warn', + 'error', + 'log', + 'assert', + 'trace', +] as const; type LoggerMethod = (...args: unknown[]) => void; type LoggerConsoleMethods = Record; diff --git a/packages/utils/src/worldwide.ts b/packages/utils/src/worldwide.ts index 12e098bf3bc7..02f4d9714372 100644 --- a/packages/utils/src/worldwide.ts +++ b/packages/utils/src/worldwide.ts @@ -24,12 +24,12 @@ export interface InternalGlobal { Integrations?: Integration[]; }; onerror?: { - (msg: unknown, url: unknown, line: unknown, column: unknown, error: unknown): boolean; + (event: Event | string, source?: string, lineno?: number, colno?: number, error?: Error): any; __SENTRY_INSTRUMENTED__?: true; __SENTRY_LOADER__?: true; }; onunhandledrejection?: { - (event: unknown): boolean; + (event: PromiseRejectionEvent): boolean; __SENTRY_INSTRUMENTED__?: true; __SENTRY_LOADER__?: true; }; diff --git a/packages/utils/test/instrument.test.ts b/packages/utils/test/instrument.test.ts deleted file mode 100644 index f1ec46b93fb1..000000000000 --- a/packages/utils/test/instrument.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { instrumentDOM, instrumentXHR, parseFetchArgs } from '../src/instrument'; - -jest.mock('../src/worldwide', () => ({ - // Return an empty object with undefined properties - getGlobalObject: () => ({ - document: undefined, - XMLHttpRequest: undefined, - }), -})); - -describe('instrument', () => { - it('instrumentXHR() does not throw if XMLHttpRequest is a key on window but not defined', () => { - expect(instrumentXHR).not.toThrow(); - }); - - it('instrumentDOM() does not throw if XMLHttpRequest is a key on window but not defined', () => { - expect(instrumentDOM).not.toThrow(); - }); - - describe('parseFetchArgs', () => { - it.each([ - ['string URL only', ['http://example.com'], { method: 'GET', url: 'http://example.com' }], - ['URL object only', [new URL('http://example.com')], { method: 'GET', url: 'http://example.com/' }], - ['Request URL only', [{ url: 'http://example.com' }], { method: 'GET', url: 'http://example.com' }], - [ - 'Request URL & method only', - [{ url: 'http://example.com', method: 'post' }], - { method: 'POST', url: 'http://example.com' }, - ], - [ - 'string URL & options', - ['http://example.com', { method: 'post' }], - { method: 'POST', url: 'http://example.com' }, - ], - [ - 'URL object & options', - [new URL('http://example.com'), { method: 'post' }], - { method: 'POST', url: 'http://example.com/' }, - ], - [ - 'Request URL & options', - [{ url: 'http://example.com' }, { method: 'post' }], - { method: 'POST', url: 'http://example.com' }, - ], - ])('%s', (_name, args, expected) => { - const actual = parseFetchArgs(args as unknown[]); - - expect(actual).toEqual(expected); - }); - }); -}); diff --git a/packages/utils/test/instrument/dom.test.ts b/packages/utils/test/instrument/dom.test.ts new file mode 100644 index 000000000000..28745109e0f8 --- /dev/null +++ b/packages/utils/test/instrument/dom.test.ts @@ -0,0 +1,18 @@ +import { instrumentDOM } from '../../src/instrument/dom'; + +jest.mock('../../src/worldwide', () => { + const original = jest.requireActual('../../src/worldwide'); + + return { + ...original, + GLOBAL_OBJ: { + document: undefined, + }, + }; +}); + +describe('instrumentDOM', () => { + it('it does not throw if document is a key on window but not defined', () => { + expect(instrumentDOM).not.toThrow(); + }); +}); diff --git a/packages/utils/test/instrument/fetch.test.ts b/packages/utils/test/instrument/fetch.test.ts new file mode 100644 index 000000000000..ea29e0c16c3e --- /dev/null +++ b/packages/utils/test/instrument/fetch.test.ts @@ -0,0 +1,29 @@ +import { parseFetchArgs } from '../../src/instrument/fetch'; + +describe('instrument > parseFetchArgs', () => { + it.each([ + ['string URL only', ['http://example.com'], { method: 'GET', url: 'http://example.com' }], + ['URL object only', [new URL('http://example.com')], { method: 'GET', url: 'http://example.com/' }], + ['Request URL only', [{ url: 'http://example.com' }], { method: 'GET', url: 'http://example.com' }], + [ + 'Request URL & method only', + [{ url: 'http://example.com', method: 'post' }], + { method: 'POST', url: 'http://example.com' }, + ], + ['string URL & options', ['http://example.com', { method: 'post' }], { method: 'POST', url: 'http://example.com' }], + [ + 'URL object & options', + [new URL('http://example.com'), { method: 'post' }], + { method: 'POST', url: 'http://example.com/' }, + ], + [ + 'Request URL & options', + [{ url: 'http://example.com' }, { method: 'post' }], + { method: 'POST', url: 'http://example.com' }, + ], + ])('%s', (_name, args, expected) => { + const actual = parseFetchArgs(args as unknown[]); + + expect(actual).toEqual(expected); + }); +}); diff --git a/packages/utils/test/instrument/xhr.test.ts b/packages/utils/test/instrument/xhr.test.ts new file mode 100644 index 000000000000..60485ff1d398 --- /dev/null +++ b/packages/utils/test/instrument/xhr.test.ts @@ -0,0 +1,18 @@ +import { instrumentXHR } from '../../src/instrument/xhr'; + +jest.mock('../../src/worldwide', () => { + const original = jest.requireActual('../../src/worldwide'); + + return { + ...original, + GLOBAL_OBJ: { + XMLHttpRequest: undefined, + }, + }; +}); + +describe('instrumentXHR', () => { + it('it does not throw if XMLHttpRequest is a key on window but not defined', () => { + expect(instrumentXHR).not.toThrow(); + }); +}); From d53401bebca24c5d7b13dbcdeceeed8164b0b830 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 13 Nov 2023 13:36:14 +0100 Subject: [PATCH 2/9] fix unhandled rejection for null --- .../src/integrations/globalhandlers.ts | 63 ++++++++++--------- packages/types/src/instrument.ts | 5 +- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index 0da5d934cb6d..87b47169232d 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -1,14 +1,6 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ import { getCurrentHub } from '@sentry/core'; -import type { - Event, - EventHint, - HandlerDataUnhandledRejection, - Hub, - Integration, - Primitive, - StackParser, -} from '@sentry/types'; +import type { Event, EventHint, Hub, Integration, Primitive, StackParser } from '@sentry/types'; import { addExceptionMechanism, addGlobalErrorInstrumentationHandler, @@ -85,7 +77,6 @@ export class GlobalHandlers implements Integration { } } -/** JSDoc */ function _installGlobalOnErrorHandler(): void { addGlobalErrorInstrumentationHandler(data => { const [hub, stackParser, attachStacktrace] = getHubAndOptions(); @@ -113,38 +104,19 @@ function _installGlobalOnErrorHandler(): void { }); } -/** JSDoc */ function _installGlobalOnUnhandledRejectionHandler(): void { addGlobalUnhandledRejectionInstrumentationHandler(e => { const [hub, stackParser, attachStacktrace] = getHubAndOptions(); if (!hub.getIntegration(GlobalHandlers)) { return; } - let error: Error | HandlerDataUnhandledRejection = e; - - // dig the object of the rejection out of known event types - try { - // PromiseRejectionEvents store the object of the rejection under 'reason' - // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent - if ('reason' in e && e.reason) { - error = e.reason; - } - // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents - // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into - // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec - // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and - // https://github.com/getsentry/sentry-javascript/issues/2380 - else if ('detail' in e && e.detail && 'reason' in e.detail && e.detail.reason) { - error = e.detail.reason; - } - } catch (_oO) { - // no-empty - } if (shouldIgnoreOnError()) { return true; } + const error = _getUnhandledRejectionError(e as unknown); + const event = isPrimitive(error) ? _eventFromRejectionWithPrimitive(error) : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, true); @@ -156,6 +128,35 @@ function _installGlobalOnUnhandledRejectionHandler(): void { }); } +function _getUnhandledRejectionError(error: unknown): unknown { + if (isPrimitive(error)) { + return error; + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const e = error as any; + + // dig the object of the rejection out of known event types + try { + // PromiseRejectionEvents store the object of the rejection under 'reason' + // see https://developer.mozilla.org/en-US/docs/Web/API/PromiseRejectionEvent + if ('reason' in e) { + return e.reason; + } + + // something, somewhere, (likely a browser extension) effectively casts PromiseRejectionEvents + // to CustomEvents, moving the `promise` and `reason` attributes of the PRE into + // the CustomEvent's `detail` attribute, since they're not part of CustomEvent's spec + // see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent and + // https://github.com/getsentry/sentry-javascript/issues/2380 + else if ('detail' in e && 'reason' in e.detail) { + return e.detail.reason; + } + } catch {} // eslint-disable-line no-empty + + return error; +} + /** * Create an event from a promise rejection where the `reason` is a primitive. * diff --git a/packages/types/src/instrument.ts b/packages/types/src/instrument.ts index 70215f1b968c..9ad5964e3455 100644 --- a/packages/types/src/instrument.ts +++ b/packages/types/src/instrument.ts @@ -85,7 +85,4 @@ export interface HandlerDataError { url?: string; } -export interface HandlerDataUnhandledRejection { - reason?: Error; - detail?: { reason?: Error }; -} +export type HandlerDataUnhandledRejection = unknown; From 10719825a60c0e40952f3650ab495f369e3b7da2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 13 Nov 2023 15:32:31 +0100 Subject: [PATCH 3/9] avoid test --- packages/utils/src/worldwide.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/src/worldwide.ts b/packages/utils/src/worldwide.ts index 02f4d9714372..5caf1b137d5a 100644 --- a/packages/utils/src/worldwide.ts +++ b/packages/utils/src/worldwide.ts @@ -29,7 +29,7 @@ export interface InternalGlobal { __SENTRY_LOADER__?: true; }; onunhandledrejection?: { - (event: PromiseRejectionEvent): boolean; + (event: unknown): boolean; __SENTRY_INSTRUMENTED__?: true; __SENTRY_LOADER__?: true; }; From 6b2f32885e24d1a79ec39a81dc155367fc9a7000 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 21 Nov 2023 16:07:26 +0100 Subject: [PATCH 4/9] fixes --- packages/tracing-internal/src/browser/request.ts | 11 ++--------- .../tracing-internal/test/browser/request.test.ts | 7 +++---- packages/types/src/instrument.ts | 4 ++++ 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 2584eb736211..50f0742c3687 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -15,13 +15,6 @@ import { import { instrumentFetchRequest } from '../common/fetch'; import { addPerformanceInstrumentationHandler } from './instrument'; -/** Exported only for tests. */ -export type ExtendedSentryWrappedXMLHttpRequest = SentryWrappedXMLHttpRequest & { - __sentry_xhr_span_id__?: string; - setRequestHeader?: (key: string, val: string) => void; - getRequestHeader?: (key: string) => string; -}; - export const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\/(?!\/)/]; /** Options for Request Instrumentation */ @@ -263,7 +256,7 @@ export function xhrCallback( shouldAttachHeaders: (url: string) => boolean, spans: Record, ): Span | undefined { - const xhr = handlerData.xhr as ExtendedSentryWrappedXMLHttpRequest; + const xhr = handlerData.xhr; const sentryXhrData = xhr && xhr[SENTRY_XHR_DATA_KEY]; if (!hasTracingEnabled() || (xhr && xhr.__sentry_own_request__) || !xhr || !sentryXhrData) { @@ -332,7 +325,7 @@ export function xhrCallback( } function setHeaderOnXhr( - xhr: ExtendedSentryWrappedXMLHttpRequest, + xhr: SentryWrappedXMLHttpRequest, sentryTraceHeader: string, sentryBaggageHeader: string | undefined, ): void { diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 3aa629cc4e7b..1795286624f4 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -1,13 +1,12 @@ /* eslint-disable deprecation/deprecation */ import * as sentryCore from '@sentry/core'; -import type { HandlerDataFetch } from '@sentry/types'; +import type { HandlerDataFetch, SentryWrappedXMLHttpRequest } from '@sentry/types'; import * as utils from '@sentry/utils'; import { SENTRY_XHR_DATA_KEY } from '@sentry/utils'; import type { Transaction } from '../../../tracing/src'; import { addExtensionMethods, Span, spanStatusfromHttpCode } from '../../../tracing/src'; import { getDefaultBrowserClientOptions } from '../../../tracing/test/testutils'; -import type { ExtendedSentryWrappedXMLHttpRequest } from '../../src/browser/request'; import { extractNetworkProtocol, instrumentOutgoingRequests, @@ -296,7 +295,7 @@ describe('callbacks', () => { }, __sentry_xhr_span_id__: '1231201211212012', setRequestHeader, - } as ExtendedSentryWrappedXMLHttpRequest, + } as SentryWrappedXMLHttpRequest, startTimestamp, }; }); @@ -352,7 +351,7 @@ describe('callbacks', () => { }); expect(newSpan.description).toBe('GET http://dogs.are.great/'); expect(newSpan.op).toBe('http.client'); - const spanId = (xhrHandlerData.xhr as ExtendedSentryWrappedXMLHttpRequest | undefined)?.__sentry_xhr_span_id__; + const spanId = xhrHandlerData.xhr?.__sentry_xhr_span_id__; expect(spanId).toBeDefined(); expect(spanId).toEqual(newSpan?.spanId); diff --git a/packages/types/src/instrument.ts b/packages/types/src/instrument.ts index 9ad5964e3455..7ec67c1bf4a6 100644 --- a/packages/types/src/instrument.ts +++ b/packages/types/src/instrument.ts @@ -8,6 +8,10 @@ export type ConsoleLevel = 'debug' | 'info' | 'warn' | 'error' | 'log' | 'assert export interface SentryWrappedXMLHttpRequest { __sentry_xhr_v3__?: SentryXhrData; __sentry_own_request__?: boolean; + // span id for the xhr request + __sentry_xhr_span_id__?: string; + setRequestHeader?: (key: string, val: string) => void; + getRequestHeader?: (key: string) => string; } // WARNING: When the shape of this type is changed bump the version in `SentryWrappedXMLHttpRequest` From 3f07f3c84dbde55281cd57c6bb19cd5f3284a3a1 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Nov 2023 10:22:04 +0100 Subject: [PATCH 5/9] fixes --- .../tracing-internal/src/browser/request.ts | 19 +------------------ .../test/browser/request.test.ts | 2 +- packages/types/src/instrument.ts | 2 +- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 50f0742c3687..2f257c3eb76b 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core'; -import type { SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; +import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; import { addFetchInstrumentationHandler, addXhrInstrumentationHandler, @@ -67,23 +67,6 @@ export interface RequestInstrumentationOptions { shouldCreateSpanForRequest?(this: void, url: string): boolean; } -/** Data returned from XHR request */ -export interface XHRData { - xhr?: { - [SENTRY_XHR_DATA_KEY]?: { - method: string; - url: string; - status_code: number; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - data: Record; - }; - __sentry_xhr_span_id__?: string; - setRequestHeader?: (key: string, val: string) => void; - getRequestHeader?: (key: string) => string; - __sentry_own_request__?: boolean; - }; -} - export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = { traceFetch: true, traceXHR: true, diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 1795286624f4..54a33396df39 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -1,6 +1,6 @@ /* eslint-disable deprecation/deprecation */ import * as sentryCore from '@sentry/core'; -import type { HandlerDataFetch, SentryWrappedXMLHttpRequest } from '@sentry/types'; +import type { HandlerDataFetch, HandlerDataXhr, SentryWrappedXMLHttpRequest } from '@sentry/types'; import * as utils from '@sentry/utils'; import { SENTRY_XHR_DATA_KEY } from '@sentry/utils'; diff --git a/packages/types/src/instrument.ts b/packages/types/src/instrument.ts index 7ec67c1bf4a6..e51f9a3eec44 100644 --- a/packages/types/src/instrument.ts +++ b/packages/types/src/instrument.ts @@ -11,7 +11,7 @@ export interface SentryWrappedXMLHttpRequest { // span id for the xhr request __sentry_xhr_span_id__?: string; setRequestHeader?: (key: string, val: string) => void; - getRequestHeader?: (key: string) => string; + getResponseHeader?: (key: string) => string | null; } // WARNING: When the shape of this type is changed bump the version in `SentryWrappedXMLHttpRequest` From 279671ca2205fa14e8467ad5771b3bef88fc816f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Nov 2023 10:33:53 +0100 Subject: [PATCH 6/9] fix test --- .../suites/onunhandledrejection.js | 20 ------------------- .../tracing-internal/src/browser/request.ts | 2 +- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/packages/browser/test/integration/suites/onunhandledrejection.js b/packages/browser/test/integration/suites/onunhandledrejection.js index f9095d7c7333..8099fbbaa68e 100644 --- a/packages/browser/test/integration/suites/onunhandledrejection.js +++ b/packages/browser/test/integration/suites/onunhandledrejection.js @@ -243,24 +243,4 @@ describe('window.onunhandledrejection', function () { } }); }); - - it('should skip our own failed requests that somehow bubbled-up to unhandledrejection handler', function () { - return runInSandbox(sandbox, function () { - if (supportsOnunhandledRejection()) { - Promise.reject({ - __sentry_own_request__: true, - }); - Promise.reject({ - __sentry_own_request__: false, - }); - Promise.reject({}); - } else { - window.resolveTest({ window: window }); - } - }).then(function (summary) { - if (summary.window.supportsOnunhandledRejection()) { - assert.equal(summary.events.length, 2); - } - }); - }); }); diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index 2f257c3eb76b..78e334e98549 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -242,7 +242,7 @@ export function xhrCallback( const xhr = handlerData.xhr; const sentryXhrData = xhr && xhr[SENTRY_XHR_DATA_KEY]; - if (!hasTracingEnabled() || (xhr && xhr.__sentry_own_request__) || !xhr || !sentryXhrData) { + if (!hasTracingEnabled() || !xhr || xhr.__sentry_own_request__ || !sentryXhrData) { return undefined; } From 8f368db83caaf37990e71b226adc455863c99655 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Nov 2023 14:18:02 +0100 Subject: [PATCH 7/9] fix --- packages/vercel-edge/src/integrations/wintercg-fetch.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vercel-edge/src/integrations/wintercg-fetch.ts b/packages/vercel-edge/src/integrations/wintercg-fetch.ts index 3ecffad83b33..4107e617af28 100644 --- a/packages/vercel-edge/src/integrations/wintercg-fetch.ts +++ b/packages/vercel-edge/src/integrations/wintercg-fetch.ts @@ -1,7 +1,7 @@ import { instrumentFetchRequest } from '@sentry-internal/tracing'; import { getCurrentHub, isSentryRequestUrl } from '@sentry/core'; import type { FetchBreadcrumbData, FetchBreadcrumbHint, HandlerDataFetch, Integration, Span } from '@sentry/types'; -import { addInstrumentationHandler, LRUMap, stringMatchesSomePattern } from '@sentry/utils'; +import { addFetchInstrumentationHandler, LRUMap, stringMatchesSomePattern } from '@sentry/utils'; export interface Options { /** @@ -48,7 +48,7 @@ export class WinterCGFetch implements Integration { public setupOnce(): void { const spans: Record = {}; - addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { + addFetchInstrumentationHandler(handlerData => { const hub = getCurrentHub(); if (!hub.getIntegration(WinterCGFetch)) { return; From be2f51ed4a0abf6133a3db0446d58dda67a7d9a9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Nov 2023 15:51:36 +0100 Subject: [PATCH 8/9] fix --- .../src/integrations/globalhandlers.ts | 20 +++++++++--- .../vercel-edge/test/wintercg-fetch.test.ts | 32 +++++++------------ 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index 87b47169232d..d913cffefd18 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -1,8 +1,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ import { getCurrentHub } from '@sentry/core'; -import type { Event, EventHint, Hub, Integration, Primitive, StackParser } from '@sentry/types'; +import type { Event, Hub, Integration, Primitive, StackParser } from '@sentry/types'; import { - addExceptionMechanism, addGlobalErrorInstrumentationHandler, addGlobalUnhandledRejectionInstrumentationHandler, getLocationHref, @@ -100,7 +99,13 @@ function _installGlobalOnErrorHandler(): void { event.level = 'error'; - addMechanismAndCapture(hub, error, event, 'onerror'); + hub.captureEvent(event, { + originalException: error, + mechanism: { + handled: false, + type: 'onerror', + }, + }); }); } @@ -123,7 +128,14 @@ function _installGlobalOnUnhandledRejectionHandler(): void { event.level = 'error'; - addMechanismAndCapture(hub, error, event, 'onunhandledrejection'); + hub.captureEvent(event, { + originalException: error, + mechanism: { + handled: false, + type: 'unhandledrejection', + }, + }); + return; }); } diff --git a/packages/vercel-edge/test/wintercg-fetch.test.ts b/packages/vercel-edge/test/wintercg-fetch.test.ts index 699fb4891257..22bd960defdf 100644 --- a/packages/vercel-edge/test/wintercg-fetch.test.ts +++ b/packages/vercel-edge/test/wintercg-fetch.test.ts @@ -30,7 +30,7 @@ const fakeHubInstance = new FakeHub( jest.spyOn(sentryCore, 'getCurrentHub').mockImplementation(() => fakeHubInstance); -const addInstrumentationHandlerSpy = jest.spyOn(sentryUtils, 'addInstrumentationHandler'); +const addFetchInstrumentationHandlerSpy = jest.spyOn(sentryUtils, 'addFetchInstrumentationHandler'); const instrumentFetchRequestSpy = jest.spyOn(internalTracing, 'instrumentFetchRequest'); const addBreadcrumbSpy = jest.spyOn(fakeHubInstance, 'addBreadcrumb'); @@ -41,13 +41,11 @@ beforeEach(() => { describe('WinterCGFetch instrumentation', () => { it('should call `instrumentFetchRequest` for outgoing fetch requests', () => { const integration = new WinterCGFetch(); - addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); integration.setupOnce(); - const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = - addInstrumentationHandlerSpy.mock.calls[0]; - expect(fetchInstrumentationHandlerType).toBe('fetch'); + const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); const startHandlerData: HandlerDataFetch = { @@ -76,13 +74,11 @@ describe('WinterCGFetch instrumentation', () => { it('should call `instrumentFetchRequest` for outgoing fetch requests to Sentry', () => { const integration = new WinterCGFetch(); - addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); integration.setupOnce(); - const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = - addInstrumentationHandlerSpy.mock.calls[0]; - expect(fetchInstrumentationHandlerType).toBe('fetch'); + const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); const startHandlerData: HandlerDataFetch = { @@ -101,13 +97,11 @@ describe('WinterCGFetch instrumentation', () => { return url === 'http://only-acceptable-url.com/'; }, }); - addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); integration.setupOnce(); - const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = - addInstrumentationHandlerSpy.mock.calls[0]; - expect(fetchInstrumentationHandlerType).toBe('fetch'); + const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); const startHandlerData: HandlerDataFetch = { @@ -126,13 +120,11 @@ describe('WinterCGFetch instrumentation', () => { it('should create a breadcrumb for an outgoing request', () => { const integration = new WinterCGFetch(); - addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); integration.setupOnce(); - const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = - addInstrumentationHandlerSpy.mock.calls[0]; - expect(fetchInstrumentationHandlerType).toBe('fetch'); + const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); const startTimestamp = Date.now(); @@ -166,13 +158,11 @@ describe('WinterCGFetch instrumentation', () => { const integration = new WinterCGFetch({ breadcrumbs: false, }); - addInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); + addFetchInstrumentationHandlerSpy.mockImplementationOnce(() => undefined); integration.setupOnce(); - const [fetchInstrumentationHandlerType, fetchInstrumentationHandlerCallback] = - addInstrumentationHandlerSpy.mock.calls[0]; - expect(fetchInstrumentationHandlerType).toBe('fetch'); + const [fetchInstrumentationHandlerCallback] = addFetchInstrumentationHandlerSpy.mock.calls[0]; expect(fetchInstrumentationHandlerCallback).toBeDefined(); const startTimestamp = Date.now(); From 7cc1856bf78d622476a76151b8df4f6732c09c59 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Nov 2023 16:38:35 +0100 Subject: [PATCH 9/9] fix for tests --- packages/browser/src/integrations/globalhandlers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index d913cffefd18..47acd2a75140 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -132,7 +132,7 @@ function _installGlobalOnUnhandledRejectionHandler(): void { originalException: error, mechanism: { handled: false, - type: 'unhandledrejection', + type: 'onunhandledrejection', }, });