Skip to content

Commit 31c558e

Browse files
romtsnclaude
andauthored
fix(replay): Preserve segment ID after buffer-to-session conversion (#5753)
* fix(replay): Preserve segment ID after buffer-to-session conversion 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> * changelog * Apply suggestion from @romtsn * Add comment explaining isFlushed flag purpose Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(replay): Use persisted timestamp for flushed buffer recovery 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> * Revert timestamp normalization change 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> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f232e4f commit 31c558e

7 files changed

Lines changed: 50 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
### Fixes
3232

33+
- Session Replay: Fix first recording segment missing for replays in `buffer` mode ([#5753](https://github.com/getsentry/sentry-java/pull/5753))
3334
- Prevent logs and metrics from remaining queued after a flush scheduling race ([#5756](https://github.com/getsentry/sentry-java/pull/5756))
3435
- Fix main thread identification for tombstone (native crash) events ([#5742](https://github.com/getsentry/sentry-java/pull/5742))
3536

sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
317317
internal const val SEGMENT_KEY_REPLAY_SCREEN_AT_START = "replay.screen-at-start"
318318
internal const val SEGMENT_KEY_REPLAY_RECORDING = "replay.recording"
319319
internal const val SEGMENT_KEY_ID = "segment.id"
320+
internal const val SEGMENT_KEY_FLUSHED = "replay.flushed"
320321

321322
fun makeReplayCacheDir(options: SentryOptions, replayId: SentryId): File? =
322323
if (options.cacheDirPath.isNullOrEmpty()) {
@@ -415,8 +416,11 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
415416
}
416417

417418
cache.frames.sortBy { it.timestamp }
418-
// TODO: this should be removed when we start sending buffered segments on next launch
419-
val normalizedSegmentId = if (replayType == SESSION) segmentId else 0
419+
val wasFlushed = lastSegment[SEGMENT_KEY_FLUSHED]?.toBooleanStrictOrNull() == true
420+
// In buffer mode, if the buffer was never flushed (no error triggered captureReplay),
421+
// no segments were ever sent, so we normalize to 0. After a flush + conversion to
422+
// session mode, the persisted segmentId is the real sequence number.
423+
val normalizedSegmentId = if (replayType == SESSION || wasFlushed) segmentId else 0
420424
val normalizedTimestamp =
421425
if (replayType == SESSION) {
422426
segmentTimestamp

sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ public class ReplayIntegration(
261261
onSegmentSent = { newTimestamp ->
262262
captureStrategy?.currentSegment = captureStrategy?.currentSegment!! + 1
263263
captureStrategy?.segmentTimestamp = newTimestamp
264+
captureStrategy?.isFlushed = true
264265
},
265266
)
266267
captureStrategy = captureStrategy?.convert()

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import io.sentry.SentryReplayEvent.ReplayType.BUFFER
1313
import io.sentry.SentryReplayEvent.ReplayType.SESSION
1414
import io.sentry.android.replay.ReplayCache
1515
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_BIT_RATE
16+
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FLUSHED
1617
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FRAME_RATE
1718
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_HEIGHT
1819
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_ID
@@ -89,6 +90,11 @@ internal abstract class BaseCaptureStrategy(
8990
get() = cache?.replayCacheDir
9091

9192
override var replayType by persistableAtomic<ReplayType>(propertyName = SEGMENT_KEY_REPLAY_TYPE)
93+
// Tracks whether the buffer was flushed (segments sent to server). Used by fromDisk()
94+
// to decide whether to normalize the segment ID to 0 on crash recovery: if never flushed,
95+
// no segments reached the server, so the recovered segment must be 0.
96+
override var isFlushed: Boolean by
97+
persistableAtomic(initialValue = false, propertyName = SEGMENT_KEY_FLUSHED)
9298

9399
protected val currentEvents: Deque<RRWebEvent> = ConcurrentLinkedDeque()
94100
private val traceIdsLock = Any()

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ internal class BufferCaptureStrategy(
105105

106106
if (segment is ReplaySegment.Created) {
107107
segment.capture(scopes)
108-
109108
// we only want to increment segment_id in the case of success, but currentSegment
110109
// might be irrelevant since we changed strategies, so in the callback we increment
111110
// it on the new strategy already

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ internal interface CaptureStrategy {
2929
val replayCacheDir: File?
3030
var replayType: ReplayType
3131
var segmentTimestamp: Date?
32+
var isFlushed: Boolean
3233

3334
fun start(segmentId: Int = 0, replayId: SentryId = SentryId(), replayType: ReplayType? = null)
3435

sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import io.sentry.SentryOptions
99
import io.sentry.SentryReplayEvent.ReplayType
1010
import io.sentry.android.replay.ReplayCache.Companion.ONGOING_SEGMENT
1111
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_BIT_RATE
12+
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FLUSHED
1213
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FRAME_RATE
1314
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_HEIGHT
1415
import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_ID
@@ -443,7 +444,7 @@ class ReplayCacheTest {
443444
}
444445

445446
@Test
446-
fun `sets segmentId to 0 for buffer mode`() {
447+
fun `sets segmentId to 0 for buffer mode when not flushed`() {
447448
fixture.options.run { cacheDirPath = tmpDir.newFolder()?.absolutePath }
448449
val replayId = SentryId()
449450
val replayCacheFolder =
@@ -474,6 +475,39 @@ class ReplayCacheTest {
474475
assertEquals(0, lastSegment.id)
475476
}
476477

478+
@Test
479+
fun `preserves segmentId for buffer mode when already flushed`() {
480+
fixture.options.run { cacheDirPath = tmpDir.newFolder()?.absolutePath }
481+
val replayId = SentryId()
482+
val replayCacheFolder =
483+
File(fixture.options.cacheDirPath!!, "replay_$replayId").also { it.mkdirs() }
484+
File(replayCacheFolder, ONGOING_SEGMENT).also {
485+
it.writeText(
486+
"""
487+
$SEGMENT_KEY_HEIGHT=912
488+
$SEGMENT_KEY_WIDTH=416
489+
$SEGMENT_KEY_FRAME_RATE=1
490+
$SEGMENT_KEY_BIT_RATE=75000
491+
$SEGMENT_KEY_ID=5
492+
$SEGMENT_KEY_TIMESTAMP=2024-07-11T10:25:21.454Z
493+
$SEGMENT_KEY_REPLAY_TYPE=BUFFER
494+
$SEGMENT_KEY_FLUSHED=true
495+
"""
496+
.trimIndent()
497+
)
498+
}
499+
500+
val screenshot = File(replayCacheFolder, "1720693523997.jpg").also { it.createNewFile() }
501+
screenshot.outputStream().use {
502+
Bitmap.createBitmap(1, 1, ARGB_8888).compress(JPEG, 80, it)
503+
it.flush()
504+
}
505+
506+
val lastSegment = ReplayCache.fromDisk(fixture.options, replayId)!!
507+
508+
assertEquals(5, lastSegment.id)
509+
}
510+
477511
@Test
478512
fun `when screenshot is corrupted, deletes it immediately`() {
479513
ShadowBitmapFactory.setAllowInvalidImageData(false)

0 commit comments

Comments
 (0)