Skip to content

Commit 56d3a74

Browse files
authored
feat(replay): Stop recording when retry fails (#6765)
1 parent 57de684 commit 56d3a74

File tree

2 files changed

+20
-3
lines changed

2 files changed

+20
-3
lines changed

packages/replay/src/replay.ts

+8-3
Original file line numberDiff line numberDiff line change
@@ -824,10 +824,16 @@ export class ReplayContainer implements ReplayContainerInterface {
824824
timestamp: new Date().getTime(),
825825
});
826826
} catch (err) {
827+
this._handleException(err);
828+
827829
if (err instanceof RateLimitError) {
828830
this._handleRateLimit(err.rateLimits);
831+
return;
829832
}
830-
this._handleException(err);
833+
834+
// This means we retried 3 times, and all of them failed
835+
// In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments
836+
this.stop();
831837
}
832838
}
833839

@@ -837,8 +843,7 @@ export class ReplayContainer implements ReplayContainerInterface {
837843
*/
838844
private _flush: () => Promise<void> = async () => {
839845
if (!this._isEnabled) {
840-
// This is just a precaution, there should be no listeners that would
841-
// cause a flush.
846+
// This can happen if e.g. the replay was stopped because of exceeding the retry limit
842847
return;
843848
}
844849

packages/replay/test/integration/sendReplayEvent.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -428,5 +428,17 @@ describe('Integration | sendReplayEvent', () => {
428428

429429
// segmentId increases despite error
430430
expect(replay.session?.segmentId).toBe(1);
431+
432+
// Replay should be completely stopped now
433+
expect(replay.isEnabled()).toBe(false);
434+
435+
// Events are ignored now, because we stopped
436+
mockRecord._emitter(TEST_EVENT);
437+
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
438+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(4);
431439
});
440+
441+
// NOTE: If you add a test after the last one, make sure to adjust the test setup
442+
// As this ends with a `stopped()` replay, which may prevent future tests from working
443+
// Sadly, fixing this turned out to be much more annoying than expected, so leaving this warning here for now
432444
});

0 commit comments

Comments
 (0)