feat(utils): Refactor addInstrumentationHandler to dedicated methods

packages/browser/src/integrations/breadcrumbs.ts

@@ -1,15 +1,26 @@
-/* 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,
   XhrBreadcrumbData,
   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<string, unknown>;
-
 /** 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;
   }
 

packages/browser/src/integrations/globalhandlers.ts

@@ -1,7 +1,15 @@
 /* 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 {
+  addGlobalErrorInstrumentationHandler,
+  addGlobalUnhandledRejectionInstrumentationHandler,
+  getLocationHref,
+  isErrorEvent,
+  isPrimitive,
+  isString,
+  logger,
+} from '@sentry/utils';
 
 import type { BrowserClient } from '../client';
 import { eventFromUnknownInput } from '../eventbuilder';
@@ -68,96 +76,97 @@ 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';
+
+    hub.captureEvent(event, {
+      originalException: error,
+      mechanism: {
+        handled: false,
+        type: '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;
-      }
-      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
-      }
+  addGlobalUnhandledRejectionInstrumentationHandler(e => {
+    const [hub, stackParser, attachStacktrace] = getHubAndOptions();
+    if (!hub.getIntegration(GlobalHandlers)) {
+      return;
+    }
 
-      if (shouldIgnoreOnError() || (error && error.__sentry_own_request__)) {
-        return true;
-      }
+    if (shouldIgnoreOnError()) {
+      return true;
+    }
 
-      const event = isPrimitive(error)
-        ? _eventFromRejectionWithPrimitive(error)
-        : eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, true);
+    const error = _getUnhandledRejectionError(e as unknown);
 
-      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;
-    },
-  );
+    hub.captureEvent(event, {
+      originalException: error,
+      mechanism: {
+        handled: false,
+        type: 'onunhandledrejection',
+      },
+    });
+
+    return;
+  });
+}
+
+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;
 }
 
 /**

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());
     }
   });

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);
-      }
-    });
-  });
 });