fix: Make sure that XHR is wrapped correctly for TryCatch and Instrument (#2343)

Author: kamilogorek

packages/browser/src/integrations/trycatch.ts

@@ -164,11 +164,10 @@ export class TryCatch implements Integration {
               type: 'instrument',
             },
           };
-          // If Instrument integration has been called before TryCatch
-          // use it as "before" callback in the wrap and use the original instead
-          if (original.__sentry__ && original.__sentry_original__) {
+
+          // If Instrument integration has been called before TryCatch, get the name of original function
+          if (original.__sentry_original__) {
             wrapOptions.mechanism.data.handler = getFunctionName(original.__sentry_original__);
-            return wrap(original.__sentry_original__, wrapOptions, original.__sentry_wrapped__);
           }
 
           // Otherwise wrap directly

packages/browser/test/integration/suites/builtins.js

@@ -389,6 +389,22 @@ describe("wrapped built-ins", function() {
     });
   });
 
+  it("should not call XMLHttpRequest onreadystatechange more than once", function() {
+    return runInSandbox(sandbox, { manual: true }, function() {
+      window.calls = 0;
+      var xhr = new XMLHttpRequest();
+      xhr.open("GET", "/base/subjects/example.json");
+      xhr.onreadystatechange = function wat() {
+        window.finalizeManualTest();
+        window.calls += 1;
+      };
+      xhr.send();
+    }).then(function(summary) {
+      assert.equal(summary.window.calls, 3);
+      delete summary.window.calls;
+    });
+  });
+
   it(
     optional(
       "should capture built-in's mechanism type as instrument",

packages/utils/src/object.ts

@@ -28,18 +28,10 @@ export function fill(source: { [key: string]: any }, name: string, replacement:
     try {
       wrapped.prototype = wrapped.prototype || {};
       Object.defineProperties(wrapped, {
-        __sentry__: {
-          enumerable: false,
-          value: true,
-        },
         __sentry_original__: {
           enumerable: false,
           value: original,
         },
-        __sentry_wrapped__: {
-          enumerable: false,
-          value: wrapped,
-        },
       });
     } catch (_Oo) {
       // This can throw if multiple fill happens on a global object like XMLHttpRequest

packages/utils/test/object.test.ts

@@ -67,13 +67,9 @@ describe('fill()', () => {
     fill(source, name, replacement);
 
     // Shouldn't show up in iteration
-    expect(Object.keys(replacement)).not.toContain('__sentry__');
     expect(Object.keys(replacement)).not.toContain('__sentry_original__');
-    expect(Object.keys(replacement)).not.toContain('__sentry_wrapped__');
     // But should be accessible directly
-    expect(source.foo.__sentry__).toBe(true);
     expect(source.foo.__sentry_original__).toBe(source.foo);
-    expect(source.foo.__sentry_wrapped__).toBe(source.foo);
   });
 
   test('should preserve functions prototype if one exists', () => {