fix: handle invalid session replay config JSON#3561
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
marandaneto
requested review from
a team,
TueHaulund,
arnohillen,
fasyy612,
ksvat and
nicowaltz
and removed request for
a team
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/browser/src/extensions/replay/session-recording.ts:278
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)
```
### Issue 2 of 2
packages/browser/src/extensions/replay/session-recording.ts:276-284
**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.
Reviews (1): Last reviewed commit: "fix: handle invalid queued replay config..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const config = typeof persistedConfig === 'object' ? persistedConfig : JSON.parse(persistedConfig) | ||
| let config: SessionRecordingPersistedConfig | ||
| try { | ||
| config = typeof persistedConfig === 'object' ? persistedConfig : JSON.parse(persistedConfig) |
Contributor
There was a problem hiding this comment.
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.
Comment on lines
+276
to
+284
| let config: SessionRecordingPersistedConfig | ||
| try { | ||
| config = typeof persistedConfig === 'object' ? 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 false | ||
| } |
Contributor
There was a problem hiding this comment.
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.
Contributor
Contributor
|
Size Change: +3.46 kB (+0.02%) Total Size: 15.9 MB
ℹ️ View Unchanged
|
arnohillen
approved these changes
May 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Invalid JSON in persisted session replay config, or in a queued snippet call executed during SDK startup, can throw and interrupt PostHog initialization/queued call processing instead of failing gracefully.
Closes #3540
Changes
_execute_arrayso one bad queued call does not abort later queued calls.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file