ref(types): Avoid some any type casting around wrap code

dev-packages/browser-integration-tests/suites/public-api/instrumentation/eventListener/event-target/subject.js

@@ -0,0 +1,11 @@
+const btn = document.createElement('button');
+btn.id = 'btn';
+document.body.appendChild(btn);
+
+const functionListener = function () {
+  throw new Error('event_listener_error');
+};
+
+btn.addEventListener('click', functionListener);
+
+btn.click();

dev-packages/browser-integration-tests/suites/public-api/instrumentation/eventListener/event-target/test.ts

@@ -0,0 +1,29 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers';
+
+sentryTest('should capture target name in mechanism data', async ({ getLocalTestUrl, page }) => {
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+  expect(eventData.exception?.values?.[0]).toMatchObject({
+    type: 'Error',
+    value: 'event_listener_error',
+    mechanism: {
+      type: 'instrument',
+      handled: false,
+      data: {
+        function: 'addEventListener',
+        handler: 'functionListener',
+        target: 'EventTarget',
+      },
+    },
+    stacktrace: {
+      frames: expect.any(Array),
+    },
+  });
+});

packages/browser/src/helpers.ts

@@ -31,6 +31,21 @@ export function ignoreNextOnError(): void {
   });
 }
 
+// eslint-disable-next-line @typescript-eslint/ban-types
+type WrappableFunction = Function;
+
+export function wrap<T extends WrappableFunction>(
+  fn: T,
+  options?: {
+    mechanism?: Mechanism;
+  },
+): WrappedFunction<T>;
+export function wrap<NonFunction>(
+  fn: NonFunction,
+  options?: {
+    mechanism?: Mechanism;
+  },
+): NonFunction;
 /**
  * Instruments the given function and sends an event to Sentry every time the
  * function throws an exception.
@@ -40,29 +55,31 @@ export function ignoreNextOnError(): void {
  * @returns The wrapped function.
  * @hidden
  */
-export function wrap(
-  fn: WrappedFunction,
+export function wrap<T extends WrappableFunction, NonFunction>(
+  fn: T | NonFunction,
   options: {
     mechanism?: Mechanism;
   } = {},
-  before?: WrappedFunction,
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-): any {
+): NonFunction | WrappedFunction<T> {
   // for future readers what this does is wrap a function and then create
   // a bi-directional wrapping between them.
   //
   // example: wrapped = wrap(original);
   //  original.__sentry_wrapped__ -> wrapped
   //  wrapped.__sentry_original__ -> original
 
-  if (typeof fn !== 'function') {
+  function isFunction(fn: T | NonFunction): fn is T {
+    return typeof fn === 'function';
+  }
+
+  if (!isFunction(fn)) {
     return fn;
   }
 
   try {
     // if we're dealing with a function that was previously wrapped, return
     // the original wrapper.
-    const wrapper = fn.__sentry_wrapped__;
+    const wrapper = (fn as WrappedFunction<T>).__sentry_wrapped__;
     if (wrapper) {
       if (typeof wrapper === 'function') {
         return wrapper;
@@ -84,18 +101,12 @@ export function wrap(
     return fn;
   }
 
-  /* eslint-disable prefer-rest-params */
+  // Wrap the function itself
   // It is important that `sentryWrapped` is not an arrow function to preserve the context of `this`
-  const sentryWrapped: WrappedFunction = function (this: unknown): void {
-    const args = Array.prototype.slice.call(arguments);
-
+  const sentryWrapped = function (this: unknown, ...args: unknown[]): unknown {
     try {
-      if (before && typeof before === 'function') {
-        before.apply(this, arguments);
-      }
-
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
-      const wrappedArguments = args.map((arg: any) => wrap(arg, options));
+      // Also wrap arguments that are themselves functions
+      const wrappedArguments = args.map(arg => wrap(arg, options));
 
       // Attempt to invoke user-land function
       // NOTE: If you are a Sentry user, and you are seeing this stack frame, it
@@ -125,18 +136,19 @@ export function wrap(
 
       throw ex;
     }
-  };
-  /* eslint-enable prefer-rest-params */
+  } as unknown as WrappedFunction<T>;
 
-  // Accessing some objects may throw
-  // ref: https://github.com/getsentry/sentry-javascript/issues/1168
+  // Wrap the wrapped function in a proxy, to ensure any other properties of the original function remain available
   try {
     for (const property in fn) {
       if (Object.prototype.hasOwnProperty.call(fn, property)) {
-        sentryWrapped[property] = fn[property];
+        sentryWrapped[property as keyof T] = fn[property as keyof T];
       }
     }
-  } catch (_oO) {} // eslint-disable-line no-empty
+  } catch {
+    // Accessing some objects may throw
+    // ref: https://github.com/getsentry/sentry-javascript/issues/1168
+  }
 
   // Signal that this function has been wrapped/filled already
   // for both debugging and to prevent it to being wrapped/filled twice
@@ -146,16 +158,19 @@ export function wrap(
 
   // Restore original function name (not all browsers allow that)
   try {
-    const descriptor = Object.getOwnPropertyDescriptor(sentryWrapped, 'name') as PropertyDescriptor;
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    const descriptor = Object.getOwnPropertyDescriptor(sentryWrapped, 'name')!;
     if (descriptor.configurable) {
       Object.defineProperty(sentryWrapped, 'name', {
         get(): string {
           return fn.name;
         },
       });
     }
-    // eslint-disable-next-line no-empty
-  } catch (_oO) {}
+  } catch {
+    // This may throw if e.g. the descriptor does not exist, or a browser does not allow redefining `name`.
+    // to save some bytes we simply try-catch this
+  }
 
   return sentryWrapped;
 }

packages/browser/src/integrations/browserapierrors.ts

@@ -96,8 +96,7 @@ const _browserApiErrorsIntegration = ((options: Partial<BrowserApiErrorsOptions>
 export const browserApiErrorsIntegration = defineIntegration(_browserApiErrorsIntegration);
 
 function _wrapTimeFunction(original: () => void): () => number {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  return function (this: any, ...args: any[]): number {
+  return function (this: unknown, ...args: unknown[]): number {
     const originalCallback = args[0];
     args[0] = wrap(originalCallback, {
       mechanism: {
@@ -110,11 +109,8 @@ function _wrapTimeFunction(original: () => void): () => number {
   };
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function _wrapRAF(original: any): (callback: () => void) => any {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  return function (this: any, callback: () => void): () => void {
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+function _wrapRAF(original: () => void): (callback: () => void) => unknown {
+  return function (this: unknown, callback: () => void): () => void {
     return original.apply(this, [
       wrap(callback, {
         mechanism: {
@@ -131,16 +127,14 @@ function _wrapRAF(original: any): (callback: () => void) => any {
 }
 
 function _wrapXHR(originalSend: () => void): () => void {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  return function (this: XMLHttpRequest, ...args: any[]): void {
+  return function (this: XMLHttpRequest, ...args: unknown[]): void {
     // eslint-disable-next-line @typescript-eslint/no-this-alias
     const xhr = this;
     const xmlHttpRequestProps: XMLHttpRequestProp[] = ['onload', 'onerror', 'onprogress', 'onreadystatechange'];
 
     xmlHttpRequestProps.forEach(prop => {
       if (prop in xhr && typeof xhr[prop] === 'function') {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        fill(xhr, prop, function (original: WrappedFunction): () => any {
+        fill(xhr, prop, function (original) {
           const wrapOptions = {
             mechanism: {
               data: {
@@ -169,30 +163,21 @@ function _wrapXHR(originalSend: () => void): () => void {
 }
 
 function _wrapEventTarget(target: string): void {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  const globalObject = WINDOW as { [key: string]: any };
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-  const proto = globalObject[target] && globalObject[target].prototype;
+  const globalObject = WINDOW as unknown as Record<string, { prototype?: object }>;
+  const targetObj = globalObject[target];
+  const proto = targetObj && targetObj.prototype;
 
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, no-prototype-builtins
+  // eslint-disable-next-line no-prototype-builtins
   if (!proto || !proto.hasOwnProperty || !proto.hasOwnProperty('addEventListener')) {
     return;
   }
 
   fill(proto, 'addEventListener', function (original: VoidFunction,): (
-    eventName: string,
-    fn: EventListenerObject,
-    options?: boolean | AddEventListenerOptions,
-  ) => void {
-    return function (
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      this: any,
-      eventName: string,
-      fn: EventListenerObject,
-      options?: boolean | AddEventListenerOptions,
-    ): (eventName: string, fn: EventListenerObject, capture?: boolean, secure?: boolean) => void {
+    ...args: Parameters<typeof WINDOW.addEventListener>
+  ) => ReturnType<typeof WINDOW.addEventListener> {
+    return function (this: unknown, eventName, fn, options): VoidFunction {
       try {
-        if (typeof fn.handleEvent === 'function') {
+        if (isEventListenerObject(fn)) {
           // ESlint disable explanation:
           //  First, it is generally safe to call `wrap` with an unbound function. Furthermore, using `.bind()` would
           //  introduce a bug here, because bind returns a new function that doesn't have our
@@ -211,14 +196,13 @@ function _wrapEventTarget(target: string): void {
             },
           });
         }
-      } catch (err) {
+      } catch {
         // can sometimes get 'Permission denied to access property "handle Event'
       }
 
       return original.apply(this, [
         eventName,
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        wrap(fn as any as WrappedFunction, {
+        wrap(fn, {
           mechanism: {
             data: {
               function: 'addEventListener',
@@ -234,48 +218,41 @@ function _wrapEventTarget(target: string): void {
     };
   });
 
-  fill(
-    proto,
-    'removeEventListener',
-    function (
-      originalRemoveEventListener: () => void,
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    ): (this: any, eventName: string, fn: EventListenerObject, options?: boolean | EventListenerOptions) => () => void {
-      return function (
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        this: any,
-        eventName: string,
-        fn: EventListenerObject,
-        options?: boolean | EventListenerOptions,
-      ): () => void {
-        /**
-         * There are 2 possible scenarios here:
-         *
-         * 1. Someone passes a callback, which was attached prior to Sentry initialization, or by using unmodified
-         * method, eg. `document.addEventListener.call(el, name, handler). In this case, we treat this function
-         * as a pass-through, and call original `removeEventListener` with it.
-         *
-         * 2. Someone passes a callback, which was attached after Sentry was initialized, which means that it was using
-         * our wrapped version of `addEventListener`, which internally calls `wrap` helper.
-         * This helper "wraps" whole callback inside a try/catch statement, and attached appropriate metadata to it,
-         * in order for us to make a distinction between wrapped/non-wrapped functions possible.
-         * If a function was wrapped, it has additional property of `__sentry_wrapped__`, holding the handler.
-         *
-         * When someone adds a handler prior to initialization, and then do it again, but after,
-         * then we have to detach both of them. Otherwise, if we'd detach only wrapped one, it'd be impossible
-         * to get rid of the initial handler and it'd stick there forever.
-         */
-        const wrappedEventHandler = fn as unknown as WrappedFunction;
-        try {
-          const originalEventHandler = wrappedEventHandler && wrappedEventHandler.__sentry_wrapped__;
-          if (originalEventHandler) {
-            originalRemoveEventListener.call(this, eventName, originalEventHandler, options);
-          }
-        } catch (e) {
-          // ignore, accessing __sentry_wrapped__ will throw in some Selenium environments
+  fill(proto, 'removeEventListener', function (originalRemoveEventListener: VoidFunction,): (
+    this: unknown,
+    ...args: Parameters<typeof WINDOW.removeEventListener>
+  ) => ReturnType<typeof WINDOW.removeEventListener> {
+    return function (this: unknown, eventName, fn, options): VoidFunction {
+      /**
+       * There are 2 possible scenarios here:
+       *
+       * 1. Someone passes a callback, which was attached prior to Sentry initialization, or by using unmodified
+       * method, eg. `document.addEventListener.call(el, name, handler). In this case, we treat this function
+       * as a pass-through, and call original `removeEventListener` with it.
+       *
+       * 2. Someone passes a callback, which was attached after Sentry was initialized, which means that it was using
+       * our wrapped version of `addEventListener`, which internally calls `wrap` helper.
+       * This helper "wraps" whole callback inside a try/catch statement, and attached appropriate metadata to it,
+       * in order for us to make a distinction between wrapped/non-wrapped functions possible.
+       * If a function was wrapped, it has additional property of `__sentry_wrapped__`, holding the handler.
+       *
+       * When someone adds a handler prior to initialization, and then do it again, but after,
+       * then we have to detach both of them. Otherwise, if we'd detach only wrapped one, it'd be impossible
+       * to get rid of the initial handler and it'd stick there forever.
+       */
+      try {
+        const originalEventHandler = (fn as WrappedFunction).__sentry_wrapped__;
+        if (originalEventHandler) {
+          originalRemoveEventListener.call(this, eventName, originalEventHandler, options);
         }
-        return originalRemoveEventListener.call(this, eventName, wrappedEventHandler, options);
-      };
-    },
-  );
+      } catch (e) {
+        // ignore, accessing __sentry_wrapped__ will throw in some Selenium environments
+      }
+      return originalRemoveEventListener.call(this, eventName, fn, options);
+    };
+  });
+}
+
+function isEventListenerObject(obj: unknown): obj is EventListenerObject {
+  return typeof (obj as EventListenerObject).handleEvent === 'function';
 }