Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ private struct SessionSegmentState {
public func pause() {
SentrySDKLog.debug("[Session Replay] Pausing session")
stopCaptureScheduler()
touchTracker?.isEnabled = false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 73240d5. Configure here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


let pauseDate = dateProvider.date()
let shouldPreparePauseSegment = state.withLock { (state: inout State) -> Bool in
Expand Down Expand Up @@ -282,6 +283,7 @@ private struct SessionSegmentState {
guard let self = self else { return }

if self.startCaptureScheduler(expectedGeneration: schedulerGeneration) {
self.touchTracker?.isEnabled = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 73240d5. Configure here.

self.resetCapturePacing(at: self.dateProvider.date())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ import UIKit
self.init(dateProvider: dateProvider, scale: scale, dispatchQueue: SentryDispatchQueueWrapper())
}

var isEnabled = true

public func trackTouchFrom(event: UIEvent) {
guard isEnabled else { return }
guard let touches = event.allTouches else { return }
let timestamp = event.timestamp

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,5 +509,42 @@ class SentryTouchTrackerTests: XCTestCase {
"Touches should have different IDs")
XCTAssertNotNil(firstTouchPointerId, "First touch events should not be orphaned")
}

func testIsEnabledFalse_DropsAllTouches() {
let sut = getSut()
sut.isEnabled = false

let event = MockUIEvent(timestamp: 1)
event.addTouch(MockUITouch(phase: .began, location: CGPoint(x: 10, y: 10)))
sut.trackTouchFrom(event: event)

let result = sut.replayEvents(from: referenceDate, until: referenceDate.addingTimeInterval(5))
XCTAssertTrue(result.isEmpty, "No touch events should be recorded when isEnabled is false")
}

func testIsEnabledToggle_OnlyRecordsTouchesWhileEnabled() {
let sut = getSut()

let began = MockUIEvent(timestamp: 1)
began.addTouch(MockUITouch(phase: .began, location: CGPoint(x: 10, y: 10)))
sut.trackTouchFrom(event: began)

sut.isEnabled = false
let whilePaused = MockUIEvent(timestamp: 2)
whilePaused.addTouch(MockUITouch(phase: .began, location: CGPoint(x: 20, y: 20)))
sut.trackTouchFrom(event: whilePaused)

sut.isEnabled = true
let afterResume = MockUIEvent(timestamp: 3)
afterResume.addTouch(MockUITouch(phase: .began, location: CGPoint(x: 30, y: 30)))
sut.trackTouchFrom(event: afterResume)

let result = sut.replayEvents(from: referenceDate, until: referenceDate.addingTimeInterval(5))
let timestamps = result.map { $0.timestamp }
XCTAssertTrue(timestamps.contains(referenceDate.addingTimeInterval(1)))
XCTAssertFalse(timestamps.contains(referenceDate.addingTimeInterval(2)),
"Touch recorded while disabled must not appear in replay events")
XCTAssertTrue(timestamps.contains(referenceDate.addingTimeInterval(3)))
}
}
#endif
Loading