Skip to content

Commit 0e69269

Browse files
committed
fix(replay): Fully stop & restart session when it expires
1 parent ef733c0 commit 0e69269

File tree

11 files changed

+925
-1269
lines changed

11 files changed

+925
-1269
lines changed

packages/replay/src/replay.ts

Lines changed: 36 additions & 49 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,
@@ -218,7 +218,7 @@ export class ReplayContainer implements ReplayContainerInterface {
218218
* Initializes the plugin based on sampling configuration. Should not be
219219
* called outside of constructor.
220220
*/
221-
public initializeSampling(): void {
221+
public initializeSampling(previousSessionId?: string): void {
222222
const { errorSampleRate, sessionSampleRate } = this._options;
223223

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

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

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

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

306305
const session = loadOrCreateSession(
307-
this.session,
308306
{
309307
timeouts: this.timeouts,
310308
traceInternals: this._options._experiments.traceInternals,
@@ -372,15 +370,18 @@ export class ReplayContainer implements ReplayContainerInterface {
372370
return;
373371
}
374372

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

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

@@ -474,16 +475,6 @@ export class ReplayContainer implements ReplayContainerInterface {
474475

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

736727
// Need to set as enabled before we start recording, as `record()` can trigger a flush with a new checkout
737728
this._isEnabled = true;
729+
this._isPaused = false;
738730

739731
this.startRecording();
740732
}
@@ -751,16 +743,16 @@ export class ReplayContainer implements ReplayContainerInterface {
751743
/**
752744
* Loads (or refreshes) the current session.
753745
*/
754-
private _initializeSessionForSampling(): void {
746+
private _initializeSessionForSampling(previousSessionId?: string): void {
755747
// Whenever there is _any_ error sample rate, we always allow buffering
756748
// Because we decide on sampling when an error occurs, we need to buffer at all times if sampling for errors
757749
const allowBuffering = this._options.errorSampleRate > 0;
758750

759751
const session = loadOrCreateSession(
760-
this.session,
761752
{
762753
timeouts: this.timeouts,
763754
traceInternals: this._options._experiments.traceInternals,
755+
previousSessionId,
764756
},
765757
{
766758
stickySession: this._options.stickySession,
@@ -785,36 +777,27 @@ export class ReplayContainer implements ReplayContainerInterface {
785777

786778
const currentSession = this.session;
787779

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

815785
return true;
816786
}
817787

788+
/**
789+
* Refresh a session with a new one.
790+
* This stops the current session (without forcing a flush, as that would never work since we are expired),
791+
* and then does a new sampling based on the refreshed session.
792+
*/
793+
private async _refreshSession(session: Session): Promise<void> {
794+
if (!this._isEnabled) {
795+
return;
796+
}
797+
await this.stop({ reason: 'refresh session' });
798+
this.initializeSampling(session.id);
799+
}
800+
818801
/**
819802
* Adds listeners to record events for the replay
820803
*/
@@ -1071,7 +1054,9 @@ export class ReplayContainer implements ReplayContainerInterface {
10711054
* Should never be called directly, only by `flush`
10721055
*/
10731056
private async _runFlush(): Promise<void> {
1074-
if (!this.session || !this.eventBuffer) {
1057+
const replayId = this.getSessionId();
1058+
1059+
if (!this.session || !this.eventBuffer || !replayId) {
10751060
__DEBUG_BUILD__ && logger.error('[Replay] No session or eventBuffer found to flush.');
10761061
return;
10771062
}
@@ -1091,6 +1076,11 @@ export class ReplayContainer implements ReplayContainerInterface {
10911076
return;
10921077
}
10931078

1079+
// if this changed in the meanwhile, e.g. because the session was refreshed or similar, we abort here
1080+
if (replayId !== this.getSessionId()) {
1081+
return;
1082+
}
1083+
10941084
try {
10951085
// This uses the data from the eventBuffer, so we need to call this before `finish()
10961086
this._updateInitialTimestampFromEventBuffer();
@@ -1107,9 +1097,6 @@ export class ReplayContainer implements ReplayContainerInterface {
11071097
throw new Error('Session is too long, not sending replay');
11081098
}
11091099

1110-
// NOTE: Copy values from instance members, as it's possible they could
1111-
// change before the flush finishes.
1112-
const replayId = this.session.id;
11131100
const eventContext = this._popEventContext();
11141101
// Always increment segmentId regardless of outcome of sending replay
11151102
const segmentId = this.session.segmentId++;

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)