diff --git a/.changeset/lucky-sheep-create.md b/.changeset/lucky-sheep-create.md new file mode 100644 index 0000000000..54c0232dd6 --- /dev/null +++ b/.changeset/lucky-sheep-create.md @@ -0,0 +1,5 @@ +--- +'posthog-js': patch +--- + +checks for session activity in other windows when timing out in any particular window, avoids a race condition when proactively marking a session as idle diff --git a/packages/browser/src/__tests__/sessionid.test.ts b/packages/browser/src/__tests__/sessionid.test.ts index d42b55af62..475308db37 100644 --- a/packages/browser/src/__tests__/sessionid.test.ts +++ b/packages/browser/src/__tests__/sessionid.test.ts @@ -39,7 +39,10 @@ describe('Session ID manager', () => { persistence = { props: { [SESSION_ID]: undefined }, - register: jest.fn(), + register: jest.fn().mockImplementation((props) => { + // Mock the behavior of register - it should update the props + Object.assign(persistence.props, props) + }), _disabled: false, } ;(sessionStore._is_supported as jest.Mock).mockReturnValue(true) @@ -393,22 +396,69 @@ describe('Session ID manager', () => { expect(sessionIdManager['_enforceIdleTimeout']).toEqual(originalTimer) }) - /** timer doesn't advance and fire this? */ - it.skip('resets session id despite no activity after timeout', () => { - ;(uuidv7 as jest.Mock).mockImplementationOnce(() => 'originalUUID') + it('resets session when idle timeout is exceeded', async () => { + jest.useFakeTimers() const sessionIdManager = sessionIdMgr(persistence) - const { sessionId: originalSessionId } = sessionIdManager.checkAndGetSessionAndWindowId( - undefined, - timestamp - ) - expect(originalSessionId).toBeDefined() + const resetSpy = jest.spyOn(sessionIdManager, 'resetSessionId') + + // Start with a fresh session + sessionIdManager.checkAndGetSessionAndWindowId(false, timestamp) + + // Set up persistence to simulate inactivity - session was last active long ago + const idleTimestamp = timestamp - (sessionIdManager.sessionTimeoutMs + 1000) + persistence.props[SESSION_ID] = [idleTimestamp, 'oldSessionID', idleTimestamp] + + // Fast-forward time to trigger the idle timeout timer + const idleTimeoutMs = sessionIdManager.sessionTimeoutMs * 1.1 + jest.advanceTimersByTime(idleTimeoutMs + 1000) + + // Timer should have fired and called resetSessionId + expect(resetSpy).toHaveBeenCalled() + + // After reset, persistence.register should have been called with null values + expect(persistence.register).toHaveBeenCalledWith({ [SESSION_ID]: [null, null, null] }) + + // Next call should generate a new session due to no session ID + const newSessionData = sessionIdManager.checkAndGetSessionAndWindowId(false) + expect(newSessionData.sessionId).toBe('newUUID') + expect(newSessionData.sessionId).not.toEqual('oldSessionID') + expect(newSessionData.changeReason?.noSessionId).toBe(true) + + jest.useRealTimers() + }) + + it('timer checks current session activity before resetting', async () => { + jest.useFakeTimers() + + const sessionIdManager = sessionIdMgr(persistence) + const resetSpy = jest.spyOn(sessionIdManager, 'resetSessionId') + + // Mock _getSessionId to control what the timer sees + const getSessionIdSpy = jest.spyOn(sessionIdManager as any, '_getSessionId') + + // Start with a fresh session + sessionIdManager.checkAndGetSessionAndWindowId(false, timestamp) + + // Initially set up an idle session + const idleTimestamp = timestamp - (sessionIdManager.sessionTimeoutMs + 1000) + getSessionIdSpy.mockReturnValue([idleTimestamp, 'sharedSessionID', timestamp]) + + // Fast-forward time almost to when timer fires + const idleTimeoutMs = sessionIdManager.sessionTimeoutMs * 1.1 + jest.advanceTimersByTime(idleTimeoutMs - 100) + + // Before timer fires, change mock to return recent activity (simulating another window updating) + const recentTimestamp = new Date().getTime() - 1000 // 1 second ago + getSessionIdSpy.mockReturnValue([recentTimestamp, 'sharedSessionID', timestamp]) + + // Let the timer fire + jest.advanceTimersByTime(200) - jest.advanceTimersByTime(DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS * 1.1 + 1) + // The timer should NOT have reset the session because it found recent activity + expect(resetSpy).not.toHaveBeenCalled() - const { sessionId: finalSessionId } = sessionIdManager.checkAndGetSessionAndWindowId(undefined, timestamp) - expect(finalSessionId).toBeDefined() - expect(finalSessionId).not.toEqual(originalSessionId) + jest.useRealTimers() }) }) }) diff --git a/packages/browser/src/sessionid.ts b/packages/browser/src/sessionid.ts index 382bfe1163..743bc6ac3d 100644 --- a/packages/browser/src/sessionid.ts +++ b/packages/browser/src/sessionid.ts @@ -212,6 +212,10 @@ export class SessionIdManager { ) } + private _sessionHasBeenIdleTooLong = (timestamp: number, lastActivityTimestamp: number) => { + return Math.abs(timestamp - lastActivityTimestamp) > this.sessionTimeoutMs + } + /* * This function returns the current sessionId and windowId. It should be used to * access these values over directly calling `._sessionId` or `._windowId`. @@ -247,7 +251,7 @@ export class SessionIdManager { let valuesChanged = false const noSessionId = !sessionId - const activityTimeout = !readOnly && Math.abs(timestamp - lastActivityTimestamp) > this.sessionTimeoutMs + const activityTimeout = !readOnly && this._sessionHasBeenIdleTooLong(timestamp, lastActivityTimestamp) if (noSessionId || activityTimeout || sessionPastMaximumLength) { sessionId = this._sessionIdGenerator() windowId = this._windowIdGenerator() @@ -297,7 +301,12 @@ export class SessionIdManager { clearTimeout(this._enforceIdleTimeout) this._enforceIdleTimeout = setTimeout(() => { // enforce idle timeout a little after the session timeout to ensure the session is reset even without activity - this.resetSessionId() + // we need to check session activity first in case a different window has kept the session active + // while this window has been idle - and the timer has not progressed - e.g. window memory frozen while hidden + const [lastActivityTimestamp] = this._getSessionId() + if (this._sessionHasBeenIdleTooLong(new Date().getTime(), lastActivityTimestamp)) { + this.resetSessionId() + } }, this.sessionTimeoutMs * 1.1) } } diff --git a/packages/browser/terser-mangled-names.json b/packages/browser/terser-mangled-names.json index 9cf55b6875..cc1ab47b6e 100644 --- a/packages/browser/terser-mangled-names.json +++ b/packages/browser/terser-mangled-names.json @@ -252,6 +252,7 @@ "_send_request", "_send_retriable_request", "_sessionDuration", + "_sessionHasBeenIdleTooLong", "_sessionId", "_sessionIdChangedHandlers", "_sessionIdGenerator",