ref(types): Streamline wrap and related types

Author: mydea

packages/browser/src/helpers.ts

@@ -40,29 +40,32 @@ export function ignoreNextOnError(): void {
  * @returns The wrapped function.
  * @hidden
  */
-export function wrap(
-  fn: WrappedFunction,
+// eslint-disable-next-line @typescript-eslint/ban-types
+export function wrap<T extends Function, NonFunctionType>(
+  fn: T | NonFunctionType,
   options: {
     mechanism?: Mechanism;
   } = {},
-  before?: WrappedFunction,
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-): any {
+): NonFunctionType | 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 | NonFunctionType): 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 +87,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: unknown) => (typeof arg === 'function' ? wrap(arg, options) : arg));
 
       // Attempt to invoke user-land function
       // NOTE: If you are a Sentry user, and you are seeing this stack frame, it
@@ -125,18 +122,18 @@ export function wrap(
 
       throw ex;
     }
-  };
-  /* eslint-enable prefer-rest-params */
+  } as unknown as WrappedFunction<T>;
 
+  // Wrap the wrapped function in a proxy, to ensure any other properties of the original function remain available
   // Accessing some objects may throw
   // ref: https://github.com/getsentry/sentry-javascript/issues/1168
   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 {} // eslint-disable-line no-empty
 
   // Signal that this function has been wrapped/filled already
   // for both debugging and to prevent it to being wrapped/filled twice
@@ -146,16 +143,16 @@ 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 {} // eslint-disable-line no-empty
 
   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,13 +163,11 @@ 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
-  if (!proto || !proto.hasOwnProperty || !proto.hasOwnProperty('addEventListener')) {
+  if (!proto || Object.prototype.hasOwnProperty.call(proto, 'addEventListener')) {
     return;
   }
 
@@ -185,8 +177,7 @@ function _wrapEventTarget(target: string): void {
     options?: boolean | AddEventListenerOptions,
   ) => void {
     return function (
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      this: any,
+      this: unknown,
       eventName: string,
       fn: EventListenerObject,
       options?: boolean | AddEventListenerOptions,
@@ -217,8 +208,7 @@ function _wrapEventTarget(target: string): void {
 
       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 +224,45 @@ 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: () => void,): (
+    this: unknown,
+    eventName: string,
+    fn: EventListenerObject,
+    options?: boolean | EventListenerOptions,
+  ) => () => void {
+    return function (
+      this: unknown,
+      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);
         }
-        return originalRemoveEventListener.call(this, eventName, wrappedEventHandler, options);
-      };
-    },
-  );
+      } catch (e) {
+        // ignore, accessing __sentry_wrapped__ will throw in some Selenium environments
+      }
+      return originalRemoveEventListener.call(this, eventName, wrappedEventHandler, options);
+    };
+  });
 }

packages/browser/test/integrations/helpers.test.ts renamed to packages/browser/test/helpers.test.ts

@@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest';
 
 import type { WrappedFunction } from '@sentry/types';
 
-import { wrap } from '../../src/helpers';
+import { wrap } from '../src/helpers';
 
 describe('internal wrap()', () => {
   it('should wrap only functions', () => {
@@ -13,13 +13,9 @@ describe('internal wrap()', () => {
     const num = 42;
 
     expect(wrap(fn)).not.toBe(fn);
-    // @ts-expect-error Issue with `WrappedFunction` type from wrap fn
     expect(wrap(obj)).toBe(obj);
-    // @ts-expect-error Issue with `WrappedFunction` type from wrap fn
     expect(wrap(arr)).toBe(arr);
-    // @ts-expect-error Issue with `WrappedFunction` type from wrap fn
     expect(wrap(str)).toBe(str);
-    // @ts-expect-error Issue with `WrappedFunction` type from wrap fn
     expect(wrap(num)).toBe(num);
   });
 
@@ -56,16 +52,6 @@ describe('internal wrap()', () => {
     expect(wrap(wrapped)).toBe(wrapped);
   });
 
-  it('calls "before" function when invoking wrapped function', () => {
-    const fn = (() => 1337) as WrappedFunction;
-    const before = vi.fn();
-
-    const wrapped = wrap(fn, {}, before);
-    wrapped();
-
-    expect(before).toHaveBeenCalledTimes(1);
-  });
-
   it('attaches metadata to original and wrapped functions', () => {
     const fn = (() => 1337) as WrappedFunction;
 
@@ -78,10 +64,11 @@ describe('internal wrap()', () => {
     expect(wrapped.__sentry_original__).toBe(fn);
   });
 
-  it('copies over original functions properties', () => {
-    const fn = (() => 1337) as WrappedFunction;
-    fn.some = 1337;
-    fn.property = 'Rick';
+  it('keeps original functions properties', () => {
+    const fn = Object.assign(() => 1337, {
+      some: 1337,
+      property: 'Rick',
+    });
 
     const wrapped = wrap(fn);
 
@@ -105,7 +92,7 @@ describe('internal wrap()', () => {
   });
 
   it('recrusively wraps arguments that are functions', () => {
-    const fn = (() => 1337) as WrappedFunction;
+    const fn = (_arg1: unknown, _arg2: unknown) => 1337;
     const fnArgA = (): number => 1337;
     const fnArgB = (): number => 1337;
 

packages/core/src/utils-hoist/object.ts

@@ -81,7 +81,8 @@ export function markFunctionWrapped(wrapped: WrappedFunction, original: WrappedF
  * @param func the function to unwrap
  * @returns the unwrapped version of the function if available.
  */
-export function getOriginalFunction(func: WrappedFunction): WrappedFunction | undefined {
+// eslint-disable-next-line @typescript-eslint/ban-types
+export function getOriginalFunction<T extends Function>(func: WrappedFunction<T>): T | undefined {
   return func.__sentry_original__;
 }
 

packages/google-cloud-serverless/src/gcpfunction/events.ts

@@ -22,16 +22,15 @@ export function wrapEventFunction(
   return proxyFunction(fn, f => domainify(_wrapEventFunction(f, wrapOptions)));
 }
 
-/** */
 function _wrapEventFunction<F extends EventFunction | EventFunctionWithCallback>(
   fn: F,
   wrapOptions: Partial<EventFunctionWrapperOptions> = {},
-): (...args: Parameters<F>) => ReturnType<F> | Promise<void> {
+): (...args: Parameters<F>) => void | Promise<void> {
   const options: EventFunctionWrapperOptions = {
     flushTimeout: 2000,
     ...wrapOptions,
   };
-  return (...eventFunctionArguments: Parameters<F>): ReturnType<F> | Promise<void> => {
+  return (...eventFunctionArguments: Parameters<F>): void | Promise<void> => {
     const [data, context, callback] = eventFunctionArguments;
 
     return startSpanManual(
@@ -47,8 +46,8 @@ function _wrapEventFunction<F extends EventFunction | EventFunctionWithCallback>
         const scope = getCurrentScope();
         scope.setContext('gcp.function.context', { ...context });
 
-        const newCallback = domainify((...args: unknown[]) => {
-          if (args[0] !== null && args[0] !== undefined) {
+        const newCallback = domainify((...args): void => {
+          if (args[0] != null) {
             captureException(args[0], scope => markEventUnhandled(scope));
           }
           span.end();