Skip to content

Commit c6a8c24

Browse files
mydeamydea
committed
ref(replay): Improve handling of checkout events
1 parent ff2fcfd commit c6a8c24

File tree

7 files changed

+160
-59
lines changed

7 files changed

+160
-59
lines changed

‎packages/integration-tests/suites/replay/multiple-pages/test.ts‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,31 @@ sentryTest(
4545

4646
const reqPromise0 = waitForReplayRequest(page, 0);
4747
const reqPromise1 = waitForReplayRequest(page, 1);
48+
const reqPromise2 = waitForReplayRequest(page, 2);
49+
const reqPromise3 = waitForReplayRequest(page, 3);
50+
const reqPromise4 = waitForReplayRequest(page, 4);
51+
const reqPromise5 = waitForReplayRequest(page, 5);
52+
const reqPromise6 = waitForReplayRequest(page, 6);
53+
const reqPromise7 = waitForReplayRequest(page, 7);
54+
const reqPromise8 = waitForReplayRequest(page, 8);
55+
const reqPromise9 = waitForReplayRequest(page, 9);
4856

4957
const url = await getLocalTestPath({ testDir: __dirname });
5058

5159
await page.goto(url);
52-
const replayEvent0 = getReplayEvent(await reqPromise0);
53-
const recording0 = getReplayRecordingContent(await reqPromise0);
60+
const req0 = await reqPromise0;
61+
const replayEvent0 = getReplayEvent(req0);
62+
const recording0 = getReplayRecordingContent(req0);
5463

5564
expect(replayEvent0).toEqual(getExpectedReplayEvent({ segment_id: 0 }));
5665
expect(normalize(recording0.fullSnapshots)).toMatchSnapshot('seg-0-snap-full');
5766
expect(recording0.incrementalSnapshots.length).toEqual(0);
5867

5968
await page.click('#go-background');
6069

61-
const replayEvent1 = getReplayEvent(await reqPromise1);
62-
const recording1 = getReplayRecordingContent(await reqPromise1);
70+
const req1 = await reqPromise1;
71+
const replayEvent1 = getReplayEvent(req1);
72+
const recording1 = getReplayRecordingContent(req1);
6373

6474
expect(replayEvent1).toEqual(
6575
getExpectedReplayEvent({ segment_id: 1, urls: [], replay_start_timestamp: undefined }),
@@ -91,20 +101,19 @@ sentryTest(
91101

92102
await page.reload();
93103

94-
const reqPromise2 = waitForReplayRequest(page, 2);
95-
const reqPromise3 = waitForReplayRequest(page, 3);
96-
97-
const replayEvent2 = getReplayEvent(await reqPromise2);
98-
const recording2 = getReplayRecordingContent(await reqPromise2);
104+
const req2 = await reqPromise2;
105+
const replayEvent2 = getReplayEvent(req2);
106+
const recording2 = getReplayRecordingContent(req2);
99107

100108
expect(replayEvent2).toEqual(getExpectedReplayEvent({ segment_id: 2, replay_start_timestamp: undefined }));
101109
expect(normalize(recording2.fullSnapshots)).toMatchSnapshot('seg-2-snap-full');
102110
expect(recording2.incrementalSnapshots.length).toEqual(0);
103111

104112
await page.click('#go-background');
105113

106-
const replayEvent3 = getReplayEvent(await reqPromise3);
107-
const recording3 = getReplayRecordingContent(await reqPromise3);
114+
const req3 = await reqPromise3;
115+
const replayEvent3 = getReplayEvent(req3);
116+
const recording3 = getReplayRecordingContent(req3);
108117

109118
expect(replayEvent3).toEqual(
110119
getExpectedReplayEvent({ segment_id: 3, urls: [], replay_start_timestamp: undefined }),
@@ -134,11 +143,9 @@ sentryTest(
134143

135144
await page.click('a');
136145

137-
const reqPromise4 = waitForReplayRequest(page, 4);
138-
const reqPromise5 = waitForReplayRequest(page, 5);
139-
140-
const replayEvent4 = getReplayEvent(await reqPromise4);
141-
const recording4 = getReplayRecordingContent(await reqPromise4);
146+
const req4 = await reqPromise4;
147+
const replayEvent4 = getReplayEvent(req4);
148+
const recording4 = getReplayRecordingContent(req4);
142149

143150
expect(replayEvent4).toEqual(
144151
getExpectedReplayEvent({
@@ -161,8 +168,9 @@ sentryTest(
161168

162169
await page.click('#go-background');
163170

164-
const replayEvent5 = getReplayEvent(await reqPromise5);
165-
const recording5 = getReplayRecordingContent(await reqPromise5);
171+
const req5 = await reqPromise5;
172+
const replayEvent5 = getReplayEvent(req5);
173+
const recording5 = getReplayRecordingContent(req5);
166174

167175
expect(replayEvent5).toEqual(
168176
getExpectedReplayEvent({
@@ -207,9 +215,9 @@ sentryTest(
207215

208216
await page.click('#spa-navigation');
209217

210-
const reqPromise6 = waitForReplayRequest(page, 6);
211-
const replayEvent6 = getReplayEvent(await reqPromise6);
212-
const recording6 = getReplayRecordingContent(await reqPromise6);
218+
const req6 = await reqPromise6;
219+
const replayEvent6 = getReplayEvent(req6);
220+
const recording6 = getReplayRecordingContent(req6);
213221

214222
expect(replayEvent6).toEqual(
215223
getExpectedReplayEvent({
@@ -231,9 +239,9 @@ sentryTest(
231239

232240
await page.click('#go-background');
233241

234-
const reqPromise7 = waitForReplayRequest(page, 7);
235-
const replayEvent7 = getReplayEvent(await reqPromise7);
236-
const recording7 = getReplayRecordingContent(await reqPromise7);
242+
const req7 = await reqPromise7;
243+
const replayEvent7 = getReplayEvent(req7);
244+
const recording7 = getReplayRecordingContent(req7);
237245

238246
expect(replayEvent7).toEqual(
239247
getExpectedReplayEvent({
@@ -279,11 +287,9 @@ sentryTest(
279287

280288
await page.click('a');
281289

282-
const reqPromise8 = waitForReplayRequest(page, 8);
283-
const reqPromise9 = waitForReplayRequest(page, 9);
284-
285-
const replayEvent8 = getReplayEvent(await reqPromise8);
286-
const recording8 = getReplayRecordingContent(await reqPromise8);
290+
const req8 = await reqPromise8;
291+
const replayEvent8 = getReplayEvent(req8);
292+
const recording8 = getReplayRecordingContent(req8);
287293

288294
expect(replayEvent8).toEqual(
289295
getExpectedReplayEvent({
@@ -296,8 +302,9 @@ sentryTest(
296302

297303
await page.click('#go-background');
298304

299-
const replayEvent9 = getReplayEvent(await reqPromise9);
300-
const recording9 = getReplayRecordingContent(await reqPromise9);
305+
const req9 = await reqPromise9;
306+
const replayEvent9 = getReplayEvent(req9);
307+
const recording9 = getReplayRecordingContent(req9);
301308

302309
expect(replayEvent9).toEqual(
303310
getExpectedReplayEvent({

‎packages/integration-tests/suites/replay/multiple-pages/test.ts-snapshots/seg-6-snap-incremental‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,4 @@
11
[
2-
{
3-
"source": 1,
4-
"positions": [
5-
{
6-
"x": 157,
7-
"y": 90,
8-
"id": 15,
9-
"timeOffset": [timeOffset]
10-
}
11-
]
12-
},
132
{
143
"source": 2,
154
"type": 1,
@@ -40,5 +29,16 @@
4029
"id": 15,
4130
"x": 157,
4231
"y": 90
32+
},
33+
{
34+
"source": 1,
35+
"positions": [
36+
{
37+
"x": 157,
38+
"y": 90,
39+
"id": 15,
40+
"timeOffset": [timeOffset]
41+
}
42+
]
4343
}
4444
]

‎packages/integration-tests/suites/replay/multiple-pages/test.ts-snapshots/seg-6-snap-incremental-chromium‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,4 @@
11
[
2-
{
3-
"source": 1,
4-
"positions": [
5-
{
6-
"x": 157,
7-
"y": 90,
8-
"id": 15,
9-
"timeOffset": [timeOffset]
10-
}
11-
]
12-
},
132
{
143
"source": 2,
154
"type": 1,
@@ -40,5 +29,16 @@
4029
"id": 15,
4130
"x": 157,
4231
"y": 90
32+
},
33+
{
34+
"source": 1,
35+
"positions": [
36+
{
37+
"x": 157,
38+
"y": 90,
39+
"id": 15,
40+
"timeOffset": [timeOffset]
41+
}
42+
]
4343
}
4444
]

‎packages/replay/src/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ export interface EventBuffer {
276276

277277
/**
278278
* Add an event to the event buffer.
279+
* `isCheckout` is true if this is either the very first event, or an event triggered by `checkoutEveryNms`.
279280
*
280281
* Returns a promise that resolves if the event was successfully added, else rejects.
281282
*/

‎packages/replay/src/util/addEvent.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import { logger } from '@sentry/utils';
44
import type { AddEventResult, RecordingEvent, ReplayContainer } from '../types';
55

66
/**
7-
* Add an event to the event buffer
7+
* Add an event to the event buffer.
8+
* `isCheckout` is true if this is either the very first event, or an event triggered by `checkoutEveryNms`.
89
*/
910
export async function addEvent(
1011
replay: ReplayContainer,

‎packages/replay/src/util/handleRecordingEmit.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { logger } from '@sentry/utils';
22

3-
import type { ReplayContainer } from '../replay';
43
import { saveSession } from '../session/saveSession';
5-
import type { RecordingEvent } from '../types';
4+
import type { RecordingEvent, ReplayContainer } from '../types';
65
import { addEvent } from './addEvent';
76

87
type RecordingEmitCallback = (event: RecordingEvent, isCheckout?: boolean) => void;
@@ -13,21 +12,29 @@ type RecordingEmitCallback = (event: RecordingEvent, isCheckout?: boolean) => vo
1312
* Adds to event buffer, and has varying flushing behaviors if the event was a checkout.
1413
*/
1514
export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCallback {
16-
return (event: RecordingEvent, isCheckout?: boolean) => {
15+
let hadFirstEvent = false;
16+
17+
return (event: RecordingEvent, _isCheckout?: boolean) => {
1718
// If this is false, it means session is expired, create and a new session and wait for checkout
1819
if (!replay.checkAndHandleExpiredSession()) {
1920
__DEBUG_BUILD__ && logger.warn('[Replay] Received replay event after session expired.');
2021

2122
return;
2223
}
2324

25+
// `_isCheckout` is only set when the checkout is due to `checkoutEveryNms`
26+
// We also want to treat the first event as a checkout, so we handle this specifically here
27+
const isCheckout = _isCheckout || !hadFirstEvent;
28+
hadFirstEvent = true;
29+
30+
// The handler returns `true` if we do not want to trigger debounced flush, `false` if we want to debounce flush.
2431
replay.addUpdate(() => {
2532
// The session is always started immediately on pageload/init, but for
2633
// error-only replays, it should reflect the most recent checkout
2734
// when an error occurs. Clear any state that happens before this current
2835
// checkout. This needs to happen before `addEvent()` which updates state
2936
// dependent on this reset.
30-
if (replay.recordingMode === 'error' && event.type === 2) {
37+
if (replay.recordingMode === 'error' && isCheckout) {
3138
replay.setInitialState();
3239
}
3340

@@ -37,7 +44,7 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
3744

3845
// Different behavior for full snapshots (type=2), ignore other event types
3946
// See https://github.com/rrweb-io/rrweb/blob/d8f9290ca496712aa1e7d472549480c4e7876594/packages/rrweb/src/types.ts#L16
40-
if (event.type !== 2) {
47+
if (!isCheckout) {
4148
return false;
4249
}
4350

‎packages/replay/test/unit/util/handleRecordingEmit.test.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { getHandleRecordingEmit } from '../../../src/util/handleRecordingEmit';
2+
import { setupReplayContainer } from '../../utils/setupReplayContainer';
3+
import { useFakeTimers } from '../../utils/use-fake-timers';
4+
import { EventType } from '@sentry-internal/rrweb';
5+
import { BASE_TIMESTAMP } from '../..';
6+
import * as SentryAddEvent from '../../../src/util/addEvent';
7+
8+
useFakeTimers();
9+
10+
describe('Unit | util | handleRecordingEmit', () => {
11+
let addEventMock: jest.SpyInstance;
12+
13+
beforeEach(function () {
14+
jest.setSystemTime(BASE_TIMESTAMP);
15+
addEventMock = jest.spyOn(SentryAddEvent, 'addEvent').mockImplementation(async () => {
16+
// Do nothing
17+
});
18+
});
19+
20+
afterEach(function () {
21+
addEventMock.mockReset();
22+
});
23+
24+
it('interprets first event as checkout event', async function () {
25+
const replay = setupReplayContainer({
26+
options: {
27+
errorSampleRate: 0,
28+
sessionSampleRate: 1,
29+
},
30+
});
31+
32+
const handler = getHandleRecordingEmit(replay);
33+
34+
const event = {
35+
type: EventType.FullSnapshot,
36+
data: {
37+
tag: 'test custom',
38+
},
39+
timestamp: BASE_TIMESTAMP + 10,
40+
};
41+
42+
handler(event);
43+
await new Promise(process.nextTick);
44+
45+
expect(addEventMock).toBeCalledTimes(1);
46+
expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true);
47+
48+
handler(event);
49+
await new Promise(process.nextTick);
50+
51+
expect(addEventMock).toBeCalledTimes(2);
52+
expect(addEventMock).toHaveBeenLastCalledWith(replay, event, false);
53+
});
54+
55+
it('interprets any event with isCheckout as checkout', async function () {
56+
const replay = setupReplayContainer({
57+
options: {
58+
errorSampleRate: 0,
59+
sessionSampleRate: 1,
60+
},
61+
});
62+
63+
const handler = getHandleRecordingEmit(replay);
64+
65+
const event = {
66+
type: EventType.IncrementalSnapshot,
67+
data: {
68+
tag: 'test custom',
69+
},
70+
timestamp: BASE_TIMESTAMP + 10,
71+
};
72+
73+
handler(event, true);
74+
await new Promise(process.nextTick);
75+
76+
expect(addEventMock).toBeCalledTimes(1);
77+
expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true);
78+
79+
handler(event, true);
80+
await new Promise(process.nextTick);
81+
82+
expect(addEventMock).toBeCalledTimes(2);
83+
expect(addEventMock).toHaveBeenLastCalledWith(replay, event, true);
84+
});
85+
});

0 commit comments

Comments
 (0)