fix

pauldambra · pauldambra · commit 1f7032827301 · 2024-11-26T16:46:24.000Z
diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts
@@ -235,7 +235,11 @@ describe('SessionRecording', () => {
         const postHogPersistence = new PostHogPersistence(config)
         postHogPersistence.clear()
 
-        sessionManager = new SessionIdManager(config, postHogPersistence, sessionIdGeneratorMock, windowIdGeneratorMock)
+        sessionManager = new SessionIdManager(
+            { config, persistence: postHogPersistence, register: jest.fn() } as unknown as PostHog,
+            sessionIdGeneratorMock,
+            windowIdGeneratorMock
+        )
 
         // add capture hook returns an unsubscribe function
         removeCaptureHookMock = jest.fn()
@@ -1130,7 +1134,11 @@ describe('SessionRecording', () => {
                 let unsubscribeCallback: () => void
 
                 beforeEach(() => {
-                    sessionManager = new SessionIdManager(config, new PostHogPersistence(config))
+                    sessionManager = new SessionIdManager({
+                        config,
+                        persistence: new PostHogPersistence(config),
+                        register: jest.fn(),
+                    } as unknown as PostHog)
                     posthog.sessionManager = sessionManager
 
                     mockCallback = jest.fn()
@@ -1216,7 +1224,11 @@ describe('SessionRecording', () => {
 
             describe('with a real session id manager', () => {
                 beforeEach(() => {
-                    sessionManager = new SessionIdManager(config, new PostHogPersistence(config))
+                    sessionManager = new SessionIdManager({
+                        config,
+                        persistence: new PostHogPersistence(config),
+                        register: jest.fn(),
+                    } as unknown as PostHog)
                     posthog.sessionManager = sessionManager
 
                     sessionRecording.startIfEnabledOrStop()
diff --git a/src/__tests__/sessionid.test.ts b/src/__tests__/sessionid.test.ts
@@ -1,10 +1,11 @@
-import { SessionIdManager } from '../sessionid'
+import { DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS, MAX_SESSION_IDLE_TIMEOUT_SECONDS, SessionIdManager } from '../sessionid'
 import { SESSION_ID } from '../constants'
 import { sessionStore } from '../storage'
 import { uuid7ToTimestampMs, uuidv7 } from '../uuidv7'
 import { BootstrapConfig, PostHogConfig, Properties } from '../types'
 import { PostHogPersistence } from '../posthog-persistence'
 import { assignableWindow } from '../utils/globals'
+import { PostHog } from '../posthog-core'
 
 jest.mock('../uuidv7')
 jest.mock('../storage')
@@ -13,14 +14,22 @@ describe('Session ID manager', () => {
     let timestamp: number | undefined
     let now: number
     let timestampOfSessionStart: number
+    let registerMock: jest.Mock
+
     const config: Partial<PostHogConfig> = {
         persistence_name: 'persistance-name',
     }
 
     let persistence: { props: Properties } & Partial<PostHogPersistence>
 
-    const sessionIdMgr = (phPersistence: Partial<PostHogPersistence>) =>
-        new SessionIdManager(config, phPersistence as PostHogPersistence)
+    const sessionIdMgr = (phPersistence: Partial<PostHogPersistence>) => {
+        registerMock = jest.fn()
+        return new SessionIdManager({
+            config,
+            persistence: phPersistence as PostHogPersistence,
+            register: registerMock,
+        } as unknown as PostHog)
+    }
 
     const originalDate = Date
 
@@ -70,7 +79,11 @@ describe('Session ID manager', () => {
             const bootstrap: BootstrapConfig = {
                 sessionID: bootstrapSessionId,
             }
-            const sessionIdManager = new SessionIdManager({ ...config, bootstrap }, persistence as PostHogPersistence)
+            const sessionIdManager = new SessionIdManager({
+                config: { ...config, bootstrap },
+                persistence: persistence as PostHogPersistence,
+                register: jest.fn(),
+            } as unknown as PostHog)
 
             // act
             const { sessionId, sessionStartTimestamp } = sessionIdManager.checkAndGetSessionAndWindowId(false, now)
@@ -79,6 +92,15 @@ describe('Session ID manager', () => {
             expect(sessionId).toEqual(bootstrapSessionId)
             expect(sessionStartTimestamp).toEqual(timestamp)
         })
+
+        it('registers the session timeout as an event property', () => {
+            config.session_idle_timeout_seconds = 8 * 60 * 60
+            const sessionIdManager = sessionIdMgr(persistence)
+            sessionIdManager.checkAndGetSessionAndWindowId(undefined, timestamp)
+            expect(registerMock).toHaveBeenCalledWith({
+                $configured_session_timeout_ms: config.session_idle_timeout_seconds * 1000,
+            })
+        })
     })
 
     describe('stored session data', () => {
@@ -317,12 +339,13 @@ describe('Session ID manager', () => {
 
     describe('custom session_idle_timeout_seconds', () => {
         const mockSessionManager = (timeout: number | undefined) =>
-            new SessionIdManager(
-                {
+            new SessionIdManager({
+                config: {
                     session_idle_timeout_seconds: timeout,
                 },
-                persistence as PostHogPersistence
-            )
+                persistence: persistence as PostHogPersistence,
+                register: jest.fn(),
+            } as unknown as PostHog)
 
         beforeEach(() => {
             console.warn = jest.fn()
@@ -336,10 +359,14 @@ describe('Session ID manager', () => {
             expect(console.warn).toBeCalledTimes(1)
             expect(mockSessionManager(30 * 60 - 1)['_sessionTimeoutMs']).toEqual((30 * 60 - 1) * 1000)
             expect(console.warn).toBeCalledTimes(1)
-            expect(mockSessionManager(30 * 60 + 1)['_sessionTimeoutMs']).toEqual(30 * 60 * 1000)
+            expect(mockSessionManager(MAX_SESSION_IDLE_TIMEOUT_SECONDS + 1)['_sessionTimeoutMs']).toEqual(
+                MAX_SESSION_IDLE_TIMEOUT_SECONDS * 1000
+            )
             expect(console.warn).toBeCalledTimes(2)
             // @ts-expect-error - test invalid input
-            expect(mockSessionManager('foobar')['_sessionTimeoutMs']).toEqual(30 * 60 * 1000)
+            expect(mockSessionManager('foobar')['_sessionTimeoutMs']).toEqual(
+                DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS * 1000
+            )
             expect(console.warn).toBeCalledTimes(3)
         })
     })
diff --git a/src/__tests__/utils/number-utils.test.ts b/src/__tests__/utils/number-utils.test.ts
@@ -10,14 +10,78 @@ jest.mock('../../utils/logger', () => ({
 describe('number-utils', () => {
     describe('clampToRange', () => {
         it.each([
-            // [value, result, min, max, expected result, test description]
-            ['returns max when value is not a number', null, 10, 100, 100],
-            ['returns max when value is not a number', 'not-a-number', 10, 100, 100],
-            ['returns max when value is greater than max', 150, 10, 100, 100],
-            ['returns min when value is less than min', 5, 10, 100, 10],
-            ['returns the value when it is within the range', 50, 10, 100, 50],
-        ])('%s', (_description, value, min, max, expected) => {
-            const result = clampToRange(value, min, max, 'Test Label')
+            [
+                'returns max when value is not a number',
+                {
+                    value: null,
+                    min: 10,
+                    max: 100,
+                    expected: 100,
+                    fallback: undefined,
+                },
+            ],
+            [
+                'returns max when value is not a number',
+                {
+                    value: 'not-a-number',
+                    min: 10,
+                    max: 100,
+                    expected: 100,
+                    fallback: undefined,
+                },
+            ],
+            [
+                'returns max when value is greater than max',
+                {
+                    value: 150,
+                    min: 10,
+                    max: 100,
+                    expected: 100,
+                    fallback: undefined,
+                },
+            ],
+            [
+                'returns min when value is less than min',
+                {
+                    value: 5,
+                    min: 10,
+                    max: 100,
+                    expected: 10,
+                    fallback: undefined,
+                },
+            ],
+            [
+                'returns the value when it is within the range',
+                {
+                    value: 50,
+                    min: 10,
+                    max: 100,
+                    expected: 50,
+                    fallback: undefined,
+                },
+            ],
+            [
+                'returns the fallback value when provided is not valid',
+                {
+                    value: 'invalid',
+                    min: 10,
+                    max: 100,
+                    expected: 20,
+                    fallback: 20,
+                },
+            ],
+            [
+                'returns the max value when fallback is not valid',
+                {
+                    value: 'invalid',
+                    min: 10,
+                    max: 75,
+                    expected: 75,
+                    fallback: '20',
+                },
+            ],
+        ])('%s', (_description, { value, min, max, expected, fallback }) => {
+            const result = clampToRange(value, min, max, 'Test Label', fallback)
             expect(result).toBe(expected)
         })
 
diff --git a/src/posthog-core.ts b/src/posthog-core.ts
@@ -427,7 +427,7 @@ export class PostHog {
         this._retryQueue = new RetryQueue(this)
         this.__request_queue = []
 
-        this.sessionManager = new SessionIdManager(this.config, this.persistence)
+        this.sessionManager = new SessionIdManager(this)
         this.sessionPropsManager = new SessionPropsManager(this.sessionManager, this.persistence)
 
         new TracingHeaders(this).startIfEnabledOrStop()
diff --git a/src/sessionid.ts b/src/sessionid.ts
@@ -9,9 +9,10 @@ import { isArray, isNumber, isUndefined } from './utils/type-utils'
 import { logger } from './utils/logger'
 
 import { clampToRange } from './utils/number-utils'
+import { PostHog } from './posthog-core'
 
-const DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS = 30 * 60 // 30 minutes
-const MAX_SESSION_IDLE_TIMEOUT_SECONDS = 10 * 60 * 60 // 10 hours
+export const DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS = 30 * 60 // 30 minutes
+export const MAX_SESSION_IDLE_TIMEOUT_SECONDS = 10 * 60 * 60 // 10 hours
 const MIN_SESSION_IDLE_TIMEOUT_SECONDS = 60 // 1 minute
 const SESSION_LENGTH_LIMIT_MILLISECONDS = 24 * 3600 * 1000 // 24 hours
 
@@ -30,32 +31,34 @@ export class SessionIdManager {
     private _sessionIdChangedHandlers: SessionIdChangedCallback[] = []
     private readonly _sessionTimeoutMs: number
 
-    constructor(
-        config: Partial<PostHogConfig>,
-        persistence: PostHogPersistence,
-        sessionIdGenerator?: () => string,
-        windowIdGenerator?: () => string
-    ) {
-        this.config = config
-        this.persistence = persistence
+    constructor(instance: PostHog, sessionIdGenerator?: () => string, windowIdGenerator?: () => string) {
+        if (!instance.persistence) {
+            throw new Error('SessionIdManager requires a PostHogPersistence instance')
+        }
+
+        this.config = instance.config
+        this.persistence = instance.persistence
         this._windowId = undefined
         this._sessionId = undefined
         this._sessionStartTimestamp = null
         this._sessionActivityTimestamp = null
         this._sessionIdGenerator = sessionIdGenerator || uuidv7
         this._windowIdGenerator = windowIdGenerator || uuidv7
 
-        const persistenceName = config['persistence_name'] || config['token']
+        const persistenceName = this.config['persistence_name'] || this.config['token']
 
-        const desiredTimeout = config['session_idle_timeout_seconds'] || DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS
+        const desiredTimeout = this.config['session_idle_timeout_seconds'] || DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS
         this._sessionTimeoutMs =
             clampToRange(
                 desiredTimeout,
                 MIN_SESSION_IDLE_TIMEOUT_SECONDS,
                 MAX_SESSION_IDLE_TIMEOUT_SECONDS,
-                'session_idle_timeout_seconds'
+                'session_idle_timeout_seconds',
+                DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS
             ) * 1000
 
+        instance.register({ $configured_session_timeout_ms: this._sessionTimeoutMs })
+
         this._window_id_storage_key = 'ph_' + persistenceName + '_window_id'
         this._primary_window_exists_storage_key = 'ph_' + persistenceName + '_primary_window_exists'
 
diff --git a/src/utils/number-utils.ts b/src/utils/number-utils.ts
@@ -1,15 +1,26 @@
 import { isNumber } from './type-utils'
 import { logger } from './logger'
 
-export function clampToRange(value: unknown, min: number, max: number, label?: string): number {
+/**
+ * Clamps a value to a range.
+ * @param value the value to clamp
+ * @param min the minimum value
+ * @param max the maximum value
+ * @param label if provided then enables logging and prefixes all logs with labels
+ * @param fallbackValue if provided then returns this value if the value is not a valid number
+ */
+export function clampToRange(value: unknown, min: number, max: number, label?: string, fallbackValue?: number): number {
     if (min > max) {
         logger.warn('min cannot be greater than max.')
         min = max
     }
 
     if (!isNumber(value)) {
-        label && logger.warn(label + ' must be a number. Defaulting to max value:' + max)
-        return max
+        label &&
+            logger.warn(
+                label + ' must be a number. using max or fallback. max: ' + max + ', fallback: ' + fallbackValue
+            )
+        return clampToRange(fallbackValue || max, min, max, label)
     } else if (value > max) {
         label && logger.warn(label + ' cannot be  greater than max: ' + max + '. Using max value instead.')
         return max
