fix(replay): stop touch tracker when session replay is paused#8455
fix(replay): stop touch tracker when session replay is paused#8455tsushanth wants to merge 1 commit into
Conversation
SentryTouchTracker.trackTouchFrom was called unconditionally from the sendEvent swizzle regardless of replay pause state. Calling SentrySDK.replay.pause() stopped the capture scheduler (no new screenshots) but left the touch tracker running, so touches continued to buffer and appeared in replay segments produced during or after the pause (e.g. triggered by an error capture). Add isEnabled to SentryTouchTracker and gate trackTouchFrom on it. Set isEnabled = false in SentrySessionReplay.pause() alongside stopCaptureScheduler(), and restore it to true when the capture scheduler successfully restarts in resume(). Fixes getsentry#8409
![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 2 potential issues.
❌ 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 6633498. Configure here.
| state.withLock { | ||
| $0.isSessionPaused = true | ||
|
|
||
| private func startFullReplay(startedAt: Date?) { |
There was a problem hiding this comment.
Touches stay disabled after restart
High Severity
pause() sets the shared touchTracker.isEnabled to false, but only resume() sets it back to true. start() starts the capture scheduler without re-enabling the tracker. Session restarts and stop()/start() go through stopCurrentReplay() → pause() then a new start(), so touch recording can stay off for the rest of the process.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 6633498. Configure here.
| } | ||
|
|
||
| public func trackTouchFrom(event: UIEvent) { | ||
| guard isEnabled else { return } |
There was a problem hiding this comment.
Unsynchronized touch enable flag
Medium Severity
isEnabled is read in trackTouchFrom on the main thread and written from pause() without hopping to main. Public SentrySDK.replay.pause()/stop() and rate-limit teardown can reach pause() off the main thread, so the new flag is shared mutable state without synchronization despite the PR assuming main-thread-only access.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6633498. Configure here.
|
@tsushanth Thank you for your contribution! Looks like your IDE has indented something resulting in a lot of changes where it's mostly white-space - could you reformat it back so we can review easier? Thanks! |
|
Closing this in favor of a clean rewrite — the whitespace churn was from an Xcode reformat that inflated the diff. New PR with only the 3-line logical change: #8456 |
|
Superseded by #8456 — clean 3-line diff with no whitespace churn. |
2 participants
Fixes #8409.
Problem
SentrySDK.replay.pause()stops the capture scheduler (no new screenshots) but the touch tracker swizzle keeps firing unconditionally.SentryTouchTracker.trackTouchFromis called on everysendEvent:regardless of replay pause state, so touches continue to buffer and appear in replay segments produced during or after the pause (e.g. triggered by an error capture).Fix
isEnabled: Bool = truetoSentryTouchTracker, checked at the top oftrackTouchFrom. No thread lock needed — reads/writes happen on the main thread (swizzle always calls on main;pause()/resume()are called on main).SentrySessionReplay.pause()setstouchTracker?.isEnabled = falseimmediately afterstopCaptureScheduler().SentrySessionReplay.resume()restorestouchTracker?.isEnabled = trueonly inside thestartCaptureScheduler(expectedGeneration:)success branch, so stale or racing resume calls that don't actually restart capture don't re-enable tracking early.Tests
Two new tests in
SentryTouchTrackerTests:testIsEnabledFalse_DropsAllTouches— verify zero events recorded whenisEnabled = falsetestIsEnabledToggle_OnlyRecordsTouchesWhileEnabled— verify only touches recorded before disable and after re-enable appear; touch during disabled window is absent#skip-changelog