From 1330ee5a526bce3e25c9eaf312f80b7208637d2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Thu, 29 Oct 2020 14:31:22 +0100 Subject: [PATCH 1/2] fix: Dont call removeEventListener for functions that were not wrapped --- packages/browser/src/integrations/trycatch.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/browser/src/integrations/trycatch.ts b/packages/browser/src/integrations/trycatch.ts index 5b638201b91c..3fa9a26c83e6 100644 --- a/packages/browser/src/integrations/trycatch.ts +++ b/packages/browser/src/integrations/trycatch.ts @@ -230,12 +230,15 @@ export class TryCatch implements Integration { * 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 callback = (fn as unknown) as WrappedFunction; try { - original.call(this, eventName, ((fn as unknown) as WrappedFunction).__sentry_wrapped__, options); + if (callback && callback.__sentry_wrapped__) { + original.call(this, eventName, callback.__sentry_wrapped__, options); + } } catch (e) { // ignore, accessing __sentry_wrapped__ will throw in some Selenium environments } - return original.call(this, eventName, fn, options); + return original.call(this, eventName, callback, options); }; }); } From cdc402edda3e3dcb2422e64b981b7f4b0ab53c59 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 29 Oct 2020 15:50:41 -0700 Subject: [PATCH 2/2] more explanatory variable names --- packages/browser/src/integrations/trycatch.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/browser/src/integrations/trycatch.ts b/packages/browser/src/integrations/trycatch.ts index 3fa9a26c83e6..bbd5a4562763 100644 --- a/packages/browser/src/integrations/trycatch.ts +++ b/packages/browser/src/integrations/trycatch.ts @@ -203,7 +203,7 @@ export class TryCatch implements Integration { }); fill(proto, 'removeEventListener', function( - original: () => void, + originalRemoveEventListener: () => void, // eslint-disable-next-line @typescript-eslint/no-explicit-any ): (this: any, eventName: string, fn: EventListenerObject, options?: boolean | EventListenerOptions) => () => void { return function( @@ -230,15 +230,16 @@ export class TryCatch implements Integration { * 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 callback = (fn as unknown) as WrappedFunction; + const wrappedEventHandler = (fn as unknown) as WrappedFunction; try { - if (callback && callback.__sentry_wrapped__) { - original.call(this, eventName, callback.__sentry_wrapped__, options); + const originalEventHandler = wrappedEventHandler?.__sentry_wrapped__; + if (originalEventHandler) { + originalRemoveEventListener.call(this, eventName, originalEventHandler, options); } } catch (e) { // ignore, accessing __sentry_wrapped__ will throw in some Selenium environments } - return original.call(this, eventName, callback, options); + return originalRemoveEventListener.call(this, eventName, wrappedEventHandler, options); }; }); }