fix(replay): ship the old session's tail on a session-id rotation - #4770
fix(replay): ship the old session's tail on a session-id rotation#4770fasyy612 wants to merge 11 commits into
Conversation
…n instead of discard
Replay incident risk checkThis diff touches code involved in past incidents. This is a heads-up, not a verdict: read the matched sections of INCIDENTS.md and answer their review questions before merging. For a judgment on whether this diff has the same failure mode, run the |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 2 · PR risk: 0/10 |
posthog-js Compliance ReportDate: 2026-09-04 15:21:26 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review.
|
Size Change: +4.81 kB (+0.02%) Total Size: 20.5 MB 📦 View Changed
ℹ️ View Unchanged
|
ksvat
left a comment
There was a problem hiding this comment.
the fix looks solid. one thing on the test: it's timing-dependent (the header even says run with --repeat-each>=3) but nothing makes CI repeat it, and retries:2 is on. so a real race failure can get retried into a green. can you pin retries: 0 on this spec and maybe repeat it a few times? otherwise it can pass while broken.
marandaneto
left a comment
There was a problem hiding this comment.
2 comments left worth checking otherwise lgtm
c760795 to
20e2f56
Compare
Problem
When a session rotates, the recorder stops and restarts from inside an rrweb emit.
stop()waited for the async compression queue to drain before its final flush, but the rotation's synchronousstart()invalidated that drain. Whatever was still queued from the old session was dropped or shipped under the new session id. The same thing happened onreset(), with the tail also landing under the new distinct_id.Changes
stop()drains the compression queue synchronously instead of deferring.reset()flushes the recording tail before persistence is cleared, so it ships under the pre-reset identity. This also runs when a deferred stop is already in flight, since the buffer is still held then.beforeunload, where a throw was harmless. On the rotation path a throw would abort the restart.start()bail on tabs open past an hour.Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted) — or — Fully autonomous