Skip to content

Commit f83b906

Browse files
authored
fix(replay): Handle multiple clicks in a short time (#8773)
We assumed that the click debounce should only let one click per second through. however, that does not work if clicking different elements quickly (as it only debounces a single click/element). This adds a guard to ensure we only handle a single click breadcrumb per second. Fixes getsentry/sentry#54293
1 parent 695a671 commit f83b906

File tree

2 files changed

+99
-0
lines changed

2 files changed

+99
-0
lines changed

packages/replay/src/coreHandlers/handleClick.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,14 @@ export class ClickDetector implements ReplayClickDetector {
136136
clickCount: 0,
137137
node,
138138
};
139+
140+
// If there was a click in the last 1s on the same element, ignore it - only keep a single reference per second
141+
if (
142+
this._clicks.some(click => click.node === newClick.node && Math.abs(click.timestamp - newClick.timestamp) < 1)
143+
) {
144+
return;
145+
}
146+
139147
this._clicks.push(newClick);
140148

141149
// If this is the first new click, set a timeout to check for multi clicks

packages/replay/test/unit/coreHandlers/handleClick.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,97 @@ describe('Unit | coreHandlers | handleClick', () => {
7171
expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1);
7272
});
7373

74+
test('it captures multiple clicks', async () => {
75+
const replay = {
76+
getCurrentRoute: () => 'test-route',
77+
} as ReplayContainer;
78+
79+
const mockAddBreadcrumbEvent = jest.fn();
80+
81+
const detector = new ClickDetector(
82+
replay,
83+
{
84+
threshold: 1_000,
85+
timeout: 3_000,
86+
scrollTimeout: 200,
87+
ignoreSelector: '',
88+
},
89+
mockAddBreadcrumbEvent,
90+
);
91+
92+
const breadcrumb1: Breadcrumb = {
93+
timestamp: BASE_TIMESTAMP / 1000,
94+
data: {
95+
nodeId: 1,
96+
},
97+
};
98+
const breadcrumb2: Breadcrumb = {
99+
timestamp: (BASE_TIMESTAMP + 200) / 1000,
100+
data: {
101+
nodeId: 1,
102+
},
103+
};
104+
const breadcrumb3: Breadcrumb = {
105+
timestamp: (BASE_TIMESTAMP + 1200) / 1000,
106+
data: {
107+
nodeId: 1,
108+
},
109+
};
110+
const node = document.createElement('button');
111+
detector.handleClick(breadcrumb1, node);
112+
detector.handleClick(breadcrumb2, node);
113+
detector.handleClick(breadcrumb3, node);
114+
115+
expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0);
116+
117+
jest.advanceTimersByTime(1_000);
118+
119+
expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0);
120+
121+
jest.advanceTimersByTime(1_000);
122+
123+
expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0);
124+
125+
jest.advanceTimersByTime(1_000);
126+
127+
expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1);
128+
expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, {
129+
category: 'ui.slowClickDetected',
130+
type: 'default',
131+
data: {
132+
clickCount: 1,
133+
endReason: 'timeout',
134+
nodeId: 1,
135+
route: 'test-route',
136+
timeAfterClickMs: 3000,
137+
url: 'http://localhost/',
138+
},
139+
message: undefined,
140+
timestamp: BASE_TIMESTAMP / 1000,
141+
});
142+
143+
jest.advanceTimersByTime(2_000);
144+
145+
expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(2);
146+
expect(mockAddBreadcrumbEvent).toHaveBeenLastCalledWith(replay, {
147+
category: 'ui.slowClickDetected',
148+
type: 'default',
149+
data: {
150+
clickCount: 1,
151+
endReason: 'timeout',
152+
nodeId: 1,
153+
route: 'test-route',
154+
timeAfterClickMs: 3000,
155+
url: 'http://localhost/',
156+
},
157+
message: undefined,
158+
timestamp: (BASE_TIMESTAMP + 1200) / 1000,
159+
});
160+
161+
jest.advanceTimersByTime(5_000);
162+
expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(2);
163+
});
164+
74165
test('it captures clicks on different elements', async () => {
75166
const replay = {
76167
getCurrentRoute: () => 'test-route',

0 commit comments

Comments
 (0)