fix(replay): trust SDK-supplied session_id in handleReplay (follow-up to #318) - #319
Merged
Merged
Conversation
Companion to #318. The replay handler still derived a session by recomputing deviceId from IP+UA, which overrode the SDK-provided session_id (and re-introduced NAT-collision behavior on the replay storage path even when the SDK was using the deviceId override correctly on the track path). With #318 shipped, the SDK's session_id is no longer self-generated: it comes from /track/device-id?deviceId=<our localStorage UUID or stable user id>, which is the same id the track flow uses to create sessions. So the SDK-supplied session_id is now a real row in the `sessions` table and trusting it is safe — which fixes the "Session not found" dashboard error from the earlier attempt (#315 / #317). Behavior: - payload.session_id present (modern SDKs): trust it. Chunks land under the same session row that the track flow created. Dashboard "session details" works. - payload.session_id missing (legacy SDKs): fall back to the IP+UA derivation that's been there all along. No regression.
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
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.
Why this is needed
PR #318 made the SDK pass its localStorage / Firebase-UID-based deviceId to `/track/device-id?deviceId=`, and the server returns the matching session_id from the `sessions` table. The SDK then includes that session_id with every replay chunk.
But the replay handler (`handleReplay`) was still overriding `payload.session_id` by recomputing deviceId from IP + UA and looking up a session for that hash — re-introducing the NAT-collision behavior on the replay storage path:
```
SDK sends: session_id = "real-session-from-sessions-table"
Server overrides: session_id = lookup-by-IP+UA hash (could be a different session)
CH stores: session_replay_chunks.session_id = the-overridden-id
↑ doesn't match sessions.id → "Session not found"
```
Verified on dev: track flow correctly created a session under the SDK's localStorage UUID, but replay chunks landed under a session keyed by the IP+UA hash. Two separate sessions, same human.
The fix
Trust the SDK-supplied session_id when present. Fall back to the IP+UA derivation only when the SDK didn't send one (older SDKs that pre-date the deviceId override path).
```diff
```
Why this is safe now (unlike #315 which was reverted)
PR #315 had the SDK generate its own session_id (a random UUID with no row in `sessions`). Trusting it broke the dashboard's session lookup ("Session not found"). #317 reverted that.
This time the SDK's session_id is not self-generated — it comes from `/track/device-id`, which is a real row in `sessions` for the same deviceId the track flow used. So:
```
sessions.id ← server-issued, real row
sessionId on the wire ← echoed back to SDK, same value
chunks.session_id ← now also that value
```
All three match. Dashboard finds the session. Replay plays.
Backward compatibility
Test plan