Skip to content

fix(replay): Keep flushed video and segment id when stop follows flush - #6340

Merged
romtsn merged 3 commits into
mainfrom
fix/replay-keep-video-on-empty-frames
Oct 21, 2026
Merged

romtsn merged 3 commits into
mainfrom
fix/replay-keep-video-on-empty-frames

Conversation

@romtsn

@romtsn romtsn commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

📜 Description

Calling stop() right after flush() in buffer mode no longer loses the flushed replay video, and no longer sends two segments with the same segment_id.

The flush encodes segment 0 on the replay executor, then the buffer strategy is converted to a session strategy that starts at segment 0. When the flushed segment is done, a callback advances the session strategy to segment 1. Since #5965 that callback hopped to the main thread and was dropped if the replay was no longer recording. A stop() queued right after flush() always runs first, and its stop segment is already queued on the executor behind the flush. So the session created its stop segment as segment 0 again, at the same 0.mp4 path:

  • No frames in the session (native recorder): ReplayCache.createVideoOf deleted the existing 0.mp4 before it checked for frames, so segment 0 was sent with no video.
  • Frames in the session (e.g. a hybrid SDK passing frames via onScreenshotRecorded(File, …)): the stop segment re-encoded 0.mp4 with the session frames, so segment 0 carried the wrong video and a second segment_id=0 envelope followed with no video.

Two changes:

  • ReplayIntegration.captureCurrentReplay converts the strategy before submitting the capture, and the callback advances currentSegment, segmentTimestamp and isFlushed directly on the replay executor, which both strategies share. Every other currentSegment write already runs there, and the session reads it when its segment task runs, so the update always lands before the next session segment. The callback updates only the converted strategy object it captured, so a stopped or restarted replay is not affected (stale capture callback cannot mutate restarted replay still passes).
  • ReplayCache.createVideoOf deletes an existing video only after it knows it has frames to encode, so a call with no frames never removes a video that is still waiting for the transport.

Review focus: convert() now runs before the buffer capture is submitted instead of after. Both are synchronous on the main thread, and the synchronous part of BufferCaptureStrategy.captureReplay reads only buffer-owned state, so the order of executor work does not change.

💡 Motivation and Context

💚 How did you test it?

Unit tests in ReplayIntegrationTest and ReplayCacheTest; the integration test fails without the fix.

On an API 34 emulator with the sample app (session sample rate 0, PixelCopy recorder) and a local mock ingest server that reads each replay envelope:

Scenario main This PR
startBuffering(), 5 s, then flush() + stop() segment_id=0, 0 video bytes segment_id=0, 12,244 bytes
Same, plus 2 frames via onScreenshotRecorded(File, …) before stop() two segment_id=0 envelopes: 4,423 bytes (session frames), then 0 bytes segment_id=0 14,350 bytes, segment_id=1 4,423 bytes

The ReplayCache change alone fixes the first row but not the second.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

Related issues found while verifying, to be tracked separately:

  • The startup cleanup of previous replays (finalizePreviousReplay → cleanupReplays) can delete the folder of a replay that started and stopped before the task ran, including videos still waiting for the transport.
  • CanvasStrategy.close() can recycle the screenshot bitmap while a PixelCopy request still writes into it, which aborts the RenderThread.

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Oct 21, 2026

Copy link
Copy Markdown

JAVA-772

@sentry

sentry Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.61.0 (1) release

⚙️ sentry-android Build Distribution Settings

romtsn and others added 2 commits October 21, 2026 12:10
createVideoOf deleted the existing segment video before checking for captured frames, so an
immediate stop() after flush() in buffer mode removed the flushed video before the transport
read it, and segment 0 was sent without video.

The segment-sent callback that advances the converted session strategy hopped to the main
thread and was dropped when stop() ran first, so the session reused the flushed segment id for
its stop segment and overwrote the flushed video. Advance it on the replay executor the
strategies share instead, which orders it before any later segment of the session.

Fixes #6247

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@romtsn
romtsn force-pushed the fix/replay-keep-video-on-empty-frames branch from faa7970 to fb21910 Compare October 21, 2026 10:14
The regression test for #6247 pushed ReplayCacheTest over the LargeClass threshold, like ReplayIntegrationTest already is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@romtsn
romtsn merged commit 814ae44 into main Oct 21, 2026
71 of 72 checks passed
@romtsn
romtsn deleted the fix/replay-keep-video-on-empty-frames branch October 21, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replay flush followed by immediate stop deletes the buffered video

2 participants