From bb67a1100d55d8ef73d5f34f57b5aebe1927045b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 11 Oct 2023 14:09:26 +0200 Subject: [PATCH] fix(utils): Dereference DOM events after they have servered their purpose (#9224) --- packages/utils/src/instrument.ts | 50 ++++++++++++-------------------- packages/utils/src/object.ts | 2 +- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/packages/utils/src/instrument.ts b/packages/utils/src/instrument.ts index f3364ba92b9c..99ddc3aaefc8 100644 --- a/packages/utils/src/instrument.ts +++ b/packages/utils/src/instrument.ts @@ -12,7 +12,7 @@ import type { import { isString } from './is'; import type { ConsoleLevel } from './logger'; import { CONSOLE_LEVELS, logger, originalConsoleMethods } from './logger'; -import { fill } from './object'; +import { addNonEnumerableProperty, fill } from './object'; import { getFunctionName } from './stacktrace'; import { supportsHistory, supportsNativeFetch } from './supports'; import { getGlobalObject, GLOBAL_OBJ } from './worldwide'; @@ -400,31 +400,24 @@ function instrumentHistory(): void { fill(WINDOW.history, 'replaceState', historyReplacementFunction); } -const debounceDuration = 1000; +const DEBOUNCE_DURATION = 1000; let debounceTimerID: number | undefined; let lastCapturedEvent: Event | undefined; /** - * Decide whether the current event should finish the debounce of previously captured one. - * @param previous previously captured event - * @param current event to be captured + * Check whether two DOM events are similar to eachother. For example, two click events on the same button. */ -function shouldShortcircuitPreviousDebounce(previous: Event | undefined, current: Event): boolean { - // If there was no previous event, it should always be swapped for the new one. - if (!previous) { - return true; - } - +function areSimilarDomEvents(a: Event, b: Event): boolean { // If both events have different type, then user definitely performed two separate actions. e.g. click + keypress. - if (previous.type !== current.type) { - return true; + if (a.type !== b.type) { + return false; } try { // If both events have the same type, it's still possible that actions were performed on different targets. // e.g. 2 clicks on different buttons. - if (previous.target !== current.target) { - return true; + if (a.target !== b.target) { + return false; } } catch (e) { // just accessing `target` property can throw an exception in some rare circumstances @@ -434,7 +427,7 @@ function shouldShortcircuitPreviousDebounce(previous: Event | undefined, current // If both events have the same type _and_ same `target` (an element which triggered an event, _not necessarily_ // to which an event listener was attached), we treat them as the same action, as we want to capture // only one breadcrumb. e.g. multiple clicks on the same button, or typing inside a user input box. - return false; + return true; } /** @@ -475,11 +468,11 @@ function shouldSkipDOMEvent(event: Event): boolean { * @hidden */ function makeDOMEventHandler(handler: Function, globalListener: boolean = false): (event: Event) => void { - return (event: Event): void => { + return (event: Event & { _sentryCaptured?: true }): void => { // It's possible this handler might trigger multiple times for the same // event (e.g. event propagation through node ancestors). // Ignore if we've already captured that event. - if (!event || lastCapturedEvent === event) { + if (!event || event['_sentryCaptured']) { return; } @@ -488,20 +481,15 @@ function makeDOMEventHandler(handler: Function, globalListener: boolean = false) return; } + // Mark event as "seen" + addNonEnumerableProperty(event, '_sentryCaptured', true); + const name = event.type === 'keypress' ? 'input' : event.type; - // If there is no debounce timer, it means that we can safely capture the new event and store it for future comparisons. - if (debounceTimerID === undefined) { - handler({ - event: event, - name, - global: globalListener, - }); - lastCapturedEvent = event; - } - // If there is a debounce awaiting, see if the new event is different enough to treat it as a unique one. + // If there is no last captured event, it means that we can safely capture the new event and store it for future comparisons. + // If there is a last captured event, see if the new event is different enough to treat it as a unique one. // If that's the case, emit the previous event and store locally the newly-captured DOM event. - else if (shouldShortcircuitPreviousDebounce(lastCapturedEvent, event)) { + if (lastCapturedEvent === undefined || !areSimilarDomEvents(lastCapturedEvent, event)) { handler({ event: event, name, @@ -513,8 +501,8 @@ function makeDOMEventHandler(handler: Function, globalListener: boolean = false) // Start a new debounce timer that will prevent us from capturing multiple events that should be grouped together. clearTimeout(debounceTimerID); debounceTimerID = WINDOW.setTimeout(() => { - debounceTimerID = undefined; - }, debounceDuration); + lastCapturedEvent = undefined; + }, DEBOUNCE_DURATION); }; } diff --git a/packages/utils/src/object.ts b/packages/utils/src/object.ts index e705214f950d..e4e866bf2763 100644 --- a/packages/utils/src/object.ts +++ b/packages/utils/src/object.ts @@ -42,7 +42,7 @@ export function fill(source: { [key: string]: any }, name: string, replacementFa * @param name The name of the property to be set * @param value The value to which to set the property */ -export function addNonEnumerableProperty(obj: { [key: string]: unknown }, name: string, value: unknown): void { +export function addNonEnumerableProperty(obj: object, name: string, value: unknown): void { try { Object.defineProperty(obj, name, { // enumerable: false, // the default, so we can save on bundle size by not explicitly setting it