Skip to content

Commit

Permalink
improve timeouts config
Browse files Browse the repository at this point in the history
  • Loading branch information
mydea committed Mar 3, 2023
1 parent 97eec7a commit c6135dd
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 44 deletions.
10 changes: 5 additions & 5 deletions packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {
ReplayContainer as ReplayContainerInterface,
ReplayPluginOptions,
Session,
Timeouts,
} from './types';
import { addEvent } from './util/addEvent';
import { addGlobalListeners } from './util/addGlobalListeners';
Expand Down Expand Up @@ -58,10 +59,10 @@ export class ReplayContainer implements ReplayContainerInterface {
* These are here so we can overwrite them in tests etc.
* @hidden
*/
public timeouts = {
public readonly timeouts: Timeouts = {
sessionIdle: SESSION_IDLE_DURATION,
maxSessionLife: MAX_SESSION_LIFE,
};
} as const;

/**
* Options to pass to `rrweb.record()`
Expand Down Expand Up @@ -417,8 +418,7 @@ export class ReplayContainer implements ReplayContainerInterface {
*/
private _loadAndCheckSession(): boolean {
const { type, session } = getSession({
expiry: this.timeouts.sessionIdle,
maxSessionLife: this.timeouts.maxSessionLife,
timeouts: this.timeouts,
stickySession: Boolean(this._options.stickySession),
currentSession: this.session,
sessionSampleRate: this._options.sessionSampleRate,
Expand Down Expand Up @@ -630,7 +630,7 @@ export class ReplayContainer implements ReplayContainerInterface {
return;
}

const expired = isSessionExpired(this.session, this.timeouts.maxSessionLife, this.timeouts.sessionIdle);
const expired = isSessionExpired(this.session, this.timeouts);

if (breadcrumb && !expired) {
this._createCustomBreadcrumb(breadcrumb);
Expand Down
15 changes: 4 additions & 11 deletions packages/replay/src/session/getSession.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
import { logger } from '@sentry/utils';

import type { Session, SessionOptions } from '../types';
import type { Session, SessionOptions, Timeouts } from '../types';
import { isSessionExpired } from '../util/isSessionExpired';
import { createSession } from './createSession';
import { fetchSession } from './fetchSession';
import { makeSession } from './Session';

interface GetSessionParams extends SessionOptions {
/**
* The length of time (in ms) which we will consider the session to be expired.
*/
expiry: number;

/** How long a session may max. be. */
maxSessionLife: number;
timeouts: Timeouts;

/**
* The current session (e.g. if stickySession is off)
Expand All @@ -25,8 +19,7 @@ interface GetSessionParams extends SessionOptions {
* Get or create a session
*/
export function getSession({
expiry,
maxSessionLife,
timeouts,
currentSession,
stickySession,
sessionSampleRate,
Expand All @@ -39,7 +32,7 @@ export function getSession({
// If there is a session, check if it is valid (e.g. "last activity" time
// should be within the "session idle time", and "session started" time is
// within "max session time").
const isExpired = isSessionExpired(session, maxSessionLife, expiry);
const isExpired = isSessionExpired(session, timeouts);

if (!isExpired) {
return { type: 'saved', session };
Expand Down
5 changes: 5 additions & 0 deletions packages/replay/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ export interface SendReplayData {
options: ReplayPluginOptions;
}

export interface Timeouts {
sessionIdle: number;
maxSessionLife: number;
}

/**
* The request payload to worker
*/
Expand Down
13 changes: 4 additions & 9 deletions packages/replay/src/util/isSessionExpired.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
import type { Session } from '../types';
import type { Session, Timeouts } from '../types';
import { isExpired } from './isExpired';

/**
* Checks to see if session is expired
*/
export function isSessionExpired(
session: Session,
maxSessionLife: number,
idleTimeout: number,
targetTime: number = +new Date(),
): boolean {
export function isSessionExpired(session: Session, timeouts: Timeouts, targetTime: number = +new Date()): boolean {
return (
// First, check that maximum session length has not been exceeded
isExpired(session.started, maxSessionLife, targetTime) ||
isExpired(session.started, timeouts.maxSessionLife, targetTime) ||
// check that the idle timeout has not been exceeded (i.e. user has
// performed an action within the last `idleTimeout` ms)
isExpired(session.lastActivity, idleTimeout, targetTime)
isExpired(session.lastActivity, timeouts.sessionIdle, targetTime)
);
}
44 changes: 29 additions & 15 deletions packages/replay/test/unit/session/getSession.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { MAX_SESSION_LIFE, WINDOW } from '../../../src/constants';
import { MAX_SESSION_LIFE, SESSION_IDLE_DURATION, WINDOW } from '../../../src/constants';
import * as CreateSession from '../../../src/session/createSession';
import * as FetchSession from '../../../src/session/fetchSession';
import { getSession } from '../../../src/session/getSession';
Expand Down Expand Up @@ -42,8 +42,10 @@ describe('Unit | session | getSession', () => {

it('creates a non-sticky session when one does not exist', function () {
const { session } = getSession({
expiry: 900000,
maxSessionLife: MAX_SESSION_LIFE,
timeouts: {
sessionIdle: SESSION_IDLE_DURATION,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: false,
...SAMPLE_RATES,
});
Expand All @@ -67,8 +69,10 @@ describe('Unit | session | getSession', () => {
saveSession(createMockSession(new Date().getTime() - 10000));

const { session } = getSession({
expiry: 1000,
maxSessionLife: MAX_SESSION_LIFE,
timeouts: {
sessionIdle: 1000,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: false,
...SAMPLE_RATES,
});
Expand All @@ -81,8 +85,10 @@ describe('Unit | session | getSession', () => {

it('creates a non-sticky session, when one is expired', function () {
const { session } = getSession({
expiry: 1000,
maxSessionLife: MAX_SESSION_LIFE,
timeouts: {
sessionIdle: 1000,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: false,
...SAMPLE_RATES,
currentSession: makeSession({
Expand All @@ -105,8 +111,10 @@ describe('Unit | session | getSession', () => {
expect(FetchSession.fetchSession()).toBe(null);

const { session } = getSession({
expiry: 900000,
maxSessionLife: MAX_SESSION_LIFE,
timeouts: {
sessionIdle: SESSION_IDLE_DURATION,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: true,
sessionSampleRate: 1.0,
errorSampleRate: 0.0,
Expand Down Expand Up @@ -138,8 +146,10 @@ describe('Unit | session | getSession', () => {
saveSession(createMockSession(now));

const { session } = getSession({
expiry: 1000,
maxSessionLife: MAX_SESSION_LIFE,
timeouts: {
sessionIdle: 1000,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: true,
sessionSampleRate: 1.0,
errorSampleRate: 0.0,
Expand All @@ -162,8 +172,10 @@ describe('Unit | session | getSession', () => {
saveSession(createMockSession(new Date().getTime() - 2000));

const { session } = getSession({
expiry: 1000,
maxSessionLife: MAX_SESSION_LIFE,
timeouts: {
sessionIdle: 1000,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: true,
...SAMPLE_RATES,
});
Expand All @@ -179,8 +191,10 @@ describe('Unit | session | getSession', () => {

it('fetches a non-expired non-sticky session', function () {
const { session } = getSession({
expiry: 1000,
maxSessionLife: MAX_SESSION_LIFE,
timeouts: {
sessionIdle: 1000,
maxSessionLife: MAX_SESSION_LIFE,
},
stickySession: false,
...SAMPLE_RATES,
currentSession: makeSession({
Expand Down
18 changes: 14 additions & 4 deletions packages/replay/test/unit/util/isSessionExpired.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,28 @@ function createSession(extra?: Record<string, any>) {

describe('Unit | util | isSessionExpired', () => {
it('session last activity is older than expiry time', function () {
expect(isSessionExpired(createSession(), MAX_SESSION_LIFE, 100, 200)).toBe(true); // Session expired at ts = 100
expect(isSessionExpired(createSession(), { maxSessionLife: MAX_SESSION_LIFE, sessionIdle: 100 }, 200)).toBe(true); // Session expired at ts = 100
});

it('session last activity is not older than expiry time', function () {
expect(isSessionExpired(createSession({ lastActivity: 100 }), MAX_SESSION_LIFE, 150, 200)).toBe(false); // Session expires at ts >= 250
expect(
isSessionExpired(
createSession({ lastActivity: 100 }),
{ maxSessionLife: MAX_SESSION_LIFE, sessionIdle: 150 },
200,
),
).toBe(false); // Session expires at ts >= 250
});

it('session age is not older than max session life', function () {
expect(isSessionExpired(createSession(), MAX_SESSION_LIFE, 1_800_000, 50_000)).toBe(false);
expect(
isSessionExpired(createSession(), { maxSessionLife: MAX_SESSION_LIFE, sessionIdle: 1_800_000 }, 50_000),
).toBe(false);
});

it('session age is older than max session life', function () {
expect(isSessionExpired(createSession(), MAX_SESSION_LIFE, 1_800_000, 1_800_001)).toBe(true); // Session expires at ts >= 1_800_000
expect(
isSessionExpired(createSession(), { maxSessionLife: MAX_SESSION_LIFE, sessionIdle: 1_800_000 }, 1_800_001),
).toBe(true); // Session expires at ts >= 1_800_000
});
});

0 comments on commit c6135dd

Please sign in to comment.