From 04514c31fd667e2d50493dbdbff71b75b28d66bb Mon Sep 17 00:00:00 2001 From: Vadim Korolik Date: Wed, 13 May 2026 16:13:06 +0000 Subject: [PATCH 1/2] fix: prune stale sessionData_* cookies and stop noisy parse log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setItem writes session data to both localStorage and cookies, but pruneSessionData only walked localStorage. As a result, sessionData_* cookies accumulated indefinitely whenever localStorage and cookies fell out of sync (cleared site data, subdomain mismatch, parallel tabs, etc.), eventually bloating the Cookie header enough to break unrelated API calls. Walk cookies too, fall back to sessionStartTime so sessions that ended before the first push still age out, and drop entries with no usable timestamp. The error-level "failed to parse session data" log fired on every page load when a cookie was truncated past 4KB — exactly the bloat scenario — so silently remove corrupt entries instead. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/__tests__/highlightSession.test.ts | 121 ++++++++++++++++++ .../utils/sessionStorage/highlightSession.ts | 70 +++++----- sdk/highlight-run/src/client/utils/storage.ts | 4 + 3 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 sdk/highlight-run/src/__tests__/highlightSession.test.ts 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..428268fe2b --- /dev/null +++ b/sdk/highlight-run/src/__tests__/highlightSession.test.ts @@ -0,0 +1,121 @@ +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('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..01ea4bbc3a 100644 --- a/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts +++ b/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts @@ -103,37 +103,49 @@ 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 { + const sessionData = JSON.parse(getItem(key) || '{}') 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() From d12d7664e0f5a0d69865d267af19cd827bb5c0bf Mon Sep 17 00:00:00 2001 From: Vadim Korolik Date: Thu, 14 May 2026 16:46:20 +0000 Subject: [PATCH 2/2] fix: read cookie value when localStorage entry is missing during prune The freshness check in pruneSessionData only read from persistent storage via getItem, so cookie-only entries (different subdomain, localStorage cleared, parallel tab not yet hydrated) always parsed as {} with no timestamps and were unconditionally pruned. Fall back to cookieStorage.getItem(key) so fresh cookie-only data from active sessions is preserved. --- .../src/__tests__/highlightSession.test.ts | 19 +++++++++++++++++++ .../utils/sessionStorage/highlightSession.ts | 8 +++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/sdk/highlight-run/src/__tests__/highlightSession.test.ts b/sdk/highlight-run/src/__tests__/highlightSession.test.ts index 428268fe2b..052e3fda40 100644 --- a/sdk/highlight-run/src/__tests__/highlightSession.test.ts +++ b/sdk/highlight-run/src/__tests__/highlightSession.test.ts @@ -101,6 +101,25 @@ describe('pruneSessionData', () => { ) }) + 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', diff --git a/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts b/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts index 01ea4bbc3a..bb550c0856 100644 --- a/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts +++ b/sdk/highlight-run/src/client/utils/sessionStorage/highlightSession.ts @@ -127,7 +127,13 @@ function pruneSessionData(keepKey: string): void { for (const key of candidates) { let stale = true try { - const sessionData = JSON.parse(getItem(key) || '{}') as SessionData + // 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.