Skip to content

Commit

Permalink
fix(utils): Dereference DOM events after they have servered their pur…
Browse files Browse the repository at this point in the history
…pose (#9224)
  • Loading branch information
lforst authored Oct 11, 2023
1 parent bde088e commit bb67a11
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 32 deletions.
50 changes: 19 additions & 31 deletions packages/utils/src/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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;
}

Expand All @@ -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,
Expand All @@ -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);
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bb67a11

Please sign in to comment.