Skip to content

Commit 6022997

Browse files
mydeamydea
authored
ref(replay): Try to make flush fully sync (#6859)
1 parent 1601a24 commit 6022997

File tree

4 files changed

+133
-72
lines changed

4 files changed

+133
-72
lines changed

‎packages/replay/src/eventBuffer.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,11 @@ class EventBufferArray implements EventBuffer {
7171
return;
7272
}
7373

74-
public finish(): Promise<string> {
75-
return new Promise<string>(resolve => {
76-
resolve(this._finish());
77-
});
78-
}
79-
80-
public finishImmediate(): string {
81-
return this._finish();
74+
public async finish(): Promise<string> {
75+
return this.finishSync();
8276
}
8377

84-
private _finish(): string {
78+
public finishSync(): string {
8579
// Make a copy of the events array reference and immediately clear the
8680
// events member so that we do not lose new events while uploading
8781
// attachment.
@@ -169,7 +163,7 @@ export class EventBufferCompressionWorker implements EventBuffer {
169163
/**
170164
* Finish the event buffer and return the pending events.
171165
*/
172-
public finishImmediate(): string {
166+
public finishSync(): string {
173167
const events = this._pendingEvents;
174168

175169
// Ensure worker is still in a good state and disregard the result

‎packages/replay/src/replay.ts

Lines changed: 116 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable max-lines */ // TODO: We might want to split this file up
22
import { addGlobalEventProcessor, captureException, getCurrentHub } from '@sentry/core';
3-
import type { Breadcrumb, ReplayRecordingData, ReplayRecordingMode } from '@sentry/types';
3+
import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types';
44
import type { RateLimits } from '@sentry/utils';
55
import { addInstrumentationHandler, disabledUntil, logger } from '@sentry/utils';
66
import { EventType, record } from 'rrweb';
@@ -625,7 +625,7 @@ export class ReplayContainer implements ReplayContainerInterface {
625625
// Send replay when the page/tab becomes hidden. There is no reason to send
626626
// replay if it becomes visible, since no actions we care about were done
627627
// while it was hidden
628-
this._conditionalFlush({ finishImmediate: true });
628+
this._conditionalFlush({ sync: true });
629629
}
630630

631631
/**
@@ -756,8 +756,8 @@ export class ReplayContainer implements ReplayContainerInterface {
756756
* Page is likely to unload so need to bypass debounce completely and
757757
* synchronously retrieve pending events from buffer and send request asap.
758758
*/
759-
if (options.finishImmediate) {
760-
void this._runFlush(options);
759+
if (options.sync) {
760+
this._flushSync();
761761
return;
762762
}
763763

@@ -804,81 +804,143 @@ export class ReplayContainer implements ReplayContainerInterface {
804804
*
805805
* Should never be called directly, only by `flush`
806806
*/
807-
private async _runFlush(options: FlushOptions = {}): Promise<void> {
808-
if (!this.session || !this.eventBuffer) {
809-
__DEBUG_BUILD__ && logger.error('[Replay] No session or eventBuffer found to flush.');
810-
return;
811-
}
812-
807+
private async _runFlush(): Promise<void> {
813808
try {
814-
this._debouncedFlush.cancel();
809+
const flushData = this._prepareFlush();
810+
811+
if (!flushData) {
812+
return;
813+
}
815814

816-
const promises: Promise<any>[] = [];
815+
const { promises, replayId, segmentId, eventContext, eventBuffer, session } = flushData;
817816

818-
promises.push(this._addPerformanceEntries());
817+
// NOTE: Be mindful that nothing after this point (the first `await`)
818+
// will run after when the page is unloaded.
819+
await Promise.all(promises);
819820

820-
// Do not continue if there are no pending events in buffer
821-
if (!this.eventBuffer || !this.eventBuffer.pendingLength) {
821+
// This can be empty due to blur events calling `runFlush` directly. In
822+
// the case where we have a snapshot checkout and a blur event
823+
// happening near the same time, the blur event can end up emptying the
824+
// buffer even if snapshot happens first.
825+
if (!eventBuffer.pendingLength) {
822826
return;
823827
}
824828

825-
// Only attach memory entry if eventBuffer is not empty
826-
promises.push(addMemoryEntry(this));
829+
// This empties the event buffer regardless of outcome of sending replay
830+
const recordingData = await eventBuffer.finish();
827831

828-
// NOTE: Copy values from instance members, as it's possible they could
829-
// change before the flush finishes.
830-
const replayId = this.session.id;
831-
const eventContext = this._popEventContext();
832-
// Always increment segmentId regardless of outcome of sending replay
833-
const segmentId = this.session.segmentId++;
834-
835-
// Save session (new segment id) after we save flush data assuming either
836-
// 1) request succeeds or 2) it fails or never happens, in which case we
837-
// need to retry this segment.
838-
this._maybeSaveSession();
832+
await sendReplay({
833+
replayId,
834+
recordingData,
835+
segmentId,
836+
includeReplayStartTimestamp: segmentId === 0,
837+
eventContext,
838+
session,
839+
options: this.getOptions(),
840+
timestamp: new Date().getTime(),
841+
});
842+
} catch (err) {
843+
this._handleSendError(err);
844+
}
845+
}
839846

840-
let recordingData: ReplayRecordingData;
847+
/**
848+
* Flush event buffer synchonously.
849+
* This is necessary e.g. when running flush on page unload or similar.
850+
*/
851+
private _flushSync(): void {
852+
try {
853+
const flushData = this._prepareFlush();
841854

842-
if (options.finishImmediate && this.eventBuffer.pendingLength) {
843-
recordingData = this.eventBuffer.finishImmediate();
844-
} else {
845-
// NOTE: Be mindful that nothing after this point (the first `await`)
846-
// will run after when the page is unloaded.
847-
await Promise.all(promises);
848-
849-
// This can be empty due to blur events calling `runFlush` directly. In
850-
// the case where we have a snapshot checkout and a blur event
851-
// happening near the same time, the blur event can end up emptying the
852-
// buffer even if snapshot happens first.
853-
if (!this.eventBuffer.pendingLength) {
854-
return;
855-
}
856-
// This empties the event buffer regardless of outcome of sending replay
857-
recordingData = await this.eventBuffer.finish();
855+
if (!flushData) {
856+
return;
858857
}
859858

860-
await sendReplay({
859+
const { replayId, segmentId, eventContext, eventBuffer, session } = flushData;
860+
861+
const recordingData = eventBuffer.finishSync();
862+
863+
sendReplay({
861864
replayId,
862865
recordingData,
863866
segmentId,
864867
includeReplayStartTimestamp: segmentId === 0,
865868
eventContext,
866-
session: this.session,
869+
session,
867870
options: this.getOptions(),
868871
timestamp: new Date().getTime(),
872+
}).catch(err => {
873+
this._handleSendError(err);
869874
});
870875
} catch (err) {
871-
this._handleException(err);
876+
this._handleSendError(err);
877+
}
878+
}
872879

873-
if (err instanceof RateLimitError) {
874-
this._handleRateLimit(err.rateLimits);
875-
return;
880+
/** Prepare flush data */
881+
private _prepareFlush():
882+
| {
883+
replayId: string;
884+
eventContext: PopEventContext;
885+
segmentId: number;
886+
promises: Promise<unknown>[];
887+
eventBuffer: EventBuffer;
888+
session: Session;
876889
}
890+
| undefined {
891+
if (!this.session || !this.eventBuffer) {
892+
__DEBUG_BUILD__ && logger.error('[Replay] No session or eventBuffer found to flush.');
893+
return;
894+
}
877895

878-
// This means we retried 3 times, and all of them failed
879-
// In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments
880-
this.stop();
896+
this._debouncedFlush.cancel();
897+
898+
const promises: Promise<unknown>[] = [];
899+
900+
promises.push(this._addPerformanceEntries());
901+
902+
// Do not continue if there are no pending events in buffer
903+
if (!this.eventBuffer || !this.eventBuffer.pendingLength) {
904+
return;
881905
}
906+
907+
// Only attach memory entry if eventBuffer is not empty
908+
promises.push(addMemoryEntry(this));
909+
910+
// NOTE: Copy values from instance members, as it's possible they could
911+
// change before the flush finishes.
912+
const replayId = this.session.id;
913+
const eventContext = this._popEventContext();
914+
// Always increment segmentId regardless of outcome of sending replay
915+
const segmentId = this.session.segmentId++;
916+
917+
// Save session (new segment id) after we save flush data assuming either
918+
// 1) request succeeds or 2) it fails or never happens, in which case we
919+
// need to retry this segment.
920+
this._maybeSaveSession();
921+
922+
return {
923+
replayId,
924+
eventContext,
925+
segmentId,
926+
promises,
927+
eventBuffer: this.eventBuffer,
928+
session: this.session,
929+
};
930+
}
931+
932+
/** Handle an error when sending a replay. */
933+
private _handleSendError(error: unknown): void {
934+
this._handleException(error);
935+
936+
if (error instanceof RateLimitError) {
937+
this._handleRateLimit(error.rateLimits);
938+
return;
939+
}
940+
941+
// This means we retried 3 times, and all of them failed
942+
// In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments
943+
this.stop();
882944
}
883945

884946
/**

‎packages/replay/src/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export interface FlushOptions {
1313
* (e.g. worker calls). This is not directly related to `flushImmediate` which
1414
* skips the debounced flush.
1515
*/
16-
finishImmediate?: boolean;
16+
sync?: boolean;
1717
}
1818

1919
export interface SendReplayData {
@@ -254,7 +254,7 @@ export interface EventBuffer {
254254
/**
255255
* Clears and synchronously returns the pending contents of the buffer. This means no compression.
256256
*/
257-
finishImmediate(): string;
257+
finishSync(): string;
258258
}
259259

260260
export type AddUpdateCallback = () => boolean | void;

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,23 +108,28 @@ describe('Integration | flush', () => {
108108
});
109109

110110
it('flushes after each blur event', async () => {
111+
// @ts-ignore privaye API
112+
const mockFlushSync = jest.spyOn(replay, '_flushSync');
113+
111114
// blur events cause an immediate flush that bypass the debounced flush
112115
// function and skip any async workers
113-
expect(mockRunFlush).toHaveBeenCalledTimes(0);
116+
expect(mockFlushSync).toHaveBeenCalledTimes(0);
114117
WINDOW.dispatchEvent(new Event('blur'));
115-
expect(mockRunFlush).toHaveBeenCalledTimes(1);
118+
expect(mockFlushSync).toHaveBeenCalledTimes(1);
116119
WINDOW.dispatchEvent(new Event('blur'));
117-
expect(mockRunFlush).toHaveBeenCalledTimes(2);
120+
expect(mockFlushSync).toHaveBeenCalledTimes(2);
118121
WINDOW.dispatchEvent(new Event('blur'));
119-
expect(mockRunFlush).toHaveBeenCalledTimes(3);
122+
expect(mockFlushSync).toHaveBeenCalledTimes(3);
120123
WINDOW.dispatchEvent(new Event('blur'));
121-
expect(mockRunFlush).toHaveBeenCalledTimes(4);
124+
expect(mockFlushSync).toHaveBeenCalledTimes(4);
122125

126+
expect(mockRunFlush).toHaveBeenCalledTimes(0);
123127
expect(mockFlush).toHaveBeenCalledTimes(0);
124128

125129
jest.runAllTimers();
126130
await new Promise(process.nextTick);
127-
expect(mockRunFlush).toHaveBeenCalledTimes(4);
131+
expect(mockFlushSync).toHaveBeenCalledTimes(4);
132+
expect(mockRunFlush).toHaveBeenCalledTimes(0);
128133
});
129134

130135
it('long first flush enqueues following events', async () => {

0 commit comments

Comments
 (0)