Skip to content

Commit 5044e6b

Browse files
authored
ref(replay): Log warning if trying to flush initial segment without checkout (#8748)
We keep track of the eventBuffer contains a checkout, and if not, log a warning when trying to flush the initial segment. If this turns out to be happening, we may think about aborting flushing or similar in this case. But for now, we can gather data.
1 parent 4c6f62e commit 5044e6b

File tree

10 files changed

+151
-24
lines changed

10 files changed

+151
-24
lines changed

packages/replay/src/eventBuffer/EventBufferArray.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,16 @@ import { EventBufferSizeExceededError } from './error';
1010
export class EventBufferArray implements EventBuffer {
1111
/** All the events that are buffered to be sent. */
1212
public events: RecordingEvent[];
13+
14+
/** @inheritdoc */
15+
public hasCheckout: boolean;
16+
1317
private _totalSize: number;
1418

1519
public constructor() {
1620
this.events = [];
1721
this._totalSize = 0;
22+
this.hasCheckout = false;
1823
}
1924

2025
/** @inheritdoc */
@@ -59,6 +64,7 @@ export class EventBufferArray implements EventBuffer {
5964
public clear(): void {
6065
this.events = [];
6166
this._totalSize = 0;
67+
this.hasCheckout = false;
6268
}
6369

6470
/** @inheritdoc */

packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { WorkerHandler } from './WorkerHandler';
1111
* Exported only for testing.
1212
*/
1313
export class EventBufferCompressionWorker implements EventBuffer {
14+
/** @inheritdoc */
15+
public hasCheckout: boolean;
16+
1417
private _worker: WorkerHandler;
1518
private _earliestTimestamp: number | null;
1619
private _totalSize;
@@ -19,6 +22,7 @@ export class EventBufferCompressionWorker implements EventBuffer {
1922
this._worker = new WorkerHandler(worker);
2023
this._earliestTimestamp = null;
2124
this._totalSize = 0;
25+
this.hasCheckout = false;
2226
}
2327

2428
/** @inheritdoc */
@@ -78,6 +82,8 @@ export class EventBufferCompressionWorker implements EventBuffer {
7882
public clear(): void {
7983
this._earliestTimestamp = null;
8084
this._totalSize = 0;
85+
this.hasCheckout = false;
86+
8187
// We do not wait on this, as we assume the order of messages is consistent for the worker
8288
void this._worker.postMessage('clear');
8389
}

packages/replay/src/eventBuffer/EventBufferProxy.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ export class EventBufferProxy implements EventBuffer {
3535
return this._used.hasEvents;
3636
}
3737

38+
/** @inheritdoc */
39+
public get hasCheckout(): boolean {
40+
return this._used.hasCheckout;
41+
}
42+
/** @inheritdoc */
43+
public set hasCheckout(value: boolean) {
44+
this._used.hasCheckout = value;
45+
}
46+
3847
/** @inheritDoc */
3948
public destroy(): void {
4049
this._fallback.destroy();

packages/replay/src/replay.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import { debounce } from './util/debounce';
4747
import { getHandleRecordingEmit } from './util/handleRecordingEmit';
4848
import { isExpired } from './util/isExpired';
4949
import { isSessionExpired } from './util/isSessionExpired';
50-
import { logInfo } from './util/log';
50+
import { logInfo, logInfoNextTick } from './util/log';
5151
import { sendReplay } from './util/sendReplay';
5252
import type { SKIPPED } from './util/throttle';
5353
import { throttle, THROTTLED } from './util/throttle';
@@ -250,7 +250,10 @@ export class ReplayContainer implements ReplayContainerInterface {
250250
this.recordingMode = 'buffer';
251251
}
252252

253-
logInfo(`[Replay] Starting replay in ${this.recordingMode} mode`, this._options._experiments.traceInternals);
253+
logInfoNextTick(
254+
`[Replay] Starting replay in ${this.recordingMode} mode`,
255+
this._options._experiments.traceInternals,
256+
);
254257

255258
this._initializeRecording();
256259
}
@@ -271,7 +274,7 @@ export class ReplayContainer implements ReplayContainerInterface {
271274
throw new Error('Replay buffering is in progress, call `flush()` to save the replay');
272275
}
273276

274-
logInfo('[Replay] Starting replay in session mode', this._options._experiments.traceInternals);
277+
logInfoNextTick('[Replay] Starting replay in session mode', this._options._experiments.traceInternals);
275278

276279
const previousSessionId = this.session && this.session.id;
277280

@@ -300,7 +303,7 @@ export class ReplayContainer implements ReplayContainerInterface {
300303
throw new Error('Replay recording is already in progress');
301304
}
302305

303-
logInfo('[Replay] Starting replay in buffer mode', this._options._experiments.traceInternals);
306+
logInfoNextTick('[Replay] Starting replay in buffer mode', this._options._experiments.traceInternals);
304307

305308
const previousSessionId = this.session && this.session.id;
306309

@@ -1151,6 +1154,12 @@ export class ReplayContainer implements ReplayContainerInterface {
11511154
return;
11521155
}
11531156

1157+
const eventBuffer = this.eventBuffer;
1158+
if (eventBuffer && this.session.segmentId === 0 && !eventBuffer.hasCheckout) {
1159+
logInfo('[Replay] Flushing initial segment without checkout.', this._options._experiments.traceInternals);
1160+
// TODO FN: Evaluate if we want to stop here, or remove this again?
1161+
}
1162+
11541163
// this._flushLock acts as a lock so that future calls to `_flush()`
11551164
// will be blocked until this promise resolves
11561165
if (!this._flushLock) {

packages/replay/src/session/fetchSession.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { REPLAY_SESSION_KEY, WINDOW } from '../constants';
22
import type { Session } from '../types';
33
import { hasSessionStorage } from '../util/hasSessionStorage';
4-
import { logInfo } from '../util/log';
4+
import { logInfoNextTick } from '../util/log';
55
import { makeSession } from './Session';
66

77
/**
@@ -22,7 +22,7 @@ export function fetchSession(traceInternals?: boolean): Session | null {
2222

2323
const sessionObj = JSON.parse(sessionStringFromStorage) as Session;
2424

25-
logInfo('[Replay] Loading existing session', traceInternals);
25+
logInfoNextTick('[Replay] Loading existing session', traceInternals);
2626

2727
return makeSession(sessionObj);
2828
} catch {

packages/replay/src/session/getSession.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Session, SessionOptions, Timeouts } from '../types';
22
import { isSessionExpired } from '../util/isSessionExpired';
3-
import { logInfo } from '../util/log';
3+
import { logInfoNextTick } from '../util/log';
44
import { createSession } from './createSession';
55
import { fetchSession } from './fetchSession';
66
import { makeSession } from './Session';
@@ -44,10 +44,10 @@ export function getSession({
4444
// and when this session is expired, it will not be renewed until user
4545
// reloads.
4646
const discardedSession = makeSession({ sampled: false });
47-
logInfo('[Replay] Session should not be refreshed', traceInternals);
47+
logInfoNextTick('[Replay] Session should not be refreshed', traceInternals);
4848
return { type: 'new', session: discardedSession };
4949
} else {
50-
logInfo('[Replay] Session has expired', traceInternals);
50+
logInfoNextTick('[Replay] Session has expired', traceInternals);
5151
}
5252
// Otherwise continue to create a new session
5353
}
@@ -57,7 +57,7 @@ export function getSession({
5757
sessionSampleRate,
5858
allowBuffering,
5959
});
60-
logInfo('[Replay] Created new session', traceInternals);
60+
logInfoNextTick('[Replay] Created new session', traceInternals);
6161

6262
return { type: 'new', session: newSession };
6363
}

packages/replay/src/types/replay.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,11 @@ export interface EventBuffer {
389389
*/
390390
readonly type: EventBufferType;
391391

392+
/**
393+
* If the event buffer contains a checkout event.
394+
*/
395+
hasCheckout: boolean;
396+
392397
/**
393398
* Destroy the event buffer.
394399
*/

packages/replay/src/util/addEvent.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export async function addEvent(
4242
try {
4343
if (isCheckout && replay.recordingMode === 'buffer') {
4444
replay.eventBuffer.clear();
45+
replay.eventBuffer.hasCheckout = true;
4546
}
4647

4748
const replayOptions = replay.getOptions();

packages/replay/src/util/log.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,42 @@ export function logInfo(message: string, shouldAddBreadcrumb?: boolean): void {
1111

1212
logger.info(message);
1313

14+
if (shouldAddBreadcrumb) {
15+
addBreadcrumb(message);
16+
}
17+
}
18+
19+
/**
20+
* Log a message, and add a breadcrumb in the next tick.
21+
* This is necessary when the breadcrumb may be added before the replay is initialized.
22+
*/
23+
export function logInfoNextTick(message: string, shouldAddBreadcrumb?: boolean): void {
24+
if (!__DEBUG_BUILD__) {
25+
return;
26+
}
27+
28+
logger.info(message);
29+
1430
if (shouldAddBreadcrumb) {
1531
// Wait a tick here to avoid race conditions for some initial logs
1632
// which may be added before replay is initialized
1733
setTimeout(() => {
18-
const hub = getCurrentHub();
19-
hub.addBreadcrumb(
20-
{
21-
category: 'console',
22-
data: {
23-
logger: 'replay',
24-
},
25-
level: 'info',
26-
message,
27-
},
28-
{ level: 'info' },
29-
);
34+
addBreadcrumb(message);
3035
}, 0);
3136
}
3237
}
38+
39+
function addBreadcrumb(message: string): void {
40+
const hub = getCurrentHub();
41+
hub.addBreadcrumb(
42+
{
43+
category: 'console',
44+
data: {
45+
logger: 'replay',
46+
},
47+
level: 'info',
48+
message,
49+
},
50+
{ level: 'info' },
51+
);
52+
}

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

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as SentryUtils from '@sentry/utils';
22

3-
import { DEFAULT_FLUSH_MIN_DELAY, WINDOW } from '../../src/constants';
3+
import { DEFAULT_FLUSH_MIN_DELAY, MAX_SESSION_LIFE, WINDOW } from '../../src/constants';
44
import type { ReplayContainer } from '../../src/replay';
55
import { clearSession } from '../../src/session/clearSession';
66
import type { EventBuffer } from '../../src/types';
@@ -286,15 +286,22 @@ describe('Integration | flush', () => {
286286

287287
expect(mockFlush).toHaveBeenCalledTimes(20);
288288
expect(mockSendReplay).toHaveBeenCalledTimes(1);
289+
290+
replay.getOptions().minReplayDuration = 0;
289291
});
290292

291293
it('does not flush if session is too long', async () => {
292294
replay.timeouts.maxSessionLife = 100_000;
293-
jest.setSystemTime(new Date(BASE_TIMESTAMP));
295+
jest.setSystemTime(BASE_TIMESTAMP);
294296

295297
sessionStorage.clear();
296298
clearSession(replay);
297299
replay['_loadAndCheckSession']();
300+
// No-op _loadAndCheckSession to avoid us resetting the session for this test
301+
const _tmp = replay['_loadAndCheckSession'];
302+
replay['_loadAndCheckSession'] = () => {
303+
return true;
304+
};
298305

299306
await advanceTimers(120_000);
300307

@@ -308,7 +315,71 @@ describe('Integration | flush', () => {
308315
mockRecord._emitter(TEST_EVENT);
309316

310317
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
318+
311319
expect(mockFlush).toHaveBeenCalledTimes(1);
312320
expect(mockSendReplay).toHaveBeenCalledTimes(0);
321+
322+
replay.timeouts.maxSessionLife = MAX_SESSION_LIFE;
323+
replay['_loadAndCheckSession'] = _tmp;
324+
});
325+
326+
it('logs warning if flushing initial segment without checkout', async () => {
327+
replay.getOptions()._experiments.traceInternals = true;
328+
329+
sessionStorage.clear();
330+
clearSession(replay);
331+
replay['_loadAndCheckSession']();
332+
await new Promise(process.nextTick);
333+
jest.setSystemTime(BASE_TIMESTAMP);
334+
335+
// Clear the event buffer to simulate no checkout happened
336+
replay.eventBuffer!.clear();
337+
338+
// click happens first
339+
domHandler({
340+
name: 'click',
341+
});
342+
343+
// no checkout!
344+
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
345+
346+
expect(mockFlush).toHaveBeenCalledTimes(1);
347+
expect(mockSendReplay).toHaveBeenCalledTimes(1);
348+
349+
const replayData = mockSendReplay.mock.calls[0][0];
350+
351+
expect(JSON.parse(replayData.recordingData)).toEqual([
352+
{
353+
type: 5,
354+
timestamp: BASE_TIMESTAMP,
355+
data: {
356+
tag: 'breadcrumb',
357+
payload: {
358+
timestamp: BASE_TIMESTAMP / 1000,
359+
type: 'default',
360+
category: 'ui.click',
361+
message: '<unknown>',
362+
data: {},
363+
},
364+
},
365+
},
366+
{
367+
type: 5,
368+
timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY,
369+
data: {
370+
tag: 'breadcrumb',
371+
payload: {
372+
timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY) / 1000,
373+
type: 'default',
374+
category: 'console',
375+
data: { logger: 'replay' },
376+
level: 'info',
377+
message: '[Replay] Flushing initial segment without checkout.',
378+
},
379+
},
380+
},
381+
]);
382+
383+
replay.getOptions()._experiments.traceInternals = false;
313384
});
314385
});

0 commit comments

Comments
 (0)