diff --git a/packages/browser/src/integrations/trycatch.ts b/packages/browser/src/integrations/trycatch.ts index 8c60cdcf0cec..a9d94d83dfbc 100644 --- a/packages/browser/src/integrations/trycatch.ts +++ b/packages/browser/src/integrations/trycatch.ts @@ -174,11 +174,34 @@ export class TryCatch implements Integration { fn: EventListenerObject, options?: boolean | EventListenerOptions, ): () => void { - let callback = (fn as any) as WrappedFunction; + /** + * 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 has `__sentry__` property, it means that it was wrapped, and it has additional property + * of `__sentry__original__`, holding the handler. And this original handler, has a reversed link, + * with `__sentry_wrapped__` property, which holds the wrapped version. + * + * 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. + * In case of second scenario, `__sentry_original__` refers to initial handler, and passed function + * is a wrapped version. + */ + const 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 index 64d5f9ee431a..b3505b7dbcb4 100644 --- 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 '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 index ea7b80cd00c1..2cfc837f27b2 100644 --- 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() {