Skip to content

Commit bf3eb7f

Browse files
authored
ref(replay): Skip events being added too long after initial segment (#8768)
This ensures we ignore any events being added which are after the session max age. Maybe this helps with cases where we send weird replays with a single event much after initial timestamp. I added a test to ensure this does not regress in buffer mode, where recording is continuous and _can_ be "forever".
1 parent bffbaf6 commit bf3eb7f

File tree

3 files changed

+114
-0
lines changed

3 files changed

+114
-0
lines changed

packages/replay/src/util/addEvent.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { logger } from '@sentry/utils';
44

55
import { EventBufferSizeExceededError } from '../eventBuffer/error';
66
import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent, ReplayPluginOptions } from '../types';
7+
import { logInfo } from './log';
78
import { timestampToMs } from './timestamp';
89

910
function isCustomEvent(event: RecordingEvent): event is ReplayFrameEvent {
@@ -39,6 +40,15 @@ export async function addEvent(
3940
return null;
4041
}
4142

43+
// Throw out events that are +60min from the initial timestamp
44+
if (timestampInMs > replay.getContext().initialTimestamp + replay.timeouts.maxSessionLife) {
45+
logInfo(
46+
`[Replay] Skipping event with timestamp ${timestampInMs} because it is after maxSessionLife`,
47+
replay.getOptions()._experiments.traceInternals,
48+
);
49+
return null;
50+
}
51+
4252
try {
4353
if (isCheckout && replay.recordingMode === 'buffer') {
4454
replay.eventBuffer.clear();

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,50 @@ describe('Integration | errorSampleRate', () => {
883883
expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0);
884884
expect(replay.isEnabled()).toBe(false);
885885
});
886+
887+
it('handles very long active buffer session', async () => {
888+
const stepDuration = 10_000;
889+
const steps = 5_000;
890+
891+
jest.setSystemTime(BASE_TIMESTAMP);
892+
893+
expect(replay).not.toHaveLastSentReplay();
894+
895+
let optionsEvent = createOptionsEvent(replay);
896+
897+
for (let i = 1; i <= steps; i++) {
898+
jest.advanceTimersByTime(stepDuration);
899+
optionsEvent = createOptionsEvent(replay);
900+
mockRecord._emitter({ data: { step: i }, timestamp: BASE_TIMESTAMP + stepDuration * i, type: 2 }, true);
901+
mockRecord._emitter({ data: { step: i }, timestamp: BASE_TIMESTAMP + stepDuration * i + 5, type: 3 });
902+
}
903+
904+
expect(replay).not.toHaveLastSentReplay();
905+
906+
expect(replay.isEnabled()).toBe(true);
907+
expect(replay.isPaused()).toBe(false);
908+
expect(replay.recordingMode).toBe('buffer');
909+
910+
// Now capture an error
911+
captureException(new Error('testing'));
912+
await waitForBufferFlush();
913+
914+
expect(replay).toHaveLastSentReplay({
915+
recordingData: JSON.stringify([
916+
{ data: { step: steps }, timestamp: BASE_TIMESTAMP + stepDuration * steps, type: 2 },
917+
optionsEvent,
918+
{ data: { step: steps }, timestamp: BASE_TIMESTAMP + stepDuration * steps + 5, type: 3 },
919+
]),
920+
replayEventPayload: expect.objectContaining({
921+
replay_start_timestamp: (BASE_TIMESTAMP + stepDuration * steps) / 1000,
922+
error_ids: [expect.any(String)],
923+
trace_ids: [],
924+
urls: ['http://localhost/'],
925+
replay_id: expect.any(String),
926+
}),
927+
recordingPayloadHeader: { segment_id: 0 },
928+
});
929+
});
886930
});
887931

888932
/**

packages/replay/test/integration/flush.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ type MockFlush = jest.MockedFunction<ReplayContainer['_flush']>;
2626
type MockRunFlush = jest.MockedFunction<ReplayContainer['_runFlush']>;
2727

2828
const prevLocation = WINDOW.location;
29+
const prevBrowserPerformanceTimeOrigin = SentryUtils.browserPerformanceTimeOrigin;
2930

3031
describe('Integration | flush', () => {
3132
let domHandler: (args: any) => any;
@@ -91,6 +92,11 @@ describe('Integration | flush', () => {
9192
}
9293
mockEventBufferFinish = replay.eventBuffer?.finish as MockEventBufferFinish;
9394
mockEventBufferFinish.mockClear();
95+
96+
Object.defineProperty(SentryUtils, 'browserPerformanceTimeOrigin', {
97+
value: BASE_TIMESTAMP,
98+
writable: true,
99+
});
94100
});
95101

96102
afterEach(async () => {
@@ -102,6 +108,10 @@ describe('Integration | flush', () => {
102108
value: prevLocation,
103109
writable: true,
104110
});
111+
Object.defineProperty(SentryUtils, 'browserPerformanceTimeOrigin', {
112+
value: prevBrowserPerformanceTimeOrigin,
113+
writable: true,
114+
});
105115
});
106116

107117
afterAll(() => {
@@ -224,6 +234,7 @@ describe('Integration | flush', () => {
224234
// flush #5 @ t=25s - debounced flush calls `flush`
225235
// 20s + `flushMinDelay` which is 5 seconds
226236
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
237+
227238
expect(mockFlush).toHaveBeenCalledTimes(5);
228239
expect(mockRunFlush).toHaveBeenCalledTimes(2);
229240
expect(mockSendReplay).toHaveBeenLastCalledWith({
@@ -382,4 +393,53 @@ describe('Integration | flush', () => {
382393

383394
replay.getOptions()._experiments.traceInternals = false;
384395
});
396+
397+
it('logs warning if adding event that is after maxSessionLife', async () => {
398+
replay.getOptions()._experiments.traceInternals = true;
399+
400+
sessionStorage.clear();
401+
clearSession(replay);
402+
replay['_loadAndCheckSession']();
403+
await new Promise(process.nextTick);
404+
jest.setSystemTime(BASE_TIMESTAMP);
405+
406+
replay.eventBuffer!.clear();
407+
408+
// We do not care about this warning here
409+
replay.eventBuffer!.hasCheckout = true;
410+
411+
// Add event that is too long after session start
412+
const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP + MAX_SESSION_LIFE + 100, type: 2 };
413+
mockRecord._emitter(TEST_EVENT);
414+
415+
// no checkout!
416+
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
417+
418+
expect(mockFlush).toHaveBeenCalledTimes(1);
419+
expect(mockSendReplay).toHaveBeenCalledTimes(1);
420+
421+
const replayData = mockSendReplay.mock.calls[0][0];
422+
423+
expect(JSON.parse(replayData.recordingData)).toEqual([
424+
{
425+
type: 5,
426+
timestamp: BASE_TIMESTAMP,
427+
data: {
428+
tag: 'breadcrumb',
429+
payload: {
430+
timestamp: BASE_TIMESTAMP / 1000,
431+
type: 'default',
432+
category: 'console',
433+
data: { logger: 'replay' },
434+
level: 'info',
435+
message: `[Replay] Skipping event with timestamp ${
436+
BASE_TIMESTAMP + MAX_SESSION_LIFE + 100
437+
} because it is after maxSessionLife`,
438+
},
439+
},
440+
},
441+
]);
442+
443+
replay.getOptions()._experiments.traceInternals = false;
444+
});
385445
});

0 commit comments

Comments
 (0)