Skip to content

Commit 281ba15

Browse files
mydeamydea
committed
add tests
1 parent 18da4d9 commit 281ba15

File tree

5 files changed

+198
-6
lines changed

5 files changed

+198
-6
lines changed

‎packages/browser-integration-tests/suites/replay/slowClick/error/init.js

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
window.Replay = new Sentry.Replay({
5+
flushMinDelay: 200,
6+
flushMaxDelay: 200,
7+
minReplayDuration: 0,
8+
slowClickTimeout: 3500,
9+
});
10+
11+
Sentry.init({
12+
dsn: 'https://[email protected]/1337',
13+
sampleRate: 1,
14+
replaysSessionSampleRate: 0.0,
15+
replaysOnErrorSampleRate: 1.0,
16+
17+
integrations: [window.Replay],
18+
});

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

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<button id="buttonError">Trigger error</button>
8+
<button id="buttonErrorMutation">Trigger error</button>
9+
10+
<script>
11+
document.getElementById('buttonError').addEventListener('click', () => {
12+
throw new Error('test error happened');
13+
});
14+
15+
document.getElementById('buttonErrorMutation').addEventListener('click', () => {
16+
document.getElementById('buttonErrorMutation').innerText = 'Test error happened!';
17+
18+
throw new Error('test error happened');
19+
});
20+
</script>
21+
</body>
22+
</html>

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

+135
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import {
5+
getCustomRecordingEvents,
6+
getReplayEventFromRequest,
7+
shouldSkipReplayTest,
8+
waitForReplayRequest,
9+
} from '../../../../utils/replayHelpers';
10+
11+
sentryTest('slow click that triggers error is captured', async ({ getLocalTestUrl, page }) => {
12+
if (shouldSkipReplayTest()) {
13+
sentryTest.skip();
14+
}
15+
16+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
17+
return route.fulfill({
18+
status: 200,
19+
contentType: 'application/json',
20+
body: JSON.stringify({ id: 'test-id' }),
21+
});
22+
});
23+
24+
const url = await getLocalTestUrl({ testDir: __dirname });
25+
26+
await page.goto(url);
27+
28+
const [req0] = await Promise.all([
29+
waitForReplayRequest(page, (_event, res) => {
30+
const { breadcrumbs } = getCustomRecordingEvents(res);
31+
32+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
33+
}),
34+
page.click('#buttonError'),
35+
]);
36+
37+
const { breadcrumbs } = getCustomRecordingEvents(req0);
38+
39+
const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
40+
41+
expect(slowClickBreadcrumbs).toEqual([
42+
{
43+
category: 'ui.slowClickDetected',
44+
type: 'default',
45+
data: {
46+
endReason: 'timeout',
47+
clickCount: 1,
48+
node: {
49+
attributes: {
50+
id: 'buttonError',
51+
},
52+
id: expect.any(Number),
53+
tagName: 'button',
54+
textContent: '******* *****',
55+
},
56+
nodeId: expect.any(Number),
57+
timeAfterClickMs: 3500,
58+
url: 'http://sentry-test.io/index.html',
59+
},
60+
message: 'body > button#buttonError',
61+
timestamp: expect.any(Number),
62+
},
63+
]);
64+
});
65+
66+
sentryTest('click that triggers error & mutation is not captured', async ({ getLocalTestUrl, page }) => {
67+
if (shouldSkipReplayTest()) {
68+
sentryTest.skip();
69+
}
70+
71+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
72+
return route.fulfill({
73+
status: 200,
74+
contentType: 'application/json',
75+
body: JSON.stringify({ id: 'test-id' }),
76+
});
77+
});
78+
79+
const url = await getLocalTestUrl({ testDir: __dirname });
80+
81+
await page.goto(url);
82+
83+
let slowClickCount = 0;
84+
85+
page.on('response', res => {
86+
const req = res.request();
87+
88+
const event = getReplayEventFromRequest(req);
89+
90+
if (!event) {
91+
return;
92+
}
93+
94+
const { breadcrumbs } = getCustomRecordingEvents(res);
95+
96+
const slowClicks = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
97+
slowClickCount += slowClicks.length;
98+
});
99+
100+
const [req1] = await Promise.all([
101+
waitForReplayRequest(page, (_event, res) => {
102+
const { breadcrumbs } = getCustomRecordingEvents(res);
103+
104+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
105+
}),
106+
page.click('#buttonErrorMutation'),
107+
]);
108+
109+
const { breadcrumbs } = getCustomRecordingEvents(req1);
110+
111+
expect(breadcrumbs).toEqual([
112+
{
113+
category: 'ui.click',
114+
data: {
115+
node: {
116+
attributes: {
117+
id: 'buttonErrorMutation',
118+
},
119+
id: expect.any(Number),
120+
tagName: 'button',
121+
textContent: '******* *****',
122+
},
123+
nodeId: expect.any(Number),
124+
},
125+
message: 'body > button#buttonErrorMutation',
126+
timestamp: expect.any(Number),
127+
type: 'default',
128+
},
129+
]);
130+
131+
// Ensure we wait for timeout, to make sure no slow click is created
132+
await new Promise(resolve => setTimeout(resolve, 3500));
133+
134+
expect(slowClickCount).toBe(0);
135+
});

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

+15-1
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,17 @@ sentryTest('immediate mutation does not trigger slow click', async ({ forceFlush
141141
await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]);
142142
await forceFlushReplay();
143143

144+
let slowClickCount = 0;
145+
146+
page.on('response', res => {
147+
const { breadcrumbs } = getCustomRecordingEvents(res);
148+
149+
const slowClicks = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
150+
slowClickCount += slowClicks.length;
151+
});
152+
144153
const [req1] = await Promise.all([
145-
waitForReplayRequest(page, (event, res) => {
154+
waitForReplayRequest(page, (_event, res) => {
146155
const { breadcrumbs } = getCustomRecordingEvents(res);
147156

148157
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
@@ -171,6 +180,11 @@ sentryTest('immediate mutation does not trigger slow click', async ({ forceFlush
171180
type: 'default',
172181
},
173182
]);
183+
184+
// Ensure we wait for timeout, to make sure no slow click is created
185+
await new Promise(resolve => setTimeout(resolve, 3500));
186+
187+
expect(slowClickCount).toBe(0);
174188
});
175189

176190
sentryTest('inline click handler does not trigger slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => {

‎packages/replay/src/coreHandlers/handleClick.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {
1010
ReplaySlowClickFrame,
1111
SlowClickConfig,
1212
} from '../types';
13-
import { ReplayEventTypeFullSnapshot, ReplayEventTypeIncrementalSnapshot } from '../types';
13+
import { ReplayEventTypeIncrementalSnapshot } from '../types';
1414
import { timestampToS } from '../util/timestamp';
1515
import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
1616
import { getClosestInteractive } from './util/domUtils';
@@ -310,10 +310,13 @@ function nowInSeconds(): number {
310310
/** Update the click detector based on a recording event of rrweb. */
311311
export function updateClickDetectorForRecordingEvent(clickDetector: ReplayClickDetector, event: RecordingEvent): void {
312312
try {
313-
// We interpret a full snapshot as a mutation (this may not be true, but there is no way for us to know)
314-
if (event.type === ReplayEventTypeFullSnapshot) {
315-
clickDetector.registerMutation(event.timestamp);
316-
}
313+
// note: We only consider incremental snapshots here
314+
// This means that any full snapshot is ignored for mutation detection - the reason is that we simply cannot know if a mutation happened here.
315+
// E.g. think that we are buffering, an error happens and we take a full snapshot because we switched to session mode -
316+
// in this scenario, we would not know if a dead click happened because of the error, which is a key dead click scenario.
317+
// Instead, by ignoring full snapshots, we have the risk that we generate a false positive
318+
// (if a mutation _did_ happen but was "swallowed" by the full snapshot)
319+
// But this should be more unlikely as we'd generally capture the incremental snapshot right away
317320

318321
if (!isIncrementalEvent(event)) {
319322
return;

0 commit comments

Comments
 (0)