Skip to content

Commit b51a7a9

Browse files
billyvgbillyvg
committed
fix(replay): Linked errors not resetting session id
This PR fixes a case where we [correctly] tag an error event w/ replay id, but something occurs where the replay event does not end up being flushed. This means the existing session is still in a buffered state, and will keep its session id until a new error event is sampled and a replay is created. When this does happen, we can have a replay with a super long duration (e.g. the time between the two error replays). We now update the session immediately when we tag an error event w/ replay id so that if the replay event does not successfully flush, the session will respect its expiration date.
1 parent c084bd6 commit b51a7a9

File tree

5 files changed

+376
-0
lines changed

5 files changed

+376
-0
lines changed

‎dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/init.js‎

Lines changed: 18 additions & 0 deletions
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 = Sentry.replayIntegration({
5+
flushMinDelay: 200,
6+
flushMaxDelay: 200,
7+
minReplayDuration: 0,
8+
stickySession: true,
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+
});

‎dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/subject.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
document.getElementById('error1').addEventListener('click', () => {
2+
throw new Error('First Error');
3+
});
4+
5+
document.getElementById('error2').addEventListener('click', () => {
6+
throw new Error('Second Error');
7+
});
8+
9+
document.getElementById('click').addEventListener('click', () => {
10+
// Just a click for interaction
11+
});

‎dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/template.html‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<button id="error1">Throw First Error</button>
8+
<button id="error2">Throw Second Error</button>
9+
<button id="click">Click me</button>
10+
</body>
11+
</html>

‎dev-packages/browser-integration-tests/suites/replay/bufferStalledRequests/test.ts‎

Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,322 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../utils/fixtures';
3+
import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers';
4+
import {
5+
getReplaySnapshot,
6+
isReplayEvent,
7+
shouldSkipReplayTest,
8+
waitForReplayRunning,
9+
} from '../../../utils/replayHelpers';
10+
11+
sentryTest(
12+
'buffer mode turns into session mode after interrupting error event ingest, and resumes session on reload',
13+
async ({ getLocalTestUrl, page, browserName }) => {
14+
if (shouldSkipReplayTest() || browserName === 'webkit') {
15+
sentryTest.skip();
16+
}
17+
18+
let errorCount = 0;
19+
let replayCount = 0;
20+
const errorEventIds: string[] = [];
21+
const replayIds: string[] = [];
22+
let firstReplayEventResolved: (value?: unknown) => void = () => {};
23+
// Need TS 5.7 for withResolvers
24+
const firstReplayEventPromise = new Promise(resolve => {
25+
firstReplayEventResolved = resolve;
26+
});
27+
28+
const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true });
29+
30+
await page.route('https://dsn.ingest.sentry.io/**/*', async route => {
31+
const event = envelopeRequestParser(route.request());
32+
33+
// Track error events
34+
if (event && !event.type && event.event_id) {
35+
errorCount++;
36+
errorEventIds.push(event.event_id);
37+
if (event.tags?.replayId) {
38+
replayIds.push(event.tags.replayId as string);
39+
40+
if (errorCount === 1) {
41+
firstReplayEventResolved();
42+
// intentional so that it never resolves, we'll force a reload instead to interrupt the normal flow
43+
await new Promise(resolve => setTimeout(resolve, 100000));
44+
}
45+
}
46+
}
47+
48+
// Track replay events and simulate failure for the first replay
49+
if (event && isReplayEvent(event)) {
50+
replayCount++;
51+
}
52+
53+
// Success for other requests
54+
return route.fulfill({
55+
status: 200,
56+
contentType: 'application/json',
57+
body: JSON.stringify({ id: 'test-id' }),
58+
});
59+
});
60+
61+
await page.goto(url);
62+
63+
// Wait for replay to initialize
64+
await waitForReplayRunning(page);
65+
66+
// Trigger first error - this should change session sampled to "session"
67+
waitForErrorRequest(page);
68+
await page.locator('#error1').click();
69+
await firstReplayEventPromise;
70+
expect(errorCount).toBe(1);
71+
expect(replayCount).toBe(0);
72+
expect(replayIds).toHaveLength(1);
73+
74+
// Get the first session info
75+
const firstSession = await getReplaySnapshot(page);
76+
const firstSessionId = firstSession.session?.id;
77+
expect(firstSessionId).toBeDefined();
78+
expect(firstSession.session?.sampled).toBe('session'); // Should be marked as 'session'
79+
expect(firstSession.recordingMode).toBe('buffer'); // But still in buffer mode
80+
81+
await page.reload();
82+
const secondSession = await getReplaySnapshot(page);
83+
expect(secondSession.session?.sampled).toBe('session');
84+
expect(secondSession.recordingMode).toBe('session'); // this turns to "session" because of `sampled` value being "session"
85+
expect(secondSession.session?.id).toBe(firstSessionId);
86+
expect(secondSession.session?.segmentId).toBe(0);
87+
},
88+
);
89+
90+
sentryTest(
91+
'buffer mode turns into session mode after interrupting replay flush, and resumes session on reload',
92+
async ({ getLocalTestUrl, page, browserName }) => {
93+
if (shouldSkipReplayTest() || browserName === 'webkit') {
94+
sentryTest.skip();
95+
}
96+
97+
let errorCount = 0;
98+
let replayCount = 0;
99+
const errorEventIds: string[] = [];
100+
const replayIds: string[] = [];
101+
let firstReplayEventResolved: (value?: unknown) => void = () => {};
102+
// Need TS 5.7 for withResolvers
103+
const firstReplayEventPromise = new Promise(resolve => {
104+
firstReplayEventResolved = resolve;
105+
});
106+
107+
const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true });
108+
109+
await page.route('https://dsn.ingest.sentry.io/**/*', async route => {
110+
const event = envelopeRequestParser(route.request());
111+
112+
// Track error events
113+
if (event && !event.type && event.event_id) {
114+
errorCount++;
115+
errorEventIds.push(event.event_id);
116+
if (event.tags?.replayId) {
117+
replayIds.push(event.tags.replayId as string);
118+
}
119+
}
120+
121+
// Track replay events and simulate failure for the first replay
122+
if (event && isReplayEvent(event)) {
123+
replayCount++;
124+
if (replayCount === 1) {
125+
firstReplayEventResolved();
126+
// intentional so that it never resolves, we'll force a reload instead to interrupt the normal flow
127+
await new Promise(resolve => setTimeout(resolve, 100000));
128+
}
129+
}
130+
131+
// Success for other requests
132+
return route.fulfill({
133+
status: 200,
134+
contentType: 'application/json',
135+
body: JSON.stringify({ id: 'test-id' }),
136+
});
137+
});
138+
139+
await page.goto(url);
140+
141+
// Wait for replay to initialize
142+
await waitForReplayRunning(page);
143+
144+
// Trigger first error - this should change session sampled to "session"
145+
await page.locator('#error1').click();
146+
await firstReplayEventPromise;
147+
expect(errorCount).toBe(1);
148+
expect(replayCount).toBe(1);
149+
expect(replayIds).toHaveLength(1);
150+
151+
// Get the first session info
152+
const firstSession = await getReplaySnapshot(page);
153+
const firstSessionId = firstSession.session?.id;
154+
expect(firstSessionId).toBeDefined();
155+
expect(firstSession.session?.sampled).toBe('session'); // Should be marked as 'session'
156+
expect(firstSession.recordingMode).toBe('buffer'); // But still in buffer mode
157+
158+
await page.reload();
159+
const secondSession = await getReplaySnapshot(page);
160+
expect(secondSession.session?.sampled).toBe('session');
161+
expect(secondSession.recordingMode).toBe('session'); // this turns to "session" because of `sampled` value being "session"
162+
expect(secondSession.session?.id).toBe(firstSessionId);
163+
expect(secondSession.session?.segmentId).toBe(1);
164+
},
165+
);
166+
167+
sentryTest(
168+
'starts a new session after interrupting replay flush and session "expires"',
169+
async ({ getLocalTestUrl, page, browserName }) => {
170+
if (shouldSkipReplayTest() || browserName === 'webkit') {
171+
sentryTest.skip();
172+
}
173+
174+
let errorCount = 0;
175+
let replayCount = 0;
176+
const errorEventIds: string[] = [];
177+
const replayIds: string[] = [];
178+
let firstReplayEventResolved: (value?: unknown) => void = () => {};
179+
// Need TS 5.7 for withResolvers
180+
const firstReplayEventPromise = new Promise(resolve => {
181+
firstReplayEventResolved = resolve;
182+
});
183+
184+
const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true });
185+
186+
await page.route('https://dsn.ingest.sentry.io/**/*', async route => {
187+
const event = envelopeRequestParser(route.request());
188+
189+
// Track error events
190+
if (event && !event.type && event.event_id) {
191+
errorCount++;
192+
errorEventIds.push(event.event_id);
193+
if (event.tags?.replayId) {
194+
replayIds.push(event.tags.replayId as string);
195+
}
196+
}
197+
198+
// Track replay events and simulate failure for the first replay
199+
if (event && isReplayEvent(event)) {
200+
replayCount++;
201+
if (replayCount === 1) {
202+
firstReplayEventResolved();
203+
// intentional so that it never resolves, we'll force a reload instead to interrupt the normal flow
204+
await new Promise(resolve => setTimeout(resolve, 100000));
205+
}
206+
}
207+
208+
// Success for other requests
209+
return route.fulfill({
210+
status: 200,
211+
contentType: 'application/json',
212+
body: JSON.stringify({ id: 'test-id' }),
213+
});
214+
});
215+
216+
await page.goto(url);
217+
218+
// Wait for replay to initialize
219+
await waitForReplayRunning(page);
220+
221+
// Trigger first error - this should change session sampled to "session"
222+
await page.locator('#error1').click();
223+
await firstReplayEventPromise;
224+
expect(errorCount).toBe(1);
225+
expect(replayCount).toBe(1);
226+
expect(replayIds).toHaveLength(1);
227+
228+
// Get the first session info
229+
const firstSession = await getReplaySnapshot(page);
230+
const firstSessionId = firstSession.session?.id;
231+
expect(firstSessionId).toBeDefined();
232+
expect(firstSession.session?.sampled).toBe('session'); // Should be marked as 'session'
233+
expect(firstSession.recordingMode).toBe('buffer'); // But still in buffer mode
234+
235+
// Now expire the session by manipulating session storage
236+
// Simulate session expiry by setting lastActivity to a time in the past
237+
await page.evaluate(() => {
238+
const replayIntegration = (window as any).Replay;
239+
const replay = replayIntegration['_replay'];
240+
241+
// Set session as expired (15 minutes ago)
242+
if (replay.session) {
243+
const fifteenMinutesAgo = Date.now() - 15 * 60 * 1000;
244+
replay.session.lastActivity = fifteenMinutesAgo;
245+
replay.session.started = fifteenMinutesAgo;
246+
247+
// Also update session storage if sticky sessions are enabled
248+
const sessionKey = 'sentryReplaySession';
249+
const sessionData = sessionStorage.getItem(sessionKey);
250+
if (sessionData) {
251+
const session = JSON.parse(sessionData);
252+
session.lastActivity = fifteenMinutesAgo;
253+
session.started = fifteenMinutesAgo;
254+
sessionStorage.setItem(sessionKey, JSON.stringify(session));
255+
}
256+
}
257+
});
258+
259+
await page.reload();
260+
const secondSession = await getReplaySnapshot(page);
261+
expect(secondSession.session?.sampled).toBe('buffer');
262+
expect(secondSession.recordingMode).toBe('buffer');
263+
expect(secondSession.session?.id).not.toBe(firstSessionId);
264+
expect(secondSession.session?.segmentId).toBe(0);
265+
},
266+
);
267+
268+
sentryTest(
269+
'[buffer-mode] marks session as sampled immediately when error is sampled',
270+
async ({ getLocalTestUrl, page, browserName }) => {
271+
if (shouldSkipReplayTest() || browserName === 'webkit') {
272+
sentryTest.skip();
273+
}
274+
275+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
276+
return route.fulfill({
277+
status: 200,
278+
contentType: 'application/json',
279+
body: JSON.stringify({ id: 'test-id' }),
280+
});
281+
});
282+
283+
const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true });
284+
await page.goto(url);
285+
286+
// Wait for replay to initialize
287+
await new Promise(resolve => setTimeout(resolve, 100));
288+
289+
// Verify initial state - buffer mode, not sampled
290+
const initialSession = await getReplaySnapshot(page);
291+
expect(initialSession.recordingMode).toBe('buffer');
292+
expect(initialSession.session?.sampled).toBe('buffer');
293+
expect(initialSession.session?.segmentId).toBe(0);
294+
295+
// Trigger error which should immediately mark session as sampled
296+
const reqErrorPromise = waitForErrorRequest(page);
297+
await page.locator('#error1').click();
298+
299+
// Check session state BEFORE waiting for error to be sent
300+
// The session should already be marked as 'session' synchronously
301+
const duringErrorProcessing = await getReplaySnapshot(page);
302+
expect(duringErrorProcessing.session?.sampled).toBe('session');
303+
expect(duringErrorProcessing.recordingMode).toBe('buffer'); // Still in buffer recording mode
304+
305+
await reqErrorPromise;
306+
307+
// After error is sent, verify state is still correct
308+
const afterError = await getReplaySnapshot(page);
309+
expect(afterError.session?.sampled).toBe('session');
310+
expect(afterError.recordingMode).toBe('session');
311+
312+
// Verify the session was persisted to sessionStorage (if sticky sessions enabled)
313+
const sessionData = await page.evaluate(() => {
314+
const sessionKey = 'sentryReplaySession';
315+
const data = sessionStorage.getItem(sessionKey);
316+
return data ? JSON.parse(data) : null;
317+
});
318+
319+
expect(sessionData).toBeDefined();
320+
expect(sessionData.sampled).toBe('session');
321+
},
322+
);

‎packages/replay-internal/src/coreHandlers/handleGlobalEvent.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Event, EventHint } from '@sentry/core';
22
import { DEBUG_BUILD } from '../debug-build';
3+
import { saveSession } from '../session/saveSession';
34
import type { ReplayContainer } from '../types';
45
import { isErrorEvent, isFeedbackEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils';
56
import { isRrwebError } from '../util/isRrwebError';
@@ -69,6 +70,19 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even
6970
event.tags = { ...event.tags, replayId: replay.getSessionId() };
7071
}
7172

73+
// If we sampled this error in buffer mode, immediately mark the session as "sampled"
74+
// by changing the sampled state from 'buffer' to 'session'. Otherwise, if the application is interrupted before `afterSendEvent` occurs, then the session would remain as "buffer" but we have an error event that is tagged with a replay id. This could end up creating replays w/ excessive durations because of the linked error.
75+
if (isErrorEventSampled && replay.recordingMode === 'buffer') {
76+
const session = replay.session;
77+
if (session?.sampled === 'buffer') {
78+
session.sampled = 'session';
79+
// Save the session if sticky sessions are enabled to persist the state change
80+
if (replay.getOptions().stickySession) {
81+
saveSession(session);
82+
}
83+
}
84+
}
85+
7286
return event;
7387
},
7488
{ id: 'Replay' },

0 commit comments

Comments
 (0)