fix(replay): stop touch tracker when session replay is paused#8456
fix(replay): stop touch tracker when session replay is paused#8456tsushanth 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.
Reviewed by Cursor Bugbot for commit 73240d5. Configure here.
| public func pause() { | ||
| SentrySDKLog.debug("[Session Replay] Pausing session") | ||
| stopCaptureScheduler() | ||
| touchTracker?.isEnabled = false |
There was a problem hiding this comment.
Touches stay disabled after restart
High Severity
pause() sets isEnabled to false on the shared touchTracker, but only resume() turns it back on. start() reuses that same tracker without resetting isEnabled, so after stop()/start(), session rotation, or max-duration end, touch capture can stay off for the rest of the process unless a later resume() happens to run.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 73240d5. Configure here.
| guard let self = self else { return } | ||
|
|
||
| if self.startCaptureScheduler(expectedGeneration: schedulerGeneration) { | ||
| self.touchTracker?.isEnabled = true |
There was a problem hiding this comment.
Resume race re-enables paused touches
Medium Severity
resume() sets touchTracker?.isEnabled = true only after startCaptureScheduler returns, with no generation or running-state check in between. A concurrent pause() can stop the scheduler and clear isEnabled, then the queued main-thread resume still flips isEnabled back on, so touches keep buffering while capture stays paused.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 73240d5. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| public func pause() { | ||
| SentrySDKLog.debug("[Session Replay] Pausing session") | ||
| stopCaptureScheduler() | ||
| touchTracker?.isEnabled = false |
There was a problem hiding this comment.
Bug: The touchTracker.isEnabled property is not reset to true when a new session starts, causing touch events to be dropped if the app remains in the foreground.
Severity: HIGH
Suggested Fix
The SentrySessionReplay.start() method should explicitly set touchTracker?.isEnabled = true to ensure the touch tracker is active at the beginning of every new session replay.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift#L236
Potential issue: The `touchTracker.isEnabled` property is set to `false` within the
`pause()` method, which is called when a session replay is stopped. When a new session
starts via `SentrySessionReplay.start()`, this property is not reset to `true`.
Consequently, if a session ends and a new one begins while the app remains in the
foreground, the touch tracker stays disabled for the entire new session, causing all
touch events to be silently ignored. This state only corrects itself if the app is
backgrounded and then foregrounded, which triggers the `resume()` method.
Did we get this right? 👍 / 👎 to inform future reviews.
|
Thanks for fixing the formatting issues! :) Our normal way of working is to open PRs as drafts, then address the bot comments + CI (you can ping me to kick it off) - once that's done, you can move it to ready and request a review from the team. |
2 participants
Summary
SentryTouchTracker.trackTouchFromwas called unconditionally from thesendEventswizzle regardless of replay pause state. CallingSentrySDK.replay.pause()stopped the capture scheduler (no new screenshots) but left the touch tracker running, so touches continued to buffer and could appear in replay segments captured during/after the pause (e.g. triggered by an error capture).Changes
var isEnabled = truetoSentryTouchTracker; gatetrackTouchFromon itSentrySessionReplay.pause(): settouchTracker?.isEnabled = falsealongsidestopCaptureScheduler()SentrySessionReplay.resume(): settouchTracker?.isEnabled = truewhen the capture scheduler successfully restartsTests
Two new unit tests in
SentryTouchTrackerTests:testIsEnabledFalse_DropsAllTouches— verifies no events recorded when disabledtestIsEnabledToggle_OnlyRecordsTouchesWhileEnabled— verifies only touches while enabled appear in replay eventsFixes #8409
#skip-changelog