Skip to content

fix: handle invalid session replay config JSON #3561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marandaneto merged 1 commit into from
May 9, 2026
Merged

fix: handle invalid session replay config JSON #3561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quiet-badgers-parse.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'posthog-js': patch
---

Handle invalid persisted session replay config JSON gracefully
20 changes: 20 additions & 0 deletions packages/browser/src/__tests__/extensions/replay/lazy-sessionrecording.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,26 @@ describe('Lazy SessionRecording', () => {
expect(result?.enabled).toBe(true)
})

it('ignores invalid persisted JSON config when checking freshness', () => {
posthog.persistence?.register({
[SESSION_RECORDING_REMOTE_CONFIG]: '{not json',
})

expect(sessionRecording['_isRemoteConfigFresh']()).toBe(false)
expect(posthog.get_property(SESSION_RECORDING_REMOTE_CONFIG)).toBe('{not json')
})

it('ignores invalid persisted JSON config when reading remote config', () => {
posthog.persistence?.register({
[SESSION_RECORDING_REMOTE_CONFIG]: '{not json',
})

const result = sessionRecording['_lazyLoadedSessionRecording']['_remoteConfig']

expect(result).toBeUndefined()
expect(posthog.get_property(SESSION_RECORDING_REMOTE_CONFIG)).toBe('{not json')
})

it('trusts stale config once recording has started (long-lived SPA)', () => {
expect(sessionRecording['_lazyLoadedSessionRecording'].isStarted).toBe(true)

Expand Down
17 changes: 17 additions & 0 deletions packages/browser/src/__tests__/posthog-core.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,5 +585,22 @@ describe('posthog core', () => {
registerSpy.mockRestore()
captureSpy.mockRestore()
})

it('should not abort queued calls when one call throws', () => {
const posthog = defaultPostHog()
const captureSpy = jest.spyOn(posthog, 'capture').mockImplementation()
;(posthog as any).parseInvalidJson = (payload: string) => JSON.parse(payload)

expect(() => {
posthog._execute_array([
['parseInvalidJson', '{not json'],
['capture', 'test-event'],
])
}).not.toThrow()

expect(captureSpy).toHaveBeenCalledWith('test-event')
captureSpy.mockRestore()
delete (posthog as any).parseInvalidJson
})
})
})
10 changes: 9 additions & 1 deletion packages/browser/src/extensions/replay/external/lazy-loaded-session-recorder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,15 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
if (!persistedConfig) {
return undefined
}
const parsedConfig = isObject(persistedConfig) ? persistedConfig : JSON.parse(persistedConfig)
let parsedConfig: SessionRecordingPersistedConfig
try {
parsedConfig = isObject(persistedConfig) ? persistedConfig : JSON.parse(persistedConfig)
} catch (e) {
// Do not unregister here: the SDK only registers structured configs, and this read path should
// ignore corrupt legacy/external values without mutating persistence.
logger.warn('persisted remote config for session recording is invalid and will be ignored', e)
return undefined
}

// Only check TTL if recording hasn't started yet
// Once started, trust the config until a hard page load
Expand Down
10 changes: 9 additions & 1 deletion packages/browser/src/extensions/replay/session-recording.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,15 @@ export class SessionRecording implements Extension {
if (!persistedConfig) {
return false
}
const config = typeof persistedConfig === 'object' ? persistedConfig : JSON.parse(persistedConfig)
let config: SessionRecordingPersistedConfig
try {
config = typeof persistedConfig === 'object' ? persistedConfig : JSON.parse(persistedConfig)

@greptile-apps greptile-apps Bot May 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The object-type guard here uses typeof persistedConfig === 'object', while the equivalent block in lazy-loaded-session-recorder.ts uses isObject(persistedConfig). These two checks differ in behaviour for null (which passes typeof === 'object' but not isObject). While both sites guard against null via the earlier !persistedConfig check, the inconsistency makes the intent harder to follow and could silently diverge if the guard is copied elsewhere. Both sites should use the same helper.

Suggested change
config = typeof persistedConfig === 'object' ? persistedConfig : JSON.parse(persistedConfig)
config = isObject(persistedConfig) ? persistedConfig : JSON.parse(persistedConfig)
Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/browser/src/extensions/replay/session-recording.ts
Line: 278

Comment:
The object-type guard here uses `typeof persistedConfig === 'object'`, while the equivalent block in `lazy-loaded-session-recorder.ts` uses `isObject(persistedConfig)`. These two checks differ in behaviour for `null` (which passes `typeof === 'object'` but not `isObject`). While both sites guard against `null` via the earlier `!persistedConfig` check, the inconsistency makes the intent harder to follow and could silently diverge if the guard is copied elsewhere. Both sites should use the same helper.

```suggestion
            config = isObject(persistedConfig) ? persistedConfig : JSON.parse(persistedConfig)
```

How can I resolve this? If you propose a fix, please make it concise.

} catch (e) {
// Do not unregister here: the SDK only registers structured configs, and this read path should
// ignore corrupt legacy/external values without mutating persistence.
logger.warn('persisted remote config for session recording is invalid and will be ignored', e)
return false
}
Comment on lines +276 to +284

@greptile-apps greptile-apps Bot May 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Duplicated safe-parse logic across two files

The try/catch block here — including its body, the identical comment, and the identical logger.warn message — is copy-pasted verbatim into lazy-loaded-session-recorder.ts (_remoteConfig getter, same shape). This violates OnceAndOnlyOnce. A small shared helper such as parsePersistedReplayConfig(raw): SessionRecordingPersistedConfig | null that returns null on parse failure would let both callers handle the error uniformly and remove the duplicated comment explaining why mutation is skipped.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/browser/src/extensions/replay/session-recording.ts
Line: 276-284

Comment:
**Duplicated safe-parse logic across two files**

The try/catch block here — including its body, the identical comment, and the identical `logger.warn` message — is copy-pasted verbatim into `lazy-loaded-session-recorder.ts` (`_remoteConfig` getter, same shape). This violates OnceAndOnlyOnce. A small shared helper such as `parsePersistedReplayConfig(raw): SessionRecordingPersistedConfig | null` that returns `null` on parse failure would let both callers handle the error uniformly and remove the duplicated comment explaining why mutation is skipped.

How can I resolve this? If you propose a fix, please make it concise.

// default to now so that configs persisted by older SDK versions
// (which never set cache_timestamp) are treated as fresh
const cacheTimestamp = config.cache_timestamp ?? Date.now()
Expand Down
26 changes: 17 additions & 9 deletions packages/browser/src/posthog-core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,11 @@ export class PostHog implements PostHogInterface {
if (isArray(fn_name)) {
capturing_calls.push(item) // chained call e.g. posthog.get_group().set()
} else if (isFunction(item)) {
;(item as any).call(this)
try {
;(item as any).call(this)
} catch (e) {
logger.error('Error executing queued PostHog call', item, e)
}
} else if (isArray(item) && fn_name === 'alias') {
alias_calls.push(item)
} else if (
Expand All @@ -1107,14 +1111,18 @@ export class PostHog implements PostHogInterface {

const execute = function (calls: SnippetArrayItem[], thisArg: any) {
eachArray(calls, function (item) {
if (isArray(item[0])) {
// chained call
let caller = thisArg
each(item, function (call) {
caller = caller[call[0]].apply(caller, call.slice(1))
})
} else {
thisArg[item[0]].apply(thisArg, item.slice(1))
try {
if (isArray(item[0])) {
// chained call
let caller = thisArg
each(item, function (call) {
caller = caller[call[0]].apply(caller, call.slice(1))
})
} else {
thisArg[item[0]].apply(thisArg, item.slice(1))
}
} catch (e) {
logger.error('Error executing queued PostHog call', item, e)
}
})
}
Expand Down
Loading