Skip to content

Commit 5f68d4d

Browse files
authored
fix(replay): Fix onError sampling when loading an expired buffered session (#13962)
This fixes a bug where an older, saved session (that has a `previousSessionId`, i.e. session was recorded and expired) would cause buffered replays to not send when an error happens. This is because the session start timestamp would never update, causing our flush logic to skip sending the replay due to incorrect replay duration calculation.
1 parent 8249a5e commit 5f68d4d

File tree

3 files changed

+95
-11
lines changed

3 files changed

+95
-11
lines changed

.size-limit.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ module.exports = [
8686
path: 'packages/browser/build/npm/esm/index.js',
8787
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'),
8888
gzip: true,
89-
limit: '91 KB',
89+
limit: '95 KB',
9090
},
9191
{
9292
name: '@sentry/browser (incl. Tracing, Replay, Feedback, metrics)',

packages/replay-internal/src/util/handleRecordingEmit.ts

+10-10
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,6 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
7171
// only be added for checkouts
7272
addSettingsEvent(replay, isCheckout);
7373

74-
// If there is a previousSessionId after a full snapshot occurs, then
75-
// the replay session was started due to session expiration. The new session
76-
// is started before triggering a new checkout and contains the id
77-
// of the previous session. Do not immediately flush in this case
78-
// to avoid capturing only the checkout and instead the replay will
79-
// be captured if they perform any follow-up actions.
80-
if (session && session.previousSessionId) {
81-
return true;
82-
}
83-
8474
// When in buffer mode, make sure we adjust the session started date to the current earliest event of the buffer
8575
// this should usually be the timestamp of the checkout event, but to be safe...
8676
if (replay.recordingMode === 'buffer' && session && replay.eventBuffer) {
@@ -97,6 +87,16 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
9787
}
9888
}
9989

90+
// If there is a previousSessionId after a full snapshot occurs, then
91+
// the replay session was started due to session expiration. The new session
92+
// is started before triggering a new checkout and contains the id
93+
// of the previous session. Do not immediately flush in this case
94+
// to avoid capturing only the checkout and instead the replay will
95+
// be captured if they perform any follow-up actions.
96+
if (session && session.previousSessionId) {
97+
return true;
98+
}
99+
100100
if (replay.recordingMode === 'session') {
101101
// If the full snapshot is due to an initial load, we will not have
102102
// a previous session ID. In this case, we want to buffer events

packages/replay-internal/test/integration/errorSampleRate.test.ts

+84
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,90 @@ describe('Integration | errorSampleRate', () => {
148148
});
149149
});
150150

151+
it('loads an old session with a previousSessionId set and can send buffered replay', async () => {
152+
vi.mock('../../src/session/fetchSession', () => ({
153+
fetchSession: () => ({
154+
id: 'newreplayid',
155+
started: BASE_TIMESTAMP,
156+
lastActivity: BASE_TIMESTAMP,
157+
segmentId: 0,
158+
sampled: 'buffer',
159+
previousSessionId: 'previoussessionid',
160+
}),
161+
}));
162+
163+
const ADVANCED_TIME = 86400000;
164+
const optionsEvent = createOptionsEvent(replay);
165+
166+
expect(replay.session.started).toBe(BASE_TIMESTAMP);
167+
168+
// advance time to make sure replay duration is invalid
169+
vi.advanceTimersByTime(ADVANCED_TIME);
170+
171+
// full snapshot should update session start time
172+
mockRecord.takeFullSnapshot(true);
173+
expect(replay.session.started).toBe(BASE_TIMESTAMP + ADVANCED_TIME);
174+
expect(replay.recordingMode).toBe('buffer');
175+
176+
// advance so we can flush
177+
vi.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
178+
179+
captureException(new Error('testing'));
180+
await vi.advanceTimersToNextTimerAsync();
181+
182+
// Converts to session mode
183+
expect(replay.recordingMode).toBe('session');
184+
expect(replay).toHaveLastSentReplay({
185+
recordingPayloadHeader: { segment_id: 0 },
186+
replayEventPayload: expect.objectContaining({
187+
replay_type: 'buffer',
188+
}),
189+
recordingData: JSON.stringify([
190+
{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + ADVANCED_TIME, type: 2 },
191+
{ ...optionsEvent, timestamp: BASE_TIMESTAMP + ADVANCED_TIME },
192+
]),
193+
});
194+
195+
// capture next event
196+
domHandler({
197+
name: 'click',
198+
event: new Event('click'),
199+
});
200+
201+
vi.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
202+
await vi.advanceTimersToNextTimerAsync();
203+
204+
expect(replay).toHaveLastSentReplay({
205+
recordingPayloadHeader: { segment_id: 1 },
206+
replayEventPayload: expect.objectContaining({
207+
// We don't change replay_type as it starts in buffer mode and that's
208+
// what we're interested in, even though recordingMode changes to
209+
// 'session'
210+
replay_type: 'buffer',
211+
}),
212+
recordingData: JSON.stringify([
213+
// There's a new checkout because we convert to session mode
214+
{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + ADVANCED_TIME + DEFAULT_FLUSH_MIN_DELAY, type: 2 },
215+
{
216+
type: 5,
217+
timestamp: BASE_TIMESTAMP + ADVANCED_TIME + DEFAULT_FLUSH_MIN_DELAY,
218+
data: {
219+
tag: 'breadcrumb',
220+
payload: {
221+
timestamp: (BASE_TIMESTAMP + ADVANCED_TIME + DEFAULT_FLUSH_MIN_DELAY) / 1000,
222+
type: 'default',
223+
category: 'ui.click',
224+
message: '<unknown>',
225+
data: {},
226+
},
227+
},
228+
},
229+
]),
230+
});
231+
vi.unmock('../../src/session/fetchSession');
232+
await waitForFlush();
233+
});
234+
151235
it('manually flushes replay and does not continue to record', async () => {
152236
const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
153237
mockRecord._emitter(TEST_EVENT);

0 commit comments

Comments
 (0)