fix(replay): Preserve segment ID after buffer-to-session conversion#5753
Open
romtsn wants to merge 6 commits into
Open
fix(replay): Preserve segment ID after buffer-to-session conversion#5753romtsn wants to merge 6 commits into
romtsn wants to merge 6 commits into
Conversation
After a buffer-to-session conversion, the replay type stays BUFFER but the segment counter reflects the real sequence. If the app crashes and finalizePreviousReplay recovers the last segment, fromDisk() was normalizing the segment ID to 0 for all BUFFER replays, creating a duplicate segment 0 that overwrites the original. Add a persisted isFlushed flag set when the buffer is successfully flushed. fromDisk() now only normalizes to 0 when the buffer was never flushed (no segments were ever sent to the server). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| abfcc92 | 309.54 ms | 380.32 ms | 70.78 ms |
| b3d8889 | 371.69 ms | 432.96 ms | 61.26 ms |
| bdbe1f4 | 380.66 ms | 464.44 ms | 83.78 ms |
| 092f017 | 353.13 ms | 433.84 ms | 80.71 ms |
| b6702b0 | 395.86 ms | 409.98 ms | 14.12 ms |
| fcec2f2 | 357.47 ms | 447.32 ms | 89.85 ms |
| 8c7718c | 307.42 ms | 374.84 ms | 67.42 ms |
| d15471f | 310.26 ms | 377.04 ms | 66.78 ms |
| 4c04bb8 | 333.16 ms | 408.16 ms | 75.00 ms |
| d501a7e | 307.33 ms | 341.94 ms | 34.61 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| abfcc92 | 1.58 MiB | 2.13 MiB | 557.31 KiB |
| b3d8889 | 1.58 MiB | 2.10 MiB | 535.06 KiB |
| bdbe1f4 | 1.58 MiB | 2.11 MiB | 538.88 KiB |
| 092f017 | 0 B | 0 B | 0 B |
| b6702b0 | 1.58 MiB | 2.12 MiB | 551.79 KiB |
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| 8c7718c | 0 B | 0 B | 0 B |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| 4c04bb8 | 0 B | 0 B | 0 B |
| d501a7e | 0 B | 0 B | 0 B |
Previous results on branch: rz/fix/replay-segment-id-normalization
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 94ec28b | 317.49 ms | 407.84 ms | 90.35 ms |
| 39e4c55 | 352.43 ms | 446.94 ms | 94.51 ms |
| 048e40b | 322.47 ms | 362.69 ms | 40.22 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 94ec28b | 0 B | 0 B | 0 B |
| 39e4c55 | 0 B | 0 B | 0 B |
| 048e40b | 0 B | 0 B | 0 B |
romtsn
commented
Jul 10, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 49846bc. Configure here.
After buffer-to-session conversion, use the persisted segmentTimestamp (which chains with previous segments) instead of the first frame timestamp, avoiding gaps in the recovered segment timeline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The gap from using first-frame timestamp vs persisted segmentTimestamp is at most ~1s (1/frameRate) — negligible for crash recovery. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

📜 Description
After a buffer-to-session conversion (
BufferCaptureStrategy.convert()), thereplayTypestaysBUFFER(intentionally — it tells the backend this replay was triggered by an error). However, if the app crashes after the conversion andfinalizePreviousReplay()recovers the last segment on next launch,ReplayCache.fromDisk()was normalizing the segment ID to 0 for allBUFFER-type replays.This created a duplicate segment 0 with a late timestamp that overwrote the original segment 0 from the buffer flush, effectively losing the first segment of the replay.
The fix adds a persisted
isFlushedflag that is set when the buffer is successfully flushed.fromDisk()now only normalizes the segment ID to 0 when the buffer was never flushed (i.e., no segments were ever sent to the server — the crash happened before any error triggeredcaptureReplay).💡 Motivation and Context
Customers reported the first segment of error-triggered replays sometimes being missing or showing content from the wrong time range. This was caused by
finalizePreviousReplaycreating a duplicate segment 0 that replaced the original.💚 How did you test it?
ReplayCacheTestcaptureException, waited for session segments, force-killed, relaunched — verified the finalized segment has the correct sequential ID (not 0)📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
captureReplayin buffer mode)