fix(utils): Try-catch monkeypatching to handle frozen objects/functions

packages/browser-integration-tests/suites/public-api/instrumentation/setTimeout/subject.js

@@ -0,0 +1,5 @@
+function callback() {
+  throw new Error('setTimeout_error');
+}
+
+setTimeout(callback, 0);

packages/browser-integration-tests/suites/public-api/instrumentation/setTimeout/test.ts

@@ -0,0 +1,24 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+
+sentryTest('Instrumentation should capture errors in setTimeout', async ({ getLocalTestPath, page }) => {
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+  expect(eventData.exception?.values).toHaveLength(1);
+  expect(eventData.exception?.values?.[0]).toMatchObject({
+    type: 'Error',
+    value: 'setTimeout_error',
+    mechanism: {
+      type: 'instrument',
+      handled: false,
+    },
+    stacktrace: {
+      frames: expect.any(Array),
+    },
+  });
+});

packages/browser-integration-tests/suites/public-api/instrumentation/setTimeoutFrozen/init.js

@@ -0,0 +1,8 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  debug: true,
+});

packages/browser-integration-tests/suites/public-api/instrumentation/setTimeoutFrozen/subject.js

@@ -0,0 +1,5 @@
+function callback() {
+  throw new Error('setTimeout_error');
+}
+
+setTimeout(Object.freeze(callback), 0);

packages/browser-integration-tests/suites/public-api/instrumentation/setTimeoutFrozen/test.ts

@@ -0,0 +1,48 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers';
+
+sentryTest(
+  'Instrumentation does not fail when using frozen callback for setTimeout',
+  async ({ getLocalTestPath, page }) => {
+    const bundleKey = process.env.PW_BUNDLE || '';
+    const hasDebug = !bundleKey.includes('_min');
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+
+    const logMessages: string[] = [];
+
+    page.on('console', msg => {
+      logMessages.push(msg.text());
+    });
+
+    const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+
+    // It still captures the error
+    expect(eventData.exception?.values).toHaveLength(1);
+    expect(eventData.exception?.values?.[0]).toMatchObject({
+      type: 'Error',
+      value: 'setTimeout_error',
+      mechanism: {
+        type: 'instrument',
+        handled: false,
+      },
+      stacktrace: {
+        frames: expect.any(Array),
+      },
+    });
+
+    // We only care about the message when debug is enabled
+    if (hasDebug) {
+      expect(logMessages).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining(
+            'Sentry Logger [log]: Failed to add non-enumerable property "__sentry_wrapped__" to object function callback()',
+          ),
+        ]),
+      );
+    }
+  },
+);

packages/browser/test/unit/index.test.ts

@@ -1,4 +1,5 @@
 import { getReportDialogEndpoint, SDK_VERSION } from '@sentry/core';
+import type { WrappedFunction } from '@sentry/types';
 import * as utils from '@sentry/utils';
 
 import type { Event } from '../../src';
@@ -391,4 +392,14 @@ describe('wrap()', () => {
 
     expect(result2).toBe(42);
   });
+
+  it('should ignore frozen functions', () => {
+    const func = Object.freeze(() => 42);
+
+    // eslint-disable-next-line deprecation/deprecation
+    wrap(func);
+
+    expect(func()).toBe(42);
+    expect((func as WrappedFunction).__sentry_wrapped__).toBeUndefined();
+  });
 });

packages/utils/src/object.ts

@@ -4,6 +4,7 @@ import type { WrappedFunction } from '@sentry/types';
 
 import { htmlTreeAsString } from './browser';
 import { isElement, isError, isEvent, isInstanceOf, isPlainObject, isPrimitive } from './is';
+import { logger } from './logger';
 import { truncate } from './string';
 
 /**
@@ -28,12 +29,7 @@ export function fill(source: { [key: string]: any }, name: string, replacementFa
   // Make sure it's a function first, as we need to attach an empty prototype for `defineProperties` to work
   // otherwise it'll throw "TypeError: Object.defineProperties called on non-object"
   if (typeof wrapped === 'function') {
-    try {
-      markFunctionWrapped(wrapped, original);
-    } catch (_Oo) {
-      // This can throw if multiple fill happens on a global object like XMLHttpRequest
-      // Fixes https://github.com/getsentry/sentry-javascript/issues/2043
-    }
+    markFunctionWrapped(wrapped, original);
   }
 
   source[name] = wrapped;
@@ -47,12 +43,16 @@ export function fill(source: { [key: string]: any }, name: string, replacementFa
  * @param value The value to which to set the property
  */
 export function addNonEnumerableProperty(obj: { [key: string]: unknown }, name: string, value: unknown): void {
-  Object.defineProperty(obj, name, {
-    // enumerable: false, // the default, so we can save on bundle size by not explicitly setting it
-    value: value,
-    writable: true,
-    configurable: true,
-  });
+  try {
+    Object.defineProperty(obj, name, {
+      // enumerable: false, // the default, so we can save on bundle size by not explicitly setting it
+      value: value,
+      writable: true,
+      configurable: true,
+    });
+  } catch (o_O) {
+    __DEBUG_BUILD__ && logger.log(`Failed to add non-enumerable property "${name}" to object`, obj);
+  }
 }
 
 /**
@@ -63,9 +63,11 @@ export function addNonEnumerableProperty(obj: { [key: string]: unknown }, name:
  * @param original the original function that gets wrapped
  */
 export function markFunctionWrapped(wrapped: WrappedFunction, original: WrappedFunction): void {
-  const proto = original.prototype || {};
-  wrapped.prototype = original.prototype = proto;
-  addNonEnumerableProperty(wrapped, '__sentry_original__', original);
+  try {
+    const proto = original.prototype || {};
+    wrapped.prototype = original.prototype = proto;
+    addNonEnumerableProperty(wrapped, '__sentry_original__', original);
+  } catch (o_O) {} // eslint-disable-line no-empty
 }
 
 /**

packages/utils/test/object.test.ts

@@ -2,7 +2,17 @@
  * @jest-environment jsdom
  */
 
-import { dropUndefinedKeys, extractExceptionKeysForMessage, fill, objectify, urlEncode } from '../src/object';
+import type { WrappedFunction } from '@sentry/types';
+
+import {
+  addNonEnumerableProperty,
+  dropUndefinedKeys,
+  extractExceptionKeysForMessage,
+  fill,
+  markFunctionWrapped,
+  objectify,
+  urlEncode,
+} from '../src/object';
 import { testOnlyIfNodeVersionAtLeast } from './testutils';
 
 describe('fill()', () => {
@@ -315,3 +325,95 @@ describe('objectify()', () => {
     expect(objectifiedNonPrimtive).toBe(notAPrimitive);
   });
 });
+
+describe('addNonEnumerableProperty', () => {
+  it('works with a plain object', () => {
+    const obj: { foo?: string } = {};
+    addNonEnumerableProperty(obj, 'foo', 'bar');
+    expect(obj.foo).toBe('bar');
+  });
+
+  it('works with a class', () => {
+    class MyClass {
+      public foo?: string;
+    }
+    const obj = new MyClass();
+    addNonEnumerableProperty(obj as any, 'foo', 'bar');
+    expect(obj.foo).toBe('bar');
+  });
+
+  it('works with a function', () => {
+    const func = jest.fn();
+    addNonEnumerableProperty(func as any, 'foo', 'bar');
+    expect((func as any).foo).toBe('bar');
+    func();
+    expect(func).toHaveBeenCalledTimes(1);
+  });
+
+  it('works with an existing property object', () => {
+    const obj = { foo: 'before' };
+    addNonEnumerableProperty(obj, 'foo', 'bar');
+    expect(obj.foo).toBe('bar');
+  });
+
+  it('works with an existing readonly property object', () => {
+    const obj = { foo: 'before' };
+
+    Object.defineProperty(obj, 'foo', {
+      value: 'defined',
+      writable: false,
+    });
+
+    addNonEnumerableProperty(obj, 'foo', 'bar');
+    expect(obj.foo).toBe('bar');
+  });
+
+  it('does not error with a frozen object', () => {
+    const obj = Object.freeze({ foo: 'before' });
+
+    addNonEnumerableProperty(obj, 'foo', 'bar');
+    expect(obj.foo).toBe('before');
+  });
+});
+
+describe('markFunctionWrapped', () => {
+  it('works with a function', () => {
+    const originalFunc = jest.fn();
+    const wrappedFunc = jest.fn();
+    markFunctionWrapped(wrappedFunc, originalFunc);
+
+    expect((wrappedFunc as WrappedFunction).__sentry_original__).toBe(originalFunc);
+
+    wrappedFunc();
+
+    expect(wrappedFunc).toHaveBeenCalledTimes(1);
+    expect(originalFunc).not.toHaveBeenCalled();
+  });
+
+  it('works with a frozen original function', () => {
+    const originalFunc = Object.freeze(jest.fn());
+    const wrappedFunc = jest.fn();
+    markFunctionWrapped(wrappedFunc, originalFunc);
+
+    expect((wrappedFunc as WrappedFunction).__sentry_original__).toBe(originalFunc);
+
+    wrappedFunc();
+
+    expect(wrappedFunc).toHaveBeenCalledTimes(1);
+    expect(originalFunc).not.toHaveBeenCalled();
+  });
+
+  it('works with a frozen wrapped function', () => {
+    const originalFunc = Object.freeze(jest.fn());
+    const wrappedFunc = Object.freeze(jest.fn());
+    markFunctionWrapped(wrappedFunc, originalFunc);
+
+    // Skips adding the property, but also doesn't error
+    expect((wrappedFunc as WrappedFunction).__sentry_original__).toBe(undefined);
+
+    wrappedFunc();
+
+    expect(wrappedFunc).toHaveBeenCalledTimes(1);
+    expect(originalFunc).not.toHaveBeenCalled();
+  });
+});