Skip to content

Commit f70f354

Browse files
committed
fix(replay): Fully stop & restart session when it expires
1 parent 32befda commit f70f354

File tree

11 files changed

+926
-1270
lines changed

11 files changed

+926
-1270
lines changed

packages/replay/src/replay.ts

Lines changed: 39 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import { setupPerformanceObserver } from './coreHandlers/performanceObserver';
1919
import { createEventBuffer } from './eventBuffer';
2020
import { clearSession } from './session/clearSession';
2121
import { loadOrCreateSession } from './session/loadOrCreateSession';
22-
import { maybeRefreshSession } from './session/maybeRefreshSession';
2322
import { saveSession } from './session/saveSession';
23+
import { shouldRefreshSession } from './session/shouldRefreshSession';
2424
import type {
2525
AddEventResult,
2626
AddUpdateCallback,
@@ -219,7 +219,7 @@ export class ReplayContainer implements ReplayContainerInterface {
219219
* Initializes the plugin based on sampling configuration. Should not be
220220
* called outside of constructor.
221221
*/
222-
public initializeSampling(): void {
222+
public initializeSampling(previousSessionId?: string): void {
223223
const { errorSampleRate, sessionSampleRate } = this._options;
224224

225225
// If neither sample rate is > 0, then do nothing - user will need to call one of
@@ -230,7 +230,7 @@ export class ReplayContainer implements ReplayContainerInterface {
230230

231231
// Otherwise if there is _any_ sample rate set, try to load an existing
232232
// session, or create a new one.
233-
this._initializeSessionForSampling();
233+
this._initializeSessionForSampling(previousSessionId);
234234

235235
if (!this.session) {
236236
// This should not happen, something wrong has occurred
@@ -275,7 +275,6 @@ export class ReplayContainer implements ReplayContainerInterface {
275275
logInfoNextTick('[Replay] Starting replay in session mode', this._options._experiments.traceInternals);
276276

277277
const session = loadOrCreateSession(
278-
this.session,
279278
{
280279
timeouts: this.timeouts,
281280
traceInternals: this._options._experiments.traceInternals,
@@ -305,7 +304,6 @@ export class ReplayContainer implements ReplayContainerInterface {
305304
logInfoNextTick('[Replay] Starting replay in buffer mode', this._options._experiments.traceInternals);
306305

307306
const session = loadOrCreateSession(
308-
this.session,
309307
{
310308
timeouts: this.timeouts,
311309
traceInternals: this._options._experiments.traceInternals,
@@ -373,15 +371,18 @@ export class ReplayContainer implements ReplayContainerInterface {
373371
return;
374372
}
375373

374+
// We can't move `_isEnabled` after awaiting a flush, otherwise we can
375+
// enter into an infinite loop when `stop()` is called while flushing.
376+
this._isEnabled = false;
377+
376378
try {
377379
logInfo(
378-
`[Replay] Stopping Replay${reason ? ` triggered by ${reason}` : ''}`,
380+
`[Replay] Stopping Replay${reason ? ` triggered by ${reason}` : ''} ${new Date().toISOString()} ${
381+
this._isEnabled
382+
}`,
379383
this._options._experiments.traceInternals,
380384
);
381385

382-
// We can't move `_isEnabled` after awaiting a flush, otherwise we can
383-
// enter into an infinite loop when `stop()` is called while flushing.
384-
this._isEnabled = false;
385386
this._removeListeners();
386387
this.stopRecording();
387388

@@ -475,16 +476,6 @@ export class ReplayContainer implements ReplayContainerInterface {
475476

476477
// Once this session ends, we do not want to refresh it
477478
if (this.session) {
478-
this.session.shouldRefresh = false;
479-
480-
// It's possible that the session lifespan is > max session lifespan
481-
// because we have been buffering beyond max session lifespan (we ignore
482-
// expiration given that `shouldRefresh` is true). Since we flip
483-
// `shouldRefresh`, the session could be considered expired due to
484-
// lifespan, which is not what we want. Update session start date to be
485-
// the current timestamp, so that session is not considered to be
486-
// expired. This means that max replay duration can be MAX_SESSION_LIFE +
487-
// (length of buffer), which we are ok with.
488479
this._updateUserActivity(activityTime);
489480
this._updateSessionActivity(activityTime);
490481
this._maybeSaveSession();
@@ -740,6 +731,7 @@ export class ReplayContainer implements ReplayContainerInterface {
740731

741732
// Need to set as enabled before we start recording, as `record()` can trigger a flush with a new checkout
742733
this._isEnabled = true;
734+
this._isPaused = false;
743735

744736
this.startRecording();
745737
}
@@ -756,16 +748,16 @@ export class ReplayContainer implements ReplayContainerInterface {
756748
/**
757749
* Loads (or refreshes) the current session.
758750
*/
759-
private _initializeSessionForSampling(): void {
751+
private _initializeSessionForSampling(previousSessionId?: string): void {
760752
// Whenever there is _any_ error sample rate, we always allow buffering
761753
// Because we decide on sampling when an error occurs, we need to buffer at all times if sampling for errors
762754
const allowBuffering = this._options.errorSampleRate > 0;
763755

764756
const session = loadOrCreateSession(
765-
this.session,
766757
{
767758
timeouts: this.timeouts,
768759
traceInternals: this._options._experiments.traceInternals,
760+
previousSessionId,
769761
},
770762
{
771763
stickySession: this._options.stickySession,
@@ -790,36 +782,27 @@ export class ReplayContainer implements ReplayContainerInterface {
790782

791783
const currentSession = this.session;
792784

793-
const newSession = maybeRefreshSession(
794-
currentSession,
795-
{
796-
timeouts: this.timeouts,
797-
traceInternals: this._options._experiments.traceInternals,
798-
},
799-
{
800-
stickySession: Boolean(this._options.stickySession),
801-
sessionSampleRate: this._options.sessionSampleRate,
802-
allowBuffering: this._options.errorSampleRate > 0,
803-
},
804-
);
805-
806-
const isNew = newSession.id !== currentSession.id;
807-
808-
// If session was newly created (i.e. was not loaded from storage), then
809-
// enable flag to create the root replay
810-
if (isNew) {
811-
this.setInitialState();
812-
this.session = newSession;
813-
}
814-
815-
if (!this.session.sampled) {
816-
void this.stop({ reason: 'session not refreshed' });
785+
if (shouldRefreshSession(currentSession, this.timeouts)) {
786+
void this._refreshSession(currentSession);
817787
return false;
818788
}
819789

820790
return true;
821791
}
822792

793+
/**
794+
* Refresh a session with a new one.
795+
* This stops the current session (without forcing a flush, as that would never work since we are expired),
796+
* and then does a new sampling based on the refreshed session.
797+
*/
798+
private async _refreshSession(session: Session): Promise<void> {
799+
if (!this._isEnabled) {
800+
return;
801+
}
802+
await this.stop({ reason: 'refresh session' });
803+
this.initializeSampling(session.id);
804+
}
805+
823806
/**
824807
* Adds listeners to record events for the replay
825808
*/
@@ -1076,7 +1059,9 @@ export class ReplayContainer implements ReplayContainerInterface {
10761059
* Should never be called directly, only by `flush`
10771060
*/
10781061
private async _runFlush(): Promise<void> {
1079-
if (!this.session || !this.eventBuffer) {
1062+
const replayId = this.getSessionId();
1063+
1064+
if (!this.session || !this.eventBuffer || !replayId) {
10801065
__DEBUG_BUILD__ && logger.error('[Replay] No session or eventBuffer found to flush.');
10811066
return;
10821067
}
@@ -1096,13 +1081,15 @@ export class ReplayContainer implements ReplayContainerInterface {
10961081
return;
10971082
}
10981083

1084+
// if this changed in the meanwhile, e.g. because the session was refreshed or similar, we abort here
1085+
if (replayId !== this.getSessionId()) {
1086+
return;
1087+
}
1088+
10991089
try {
11001090
// This uses the data from the eventBuffer, so we need to call this before `finish()
11011091
this._updateInitialTimestampFromEventBuffer();
11021092

1103-
// Note this empties the event buffer regardless of outcome of sending replay
1104-
const recordingData = await this.eventBuffer.finish();
1105-
11061093
const timestamp = Date.now();
11071094

11081095
// Check total duration again, to avoid sending outdated stuff
@@ -1112,14 +1099,14 @@ export class ReplayContainer implements ReplayContainerInterface {
11121099
throw new Error('Session is too long, not sending replay');
11131100
}
11141101

1115-
// NOTE: Copy values from instance members, as it's possible they could
1116-
// change before the flush finishes.
1117-
const replayId = this.session.id;
11181102
const eventContext = this._popEventContext();
11191103
// Always increment segmentId regardless of outcome of sending replay
11201104
const segmentId = this.session.segmentId++;
11211105
this._maybeSaveSession();
11221106

1107+
// Note this empties the event buffer regardless of outcome of sending replay
1108+
const recordingData = await this.eventBuffer.finish();
1109+
11231110
await sendReplay({
11241111
replayId,
11251112
recordingData,

packages/replay/src/session/Session.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ export function makeSession(session: Partial<Session> & { sampled: Sampled }): S
1313
const lastActivity = session.lastActivity || now;
1414
const segmentId = session.segmentId || 0;
1515
const sampled = session.sampled;
16-
const shouldRefresh = typeof session.shouldRefresh === 'boolean' ? session.shouldRefresh : true;
1716
const previousSessionId = session.previousSessionId;
1817

1918
return {
@@ -22,7 +21,6 @@ export function makeSession(session: Partial<Session> & { sampled: Sampled }): S
2221
lastActivity,
2322
segmentId,
2423
sampled,
25-
shouldRefresh,
2624
previousSessionId,
2725
};
2826
}

packages/replay/src/session/loadOrCreateSession.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,36 @@ import type { Session, SessionOptions, Timeouts } from '../types';
22
import { logInfoNextTick } from '../util/log';
33
import { createSession } from './createSession';
44
import { fetchSession } from './fetchSession';
5-
import { maybeRefreshSession } from './maybeRefreshSession';
5+
import { shouldRefreshSession } from './shouldRefreshSession';
66

77
/**
88
* Get or create a session, when initializing the replay.
99
* Returns a session that may be unsampled.
1010
*/
1111
export function loadOrCreateSession(
12-
currentSession: Session | undefined,
1312
{
1413
timeouts,
1514
traceInternals,
15+
previousSessionId,
1616
}: {
1717
timeouts: Timeouts;
1818
traceInternals?: boolean;
19+
previousSessionId?: string;
1920
},
2021
sessionOptions: SessionOptions,
2122
): Session {
22-
// If session exists and is passed, use it instead of always hitting session storage
23-
const existingSession = currentSession || (sessionOptions.stickySession && fetchSession(traceInternals));
23+
const existingSession = sessionOptions.stickySession && fetchSession(traceInternals);
2424

2525
// No session exists yet, just create a new one
2626
if (!existingSession) {
27-
logInfoNextTick('[Replay] Created new session', traceInternals);
28-
return createSession(sessionOptions);
27+
logInfoNextTick('[Replay] Creating new session', traceInternals);
28+
return createSession(sessionOptions, { previousSessionId });
2929
}
3030

31-
return maybeRefreshSession(existingSession, { timeouts, traceInternals }, sessionOptions);
31+
if (!shouldRefreshSession(existingSession, timeouts)) {
32+
return existingSession;
33+
}
34+
35+
logInfoNextTick('[Replay] Session in sessionStorage is expired, creating new one...');
36+
return createSession(sessionOptions, { previousSessionId: existingSession.id });
3237
}

packages/replay/src/session/maybeRefreshSession.ts

Lines changed: 0 additions & 48 deletions
This file was deleted.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import type { Session, Timeouts } from '../types';
2+
import { isSessionExpired } from '../util/isSessionExpired';
3+
4+
/** If the session should be refreshed or not. */
5+
export function shouldRefreshSession(session: Session, timeouts: Timeouts): boolean {
6+
// If not expired, all good, just keep the session
7+
if (!isSessionExpired(session, timeouts)) {
8+
return false;
9+
}
10+
11+
// If we are buffering & haven't ever flushed yet, always continue
12+
if (session.sampled === 'buffer' && session.segmentId === 0) {
13+
return false;
14+
}
15+
16+
return true;
17+
}

packages/replay/src/types/replay.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -368,12 +368,6 @@ export interface Session {
368368
* Is the session sampled? `false` if not sampled, otherwise, `session` or `buffer`
369369
*/
370370
sampled: Sampled;
371-
372-
/**
373-
* If this is false, the session should not be refreshed when it was inactive.
374-
* This can be the case if you had a buffered session which is now recording because an error happened.
375-
*/
376-
shouldRefresh: boolean;
377371
}
378372

379373
export type EventBufferType = 'sync' | 'worker';

0 commit comments

Comments
 (0)