diff --git a/sdk/highlight-run/src/__tests__/highlightSession.test.ts b/sdk/highlight-run/src/__tests__/highlightSession.test.ts new file mode 100644 index 0000000000..052e3fda40 --- /dev/null +++ b/sdk/highlight-run/src/__tests__/highlightSession.test.ts @@ -0,0 +1,140 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import Cookies from 'js-cookie' + +import { + setSessionData, + type SessionData, +} from '../client/utils/sessionStorage/highlightSession' +import { SESSION_PUSH_THRESHOLD } from '../client/constants/sessions' +import { cookieStorage, setItem } from '../client/utils/storage' + +const sessionDataKey = (id: string) => `sessionData_${id}` + +const makeSessionData = ( + id: string, + overrides: Partial = {}, +): SessionData => ({ + sessionSecureID: id, + projectID: 1, + sessionStartTime: Date.now(), + lastPushTime: Date.now(), + ...overrides, +}) + +const clearAllSessionData = () => { + if (typeof window !== 'undefined') { + try { + window.localStorage.clear() + } catch {} + } + for (const key of Object.keys(Cookies.get() ?? {})) { + Cookies.remove(key) + } +} + +describe('pruneSessionData', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(new Date(2024, 0, 1, 12, 0, 0)) + clearAllSessionData() + }) + + afterEach(() => { + clearAllSessionData() + vi.useRealTimers() + }) + + it('removes stale sessionData_* cookies that have no matching localStorage entry', () => { + // Simulate cookies left behind from prior sessions, with no matching + // localStorage entries (e.g. user cleared site data, different subdomain, + // or localStorage was unavailable when they were written). + const stale = makeSessionData('old-1', { + lastPushTime: Date.now() - SESSION_PUSH_THRESHOLD - 1000, + }) + cookieStorage.setItem(sessionDataKey('old-1'), JSON.stringify(stale)) + + const fresh = makeSessionData('new-session') + setSessionData(fresh) + + expect(cookieStorage.getItem(sessionDataKey('old-1'))).toBe('') + expect(cookieStorage.getItem(sessionDataKey('new-session'))).not.toBe( + '', + ) + }) + + it('removes sessionData_* cookies with corrupt/truncated values without logging', () => { + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => {}) + cookieStorage.setItem(sessionDataKey('truncated'), '{"sessionSecu') + + setSessionData(makeSessionData('new-session')) + + expect(cookieStorage.getItem(sessionDataKey('truncated'))).toBe('') + expect(consoleError).not.toHaveBeenCalled() + consoleError.mockRestore() + }) + + it('removes entries with no timestamp metadata', () => { + cookieStorage.setItem( + sessionDataKey('no-ts'), + JSON.stringify({ sessionSecureID: 'no-ts', projectID: 1 }), + ) + + setSessionData(makeSessionData('new-session')) + + expect(cookieStorage.getItem(sessionDataKey('no-ts'))).toBe('') + }) + + it('keeps recent session data from a parallel tab', () => { + // Parallel tabs write to both cookies and localStorage via setItem. + const recent = makeSessionData('parallel', { + lastPushTime: Date.now() - 1000, + }) + setItem(sessionDataKey('parallel'), JSON.stringify(recent)) + + setSessionData(makeSessionData('new-session')) + + expect(cookieStorage.getItem(sessionDataKey('parallel'))).not.toBe('') + expect(cookieStorage.getItem(sessionDataKey('new-session'))).not.toBe( + '', + ) + }) + + it('keeps a fresh cookie-only entry with no matching localStorage value', () => { + // Parallel tab on a different subdomain (or after localStorage was + // cleared) leaves only the cookie behind. The freshness check has to + // fall back to the cookie value or it will unconditionally prune it. + const recent = makeSessionData('cookie-only', { + lastPushTime: Date.now() - 1000, + }) + cookieStorage.setItem( + sessionDataKey('cookie-only'), + JSON.stringify(recent), + ) + + setSessionData(makeSessionData('new-session')) + + expect(cookieStorage.getItem(sessionDataKey('cookie-only'))).not.toBe( + '', + ) + }) + + it('falls back to sessionStartTime when lastPushTime is missing', () => { + const stale = { + sessionSecureID: 'started-never-pushed', + projectID: 1, + sessionStartTime: Date.now() - SESSION_PUSH_THRESHOLD - 1000, + } + cookieStorage.setItem( + sessionDataKey('started-never-pushed'), + JSON.stringify(stale), + ) + + setSessionData(makeSessionData('new-session')) + + expect( + cookieStorage.getItem(sessionDataKey('started-never-pushed')), + ).toBe('') + }) +}) diff --git a/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts b/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts index 7bed02fdd4..bb550c0856 100644 --- a/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts +++ b/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts @@ -103,37 +103,55 @@ export const loadCookieSessionData = function () { function pruneSessionData(keepKey: string): void { const prefix = `${SESSION_STORAGE_KEYS.SESSION_DATA}_` + const candidates = new Set() - // Walk backwards so index order isn’t upset by removals. - for (let i = getPersistentStorage().length - 1; i >= 0; i--) { - const key = getPersistentStorage().key(i) + const storage = getPersistentStorage() + for (let i = storage.length - 1; i >= 0; i--) { + const key = storage.key(i) if (key && key.startsWith(prefix) && key !== keepKey) { - try { - const sessionData = JSON.parse( - getItem(key) || '{}', - ) as SessionData - if ( - sessionData.lastPushTime !== undefined && - Date.now() - sessionData.lastPushTime >= - SESSION_PUSH_THRESHOLD - ) { - internalLogOnce( - 'highlightSession', - 'pruneSessionData', - 'debug', - `removing session data for stale key ${key}`, - ) - removeItem(key) - } - } catch (e) { - internalLogOnce( - 'highlightSession', - 'pruneSessionData', - 'error', - `failed to parse session data for key ${key}`, - e, - ) - } + candidates.add(key) + } + } + + // Cookies are written alongside localStorage but persist independently + // (they survive localStorage being cleared, and outlive many sessions + // via the SESSION_PUSH_THRESHOLD expiry). Without enumerating them here, + // sessionData_* cookies pile up over time and bloat the Cookie header + // enough to break unrelated API calls. + for (const key of cookieStorage.keys()) { + if (key.startsWith(prefix) && key !== keepKey) { + candidates.add(key) + } + } + + for (const key of candidates) { + let stale = true + try { + // Cookie-only entries (e.g. from a parallel tab that hasn't + // hydrated localStorage in this context) won't be found by + // getItem, which only reads persistent storage. Fall back to + // the cookie value so we don't unconditionally prune fresh + // data belonging to active sessions. + const raw = getItem(key) || cookieStorage.getItem(key) || '{}' + const sessionData = JSON.parse(raw) as SessionData + // Fall back to sessionStartTime so entries from sessions that + // ended before the first push still age out. Entries with no + // usable timestamp (foreign / corrupt / truncated) are stale. + const ts = sessionData.lastPushTime ?? sessionData.sessionStartTime + stale = + ts === undefined || Date.now() - ts >= SESSION_PUSH_THRESHOLD + } catch { + // Truncated past the 4KB cookie limit or otherwise corrupt — + // drop silently rather than logging on every page load. + } + if (stale) { + internalLogOnce( + 'highlightSession', + 'pruneSessionData', + 'debug', + `removing session data for stale key ${key}`, + ) + removeItem(key) } } } diff --git a/sdk/highlight-run/src/client/utils/storage.ts b/sdk/highlight-run/src/client/utils/storage.ts index ab00984bd9..204a1d8d98 100644 --- a/sdk/highlight-run/src/client/utils/storage.ts +++ b/sdk/highlight-run/src/client/utils/storage.ts @@ -54,6 +54,10 @@ export class CookieStorage { } Cookies.remove(key) } + + public keys(): string[] { + return Object.keys(Cookies.get() ?? {}) + } } export const globalStorage = new Storage()