Repository navigation
fix(replay): Keep pending video when stop follows flush - #9429
Merged
Merged
Conversation
romtsn
requested review from
NinjaLikesCheez,
itaybre,
noahsmartin and
philprime
as code owners
October 23, 2026 09:22
The video encoding completion captured the session replay weakly. SentrySDK.replay.stop() releases the replay, so a stop right after flush() dropped the encoded segment and no replay was sent. Capture the replay strongly in the completion. The closure is released after it runs, so the replay lives only until the pending encoding finishes. Fixes #9320 Fixes COCOA-2050
romtsn
force-pushed
the
fix/replay-flush-stop-pending-video
branch
from
October 23, 2026 09:41
10eaa1e to
7b36d78
Compare
The encoding completion now holds the replay until it finishes, so a segment can arrive after stop(). If start() already made a new replay, that segment overwrote the new replay's global crash-sync segment id and end time. Update the crash-sync info only for segments of the replay it was last started for.
romtsn
added
the
run-full-ci
Size Analysis1 component analyzed iOS Builds
|
📲 Install BuildsiOS
|
itaybre
approved these changes
Oct 23, 2026
itaybre
left a comment
itaybre
left a comment
Contributor
There was a problem hiding this comment.
Almost LGTM, some comments to check
A pending encoding can now release the replay on the processing queue. Hold the root view weakly, so the replay never deallocates a UIView off the main thread. The app owns the window. Reset and update the crash-sync info under the replay id lock, so a late segment check can't interleave with a new replay's reset.
Contributor
Author
romtsn
enabled auto-merge (squash)
October 27, 2026 12:48
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 d89e7bc. Configure here.
A rate-limited segment stops the current replay. A late segment of a stopped replay could now reach that path and stop the replay that replaced it, which may be buffering and would outlive the rate limit. Apply the current-replay-id guard to the rate-limit branch as well, and rename the id to currentReplayId since it now serves both checks. Existing rate-limit tests send segments for the current replay.
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
The video encoding completion in
SentrySessionReplay.createAndCaptureInBackgroundcaptured the replay with[weak self].SentrySDK.replay.stop()releases the replay, so whenstop()comes right afterflush(), encoding finishes after the replay is gone and the segment is dropped. No replay envelope is sent, in buffer or session mode.This PR captures the replay strongly in the completion. The video maker releases the closure after it runs, so the replay lives only until the pending encoding finishes. In session mode, the pause segment that
stop()creates is now sent as well.Because a segment can now arrive after
stop(), astart()in that window would let the old replay's late segment overwrite the new replay's global crash-sync info (segment id and last segment end). The same late segment, if rate limited, would also stop the new replay, which may be buffering and would otherwise outlive the rate limit. The integration now applies both the crash-sync update and the rate-limit stop only to segments of the current replay. A late segment still clears the shared touch tracker's finished touches; that needs a touch tracker per replay and is left as is.This is the Cocoa side of the Android fix in getsentry/sentry-java#6340. The other two Android problems do not apply here:
💡 Motivation and Context
Fixes #9320
Fixes COCOA-2050
💚 How did you test it?
testFlush_whenReleasedBeforeVideoCreationCompletes_shouldStillSendSegment: flush, pause, release the replay, then finish encoding. It fails without the fix and passes with it.testDealloc_DoesNotRetainSessionReplayDuringVideoCreationis nowtestDealloc_RetainsSessionReplayOnlyUntilVideoCreationCompletes. It checks that the replay is released after encoding finishes.testSessionReplayNewSegment_whenSegmentOfStoppedReplay_shouldNotOverwriteCrashInfo: stop, start, then send a segment for the old and the new replay id. It fails without the guard.testSessionReplayNewSegment_whenRateLimitedSegmentOfStoppedReplay_shouldNotStopCurrentReplay: a rate-limited segment of the stopped replay leaves the current one running, one of the current replay stops it. It fails without the guard. The existing rate-limit tests now send segments for the current replay instead of a random id.SentrySessionReplayIntegrationTests(94 tests) andSentrySessionReplayTests(76 tests) pass on the iOS 18.6 simulator.make formatandmake analyzeare clean.📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.