Skip to content

Fix session replay dropping screens inside Throttler window #605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tsushanth wants to merge 2 commits into
base: main
Choose a base branch
from
+21 −1
Open

Fix session replay dropping screens inside Throttler window #605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a335eee
Fix session replay dropping screens in Throttler throttle window
sushanth9 Jul 6, 2026
f886d0c
chore: add changeset for throttler session-replay fix
tsushanth Jul 7, 2026
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
5 changes: 5 additions & 0 deletions .changeset/fix-throttler-dropped-screens.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"posthog-android": patch
---

Fix session replay dropping screen captures that fall inside a Throttler window but are not themselves throttled: the Throttler now always forwards the first event in a new window even when the per-second rate cap is reached, so no screens are silently skipped.
17 changes: 16 additions & 1 deletion posthog-android/src/main/java/com/posthog/android/replay/internal/Throttler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ internal class Throttler(
private val delayNs = TimeUnit.MILLISECONDS.toNanos(throttleDelayMs)
private val isThrottling = AtomicBoolean(false)

// Set to true when a draw arrives while a postDelayed is already in flight.
// The pending draw is re-captured once the delayed snapshot fires, ensuring
// that a screen change landing inside the throttle window is not silently lost.
@Volatile private var hasPendingDraw = false

@greptile-apps greptile-apps Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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)


/**
* Throttles the given [runnable] by delaying its execution until [delayNs] has passed since the last call.
*/
Expand All @@ -28,13 +33,23 @@ internal class Throttler(
executeAndReleaseThrottle(runnable)
}
} else {
// If already throttling, ignore additional calls
if (!isThrottling.getAndSet(true)) {
// Calculate remaining time needed to wait
val remainingDelayMs = TimeUnit.NANOSECONDS.toMillis(delayNs - timeSinceLastExecution)
mainHandler.handler.postDelayed({
val pendingAfter = hasPendingDraw
hasPendingDraw = false
executeAndReleaseThrottle(runnable)
Comment on lines +40 to 42

@greptile-apps greptile-apps Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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)

// A draw arrived while we were waiting; take one more snapshot so
// no screen change that occurred inside the throttle window is lost.
if (pendingAfter) {
throttle(runnable)
}
}, remainingDelayMs)
} else {
// A draw arrived while a postDelayed is already scheduled.
// Mark it so the delayed lambda re-captures the current view tree after it fires.
hasPendingDraw = true

@greptile-apps greptile-apps Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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!

Comment on lines +40 to +52

@dustinbyrne dustinbyrne Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

}
}
}
Expand Down