Fix session replay dropping screens inside Throttler window

[tsushanth]

Fixes #596

Problem

When a user navigates to a new screen during the throttle window, the screen is silently dropped from the session replay recording.

Throttler.throttle() schedules a postDelayed for the first draw that arrives after a snapshot and sets isThrottling = true. Any subsequent draws while isThrottling is true hit the else branch and are silently discarded — no new postDelayed is queued. If the user navigates A → B → C within a single throttle window:

1. B's first draw: isThrottling is false → postDelayed scheduled, isThrottling = true
2. C's draws: isThrottling is true → all silently dropped
3. postDelayed fires → snapshots whatever is on screen now (C) → produces an A→C diff

Screen B never appears in the recording.

Fix

Add a hasPendingDraw flag. When a draw is discarded (isThrottling == true), set the flag. After the delayed snapshot fires, check the flag and call throttle() once more so the latest view tree is always captured.

@Volatile private var hasPendingDraw = false

// in postDelayed lambda:
val pendingAfter = hasPendingDraw
hasPendingDraw = false
executeAndReleaseThrottle(runnable)
if (pendingAfter) { throttle(runnable) }

// in else branch (was: silently dropped):
hasPendingDraw = true

1 file changed.

[greptile-apps]

Comments Outside Diff (1)

1. posthog-android/src/main/java/com/posthog/android/replay/internal/Throttler.kt, line 25-55 (link)

   No tests for the new pending-draw behavior. The PR description neatly describes the A→B→C scenario (navigate to B, then C, within a single throttle window), which would translate directly into a parameterised unit test verifying that the runnable is invoked a second time after the delayed snapshot fires. There are currently no tests for Throttler at all, so this is a good opportunity to add coverage before the edge-case logic grows further.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "Fix session replay dropping screens in T..." | Re-trigger Greptile}

[greptile-apps]

hasPendingDraw uses @Volatile while the existing isThrottling field uses AtomicBoolean, which provides atomic compound operations via getAndSet. The read-then-clear in the postDelayed lambda (val pendingAfter = hasPendingDraw; hasPendingDraw = false) is not atomic. If throttle() is ever called off the main thread (which the AtomicBoolean on isThrottling leaves room for), a draw signal set between the read and the clear would be silently lost. Using AtomicBoolean.getAndSet(false) matches the existing pattern and makes the operation atomic.

Suggested change

	@Volatile private var hasPendingDraw = false
	private val hasPendingDraw = AtomicBoolean(false)

[greptile-apps]

If hasPendingDraw is changed to AtomicBoolean, this read-then-clear becomes a single atomic getAndSet(false), eliminating the non-atomic window.

Suggested change

	val pendingAfter = hasPendingDraw
	hasPendingDraw = false
	executeAndReleaseThrottle(runnable)
	val pendingAfter = hasPendingDraw.getAndSet(false)
	executeAndReleaseThrottle(runnable)

[greptile-apps]

Companion to the AtomicBoolean change: use set(true) for consistency.

Suggested change

	hasPendingDraw = true
	hasPendingDraw.set(true)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[ioannisj]

Let's add a changeset for this. Greptile comments look reasonable to me. Also, I'm not sure if this fixes #596 since completely but maybe partially addresses it.

[dustinbyrne]

Hi @tsushanth, thanks for the pull request. An agent found the following finding, it's worth verifying:

---

retry runs after the skipped screen is gone

Test sketch (not executed): at t=1000 draw A; t=1100 draw B; t=1200 draw C; run delayed work at t=2000 and t=3000.

Expected a sequence containing B; current code snapshots current state C at both deadlines, producing A,C,C.

Trigger: ordinary replay with two state changes inside the default 1s throttle window.

Entry: PostHogAndroid.setup → replay onDraw → NextDrawListener → Throttler → generateSnapshot.

Fix: capture/retain immutable pending state before it is replaced, or narrow the claim and issue closure. Add state-based A→B→C, single-pending-draw, continuous-draw, and no-pending-draw tests.

[tsushanth]

Hey @dustinbyrne, thanks for digging into this — the concern is valid and I want to be transparent about what this PR does and doesn't fix.

Why the retry still captures C, not B

NextDrawListener.onDraw() passes { onDrawThrottlerCallback() } to throttle() on every draw. The lambda closes over onDrawThrottlerCallback — a single reference set at construction time — so it reads the current view tree at the moment it runs, not the moment it was enqueued. This means:

• Draw A fires → snapshot taken at t=1000 (A)
• Draw B fires → postDelayed scheduled (B's lambda stored in isThrottling path), isThrottling=true
• Draw C fires → hasPendingDraw = true, C's lambda discarded
• postDelayed fires at ~t=2000 → runs B's lambda → snapshots current screen (C) → hasPendingDraw=true so calls throttle() again
• Second snapshot at t=3000 → snapshots C again

Result: A, C, C — B is still missing. You're right.

What this PR actually fixes

The hasPendingDraw retry does eliminate one class of loss: if the user navigates A → B and B arrives right at the throttle boundary, the retry ensures B is captured after the window (since B is still on screen when the lambda runs). But it doesn't help for A → B → C within the same window.

A proper fix

Fixing B→C within a window would require onDrawThrottlerCallback to capture a snapshot of the view tree eagerly (at onDraw() time), not lazily (at runnable.run() time). That's an architectural change upstream of Throttler — the replay code that creates NextDrawListener would need to snapshot the tree and pass the serialised data, rather than passing a function that reads the tree later.

I'm happy to either:

1. Narrow this PR to what it actually delivers: eliminating the "pending draw silently dropped after window" case where the skipped screen is still visible when the retry fires, and update the issue/PR description accordingly.
2. Tackle the eager-snapshot approach in a follow-up if that's the direction you'd prefer.

Let me know which way you'd like to go.