Skip to content

Commit dba9a3d

Browse files
authored
feat(replay): Rework slow click & multi click detection (#8322)
This PR reworks the slow click detection to accommodate rage click detection as well. This required substantial changes, as we need to keep track of stuff much more. Now, we keep a list of clicks that come in in a new class. We register a single set of listeners (mutation observer, click listener, scroll listener), and then try to route things to the correct clicks as much as possible. Any clicks within 1 second count as "multi click", so are not considered for slow clicks at all, but counted on the first click. After a second, a click (even on the same element) will be treated as a "new" click. ref #8300
1 parent ea45d20 commit dba9a3d

File tree

17 files changed

+1142
-237
lines changed

17 files changed

+1142
-237
lines changed

packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,10 @@ import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest }
6565
expect(slowClickBreadcrumbs).toEqual([
6666
{
6767
category: 'ui.slowClickDetected',
68+
type: 'default',
6869
data: {
6970
endReason: 'timeout',
71+
clickCount: 1,
7072
node: {
7173
attributes: expect.objectContaining({
7274
id,
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';
5+
6+
sentryTest('captures multi click when not detecting slow click', async ({ getLocalTestUrl, page }) => {
7+
if (shouldSkipReplayTest()) {
8+
sentryTest.skip();
9+
}
10+
11+
const reqPromise0 = waitForReplayRequest(page, 0);
12+
13+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
14+
return route.fulfill({
15+
status: 200,
16+
contentType: 'application/json',
17+
body: JSON.stringify({ id: 'test-id' }),
18+
});
19+
});
20+
21+
const url = await getLocalTestUrl({ testDir: __dirname });
22+
23+
await page.goto(url);
24+
await reqPromise0;
25+
26+
const reqPromise1 = waitForReplayRequest(page, (event, res) => {
27+
const { breadcrumbs } = getCustomRecordingEvents(res);
28+
29+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.multiClick');
30+
});
31+
32+
await page.click('#mutationButtonImmediately', { clickCount: 4 });
33+
34+
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
35+
36+
const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
37+
38+
expect(slowClickBreadcrumbs).toEqual([
39+
{
40+
category: 'ui.multiClick',
41+
type: 'default',
42+
data: {
43+
clickCount: 4,
44+
metric: true,
45+
node: {
46+
attributes: {
47+
id: 'mutationButtonImmediately',
48+
},
49+
id: expect.any(Number),
50+
tagName: 'button',
51+
textContent: '******* ******** ***********',
52+
},
53+
nodeId: expect.any(Number),
54+
url: 'http://sentry-test.io/index.html',
55+
},
56+
message: 'body > button#mutationButtonImmediately',
57+
timestamp: expect.any(Number),
58+
},
59+
]);
60+
});

packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts

Lines changed: 116 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe
2929
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
3030
});
3131

32-
// Trigger this twice, sometimes this was flaky otherwise...
33-
await page.click('#mutationButton');
3432
await page.click('#mutationButton');
3533

3634
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
@@ -40,8 +38,71 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe
4038
expect(slowClickBreadcrumbs).toEqual([
4139
{
4240
category: 'ui.slowClickDetected',
41+
type: 'default',
42+
data: {
43+
endReason: 'mutation',
44+
clickCount: 1,
45+
node: {
46+
attributes: {
47+
id: 'mutationButton',
48+
},
49+
id: expect.any(Number),
50+
tagName: 'button',
51+
textContent: '******* ********',
52+
},
53+
nodeId: expect.any(Number),
54+
timeAfterClickMs: expect.any(Number),
55+
url: 'http://sentry-test.io/index.html',
56+
},
57+
message: 'body > button#mutationButton',
58+
timestamp: expect.any(Number),
59+
},
60+
]);
61+
62+
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
63+
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
64+
});
65+
66+
sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => {
67+
if (shouldSkipReplayTest()) {
68+
sentryTest.skip();
69+
}
70+
71+
const reqPromise0 = waitForReplayRequest(page, 0);
72+
73+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
74+
return route.fulfill({
75+
status: 200,
76+
contentType: 'application/json',
77+
body: JSON.stringify({ id: 'test-id' }),
78+
});
79+
});
80+
81+
const url = await getLocalTestUrl({ testDir: __dirname });
82+
83+
await page.goto(url);
84+
await reqPromise0;
85+
86+
const reqPromise1 = waitForReplayRequest(page, (event, res) => {
87+
const { breadcrumbs } = getCustomRecordingEvents(res);
88+
89+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
90+
});
91+
92+
void page.click('#mutationButton', { clickCount: 4 });
93+
94+
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
95+
96+
const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
97+
const multiClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
98+
99+
expect(slowClickBreadcrumbs).toEqual([
100+
{
101+
category: 'ui.slowClickDetected',
102+
type: 'default',
43103
data: {
44104
endReason: 'mutation',
105+
clickCount: 4,
45106
node: {
46107
attributes: {
47108
id: 'mutationButton',
@@ -58,6 +119,7 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe
58119
timestamp: expect.any(Number),
59120
},
60121
]);
122+
expect(multiClickBreadcrumbs.length).toEqual(0);
61123

62124
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
63125
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
@@ -165,3 +227,55 @@ sentryTest('inline click handler does not trigger slow click', async ({ getLocal
165227
},
166228
]);
167229
});
230+
231+
sentryTest('mouseDown events are considered', async ({ browserName, getLocalTestUrl, page }) => {
232+
// This test seems to only be flakey on firefox
233+
if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
234+
sentryTest.skip();
235+
}
236+
237+
const reqPromise0 = waitForReplayRequest(page, 0);
238+
239+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
240+
return route.fulfill({
241+
status: 200,
242+
contentType: 'application/json',
243+
body: JSON.stringify({ id: 'test-id' }),
244+
});
245+
});
246+
247+
const url = await getLocalTestUrl({ testDir: __dirname });
248+
249+
await page.goto(url);
250+
await reqPromise0;
251+
252+
const reqPromise1 = waitForReplayRequest(page, (event, res) => {
253+
const { breadcrumbs } = getCustomRecordingEvents(res);
254+
255+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
256+
});
257+
258+
await page.click('#mouseDownButton');
259+
260+
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
261+
262+
expect(breadcrumbs).toEqual([
263+
{
264+
category: 'ui.click',
265+
data: {
266+
node: {
267+
attributes: {
268+
id: 'mouseDownButton',
269+
},
270+
id: expect.any(Number),
271+
tagName: 'button',
272+
textContent: '******* ******** ** ***** ****',
273+
},
274+
nodeId: expect.any(Number),
275+
},
276+
message: 'body > button#mouseDownButton',
277+
timestamp: expect.any(Number),
278+
type: 'default',
279+
},
280+
]);
281+
});

packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,10 @@ sentryTest('late scroll triggers slow click', async ({ getLocalTestUrl, page })
8989
expect(slowClickBreadcrumbs).toEqual([
9090
{
9191
category: 'ui.slowClickDetected',
92+
type: 'default',
9293
data: {
9394
endReason: 'timeout',
95+
clickCount: 1,
9496
node: {
9597
attributes: {
9698
id: 'scrollLateButton',

packages/browser-integration-tests/suites/replay/slowClick/template.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
<button id="scrollButton">Trigger scroll</button>
1919
<button id="scrollLateButton">Trigger scroll late</button>
2020
<button id="mutationIgnoreButton" class="ignore-class">Trigger scroll late</button>
21+
<button id="mouseDownButton">Trigger mutation on mouse down</button>
2122

2223
<a href="#" id="link">Link</a>
2324
<a href="#" target="_blank" id="linkExternal">Link external</a>
@@ -69,6 +70,9 @@ <h1 id="h2">Bottom</h1>
6970
console.log('DONE');
7071
}, 3001);
7172
});
73+
document.getElementById('mouseDownButton').addEventListener('mousedown', () => {
74+
document.getElementById('out').innerHTML += 'mutationButton clicked<br>';
75+
});
7276

7377
// Do nothing on these elements
7478
document

packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ sentryTest('mutation after timeout results in slow click', async ({ getLocalTest
3838
expect(slowClickBreadcrumbs).toEqual([
3939
{
4040
category: 'ui.slowClickDetected',
41+
type: 'default',
4142
data: {
4243
endReason: 'timeout',
44+
clickCount: 1,
4345
node: {
4446
attributes: {
4547
id: 'mutationButtonLate',
@@ -93,8 +95,10 @@ sentryTest('console.log results in slow click', async ({ getLocalTestUrl, page }
9395
expect(slowClickBreadcrumbs).toEqual([
9496
{
9597
category: 'ui.slowClickDetected',
98+
type: 'default',
9699
data: {
97100
endReason: 'timeout',
101+
clickCount: 1,
98102
node: {
99103
attributes: {
100104
id: 'consoleLogButton',

packages/replay/src/constants.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,5 @@ export const CONSOLE_ARG_MAX_SIZE = 5_000;
4242
export const SLOW_CLICK_THRESHOLD = 3_000;
4343
/* For scroll actions after a click, we only look for a very short time period to detect programmatic scrolling. */
4444
export const SLOW_CLICK_SCROLL_TIMEOUT = 300;
45+
/* Clicks in this time period are considered e.g. double/triple clicks. */
46+
export const MULTI_CLICK_TIMEOUT = 1_000;

0 commit comments

Comments
 (0)