fix: Correctly remove all event listeners

getsentry · Jul 7, 2020 · 9a68a35 · 9a68a35
1 parent 6d986dd
commit 9a68a35
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 14 deletions.
diff --git a/packages/browser/src/integrations/trycatch.ts b/packages/browser/src/integrations/trycatch.ts
@@ -176,9 +176,11 @@ export class TryCatch implements Integration {
       ): () => void {
         let callback = (fn as any) as WrappedFunction;
         try {
-          callback = callback && (callback.__sentry_wrapped__ || callback);
+          if (callback && callback.__sentry__) {
+            original.call(this, eventName, callback.__sentry_original__, options);
+          }
         } catch (e) {
-          // ignore, accessing __sentry_wrapped__ will throw in some Selenium environments
+          // ignore, accessing __sentry__ will throw in some Selenium environments
         }
         return original.call(this, eventName, callback, options);
       };

diff --git a/packages/browser/test/integration/common/init.js b/packages/browser/test/integration/common/init.js
@@ -3,16 +3,7 @@
 // - make assertions re: wrapped functions
 var originalBuiltIns = {
   setTimeout: setTimeout,
-  setInterval: setInterval,
-  requestAnimationFrame: requestAnimationFrame,
-  xhrProtoOpen: XMLHttpRequest.prototype.open,
-  headAddEventListener: document.head.addEventListener, // use <head> 'cause body isn't closed yet
-  headRemoveEventListener: document.head.removeEventListener,
-  consoleDebug: console.debug,
-  consoleInfo: console.info,
-  consoleWarn: console.warn,
-  consoleError: console.error,
-  consoleLog: console.log,
+  addEventListener: document.addEventListener,
 };
 
 var events = [];

diff --git a/packages/browser/test/integration/suites/builtins.js b/packages/browser/test/integration/suites/builtins.js
@@ -27,14 +27,13 @@ describe("wrapped built-ins", function() {
     return runInSandbox(sandbox, function() {
       var div = document.createElement("div");
       document.body.appendChild(div);
-      var click = new MouseEvent("click");
       var fooFn = function() {
         foo();
       };
       var barFn = function() {
         bar();
       };
-      div.addEventListener("click", fooFn, false);
+      div.addEventListener("click", fooFn);
       div.addEventListener("click", barFn);
       div.removeEventListener("click", barFn);
       div.dispatchEvent(new MouseEvent("click"));
@@ -43,6 +42,26 @@ describe("wrapped built-ins", function() {
     });
   });
 
+  it("should remove the original callback if it was registered before Sentry initialized (w. original method)", function() {
+    return runInSandbox(sandbox, function() {
+      var div = document.createElement("div");
+      document.body.appendChild(div);
+      window.capturedCall = false;
+      var captureFn = function() {
+        window.capturedCall = true;
+      };
+      // Use original addEventListener to simulate non-wrapped behavior (callback is attached without __sentry_wrapped__)
+      window.originalBuiltIns.addEventListener.call(div, "click", captureFn);
+      // Then attach the same callback again, but with already wrapped method
+      div.addEventListener("click", captureFn);
+      div.removeEventListener("click", captureFn);
+      div.dispatchEvent(new MouseEvent("click"));
+    }).then(function(summary) {
+      assert.equal(summary.window.capturedCall, false);
+      delete summary.window.capturedCalls;
+    });
+  });
+
   it("should capture exceptions inside setTimeout", function() {
     return runInSandbox(sandbox, function() {
       setTimeout(function() {