Skip to content

Commit ef44abc

Browse files
committed
fix(replay): Fix buffered replays creating replay w/o error occuring
Introduced in #7741. This happens when session replay rate is > 0 and the session is unsampled, it calls `stop()` which ends up flushing without checking the recording mode (session vs buffer). Closes #8054
1 parent 5543808 commit ef44abc

File tree

4 files changed

+57
-4
lines changed

4 files changed

+57
-4
lines changed

packages/replay/jest.setup.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,21 @@ function checkCallForSentReplay(
128128
};
129129
}
130130

131+
function getReplayCalls(calls: any[][][]) {
132+
return calls.map(call => {
133+
const arg = call[0];
134+
if (arg.length !== 2) {
135+
return [];
136+
}
137+
138+
if (!arg[1][0].find(({type}: {type: string}) => ['replay_event', 'replay_recording'].includes(type))) {
139+
return [];
140+
}
141+
142+
return [ arg ];
143+
}).filter(Boolean);
144+
}
145+
131146
/**
132147
* Checks all calls to `fetch` and ensures a replay was uploaded by
133148
* checking the `fetch()` request's body.
@@ -143,7 +158,10 @@ const toHaveSentReplay = function (
143158

144159
const expectedKeysLength = expected ? ('sample' in expected ? Object.keys(expected.sample) : Object.keys(expected)).length : 0;
145160

146-
for (const currentCall of calls) {
161+
// Only want calls that send replay events (ignore error events)
162+
const replayCalls = getReplayCalls(calls)
163+
164+
for (const currentCall of replayCalls) {
147165
result = checkCallForSentReplay.call(this, currentCall[0], expected);
148166
if (result.pass) {
149167
break;
@@ -193,7 +211,9 @@ const toHaveLastSentReplay = function (
193211
expected?: SentReplayExpected | { sample: SentReplayExpected; inverse: boolean },
194212
) {
195213
const { calls } = (getCurrentHub().getClient()?.getTransport()?.send as MockTransport).mock;
196-
const lastCall = calls[calls.length - 1]?.[0];
214+
const replayCalls = getReplayCalls(calls)
215+
216+
const lastCall = replayCalls[calls.length - 1]?.[0];
197217

198218
const { results, call, pass } = checkCallForSentReplay.call(this, lastCall, expected);
199219

packages/replay/src/replay.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,9 @@ export class ReplayContainer implements ReplayContainerInterface {
327327
this._debouncedFlush.cancel();
328328
// See comment above re: `_isEnabled`, we "force" a flush, ignoring the
329329
// `_isEnabled` state of the plugin since it was disabled above.
330-
await this._flush({ force: true });
330+
if (this.recordingMode === 'session') {
331+
await this._flush({ force: true });
332+
}
331333

332334
// After flush, destroy event buffer
333335
this.eventBuffer && this.eventBuffer.destroy();

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,21 @@ describe('Integration | errorSampleRate with delayed flush', () => {
229229
});
230230
});
231231

232+
// This tests a regression where we were calling flush indiscriminantly in `stop()`
233+
it('does not upload a replay event if error is not sampled', async () => {
234+
// We are trying to replicate the case where error rate is 0 and session
235+
// rate is > 0, we can't set them both to 0 otherwise
236+
// `_loadAndCheckSession` is not called when initializing the plugin.
237+
replay.stop();
238+
replay['_options']['errorSampleRate'] = 0;
239+
replay['_loadAndCheckSession']();
240+
241+
jest.runAllTimers();
242+
await new Promise(process.nextTick);
243+
expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
244+
expect(replay).not.toHaveLastSentReplay();
245+
});
246+
232247
it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_EXPIRE_DURATION]ms', async () => {
233248
Object.defineProperty(document, 'visibilityState', {
234249
configurable: true,
@@ -664,7 +679,7 @@ describe('Integration | errorSampleRate with delayed flush', () => {
664679
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
665680
await new Promise(process.nextTick);
666681

667-
expect(replay).toHaveLastSentReplay();
682+
expect(replay).not.toHaveLastSentReplay();
668683

669684
// Wait a bit, shortly before session expires
670685
jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000);

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,22 @@ describe('Integration | errorSampleRate', () => {
234234
});
235235
});
236236

237+
// This tests a regression where we were calling flush indiscriminantly in `stop()`
238+
it('does not upload a replay event if error is not sampled', async () => {
239+
// We are trying to replicate the case where error rate is 0 and session
240+
// rate is > 0, we can't set them both to 0 otherwise
241+
// `_loadAndCheckSession` is not called when initializing the plugin.
242+
replay.stop();
243+
replay['_options']['errorSampleRate'] = 0;
244+
replay['_loadAndCheckSession']();
245+
246+
jest.runAllTimers();
247+
await new Promise(process.nextTick);
248+
expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
249+
expect(replay).not.toHaveLastSentReplay();
250+
});
251+
252+
237253
it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_EXPIRE_DURATION]ms', async () => {
238254
Object.defineProperty(document, 'visibilityState', {
239255
configurable: true,

0 commit comments

Comments
 (0)