diff --git a/.changeset/session-replay-gate-event-trigger-starts.md b/.changeset/session-replay-gate-event-trigger-starts.md new file mode 100644 index 000000000..e629e7766 --- /dev/null +++ b/.changeset/session-replay-gate-event-trigger-starts.md @@ -0,0 +1,6 @@ +--- +"posthog": patch +"posthog-android": patch +--- + +Fix: a session recording started by an event trigger now checks the same gates as every other start path. A matching event used to start recording even when `PostHogConfig.sessionReplay` was false, the project flag was off, or sampling excluded the session, so an app that gates replay behind its own feature flag recorded the users the flag excluded. A manual start can still wait for a matching event, and `PostHog.stopSessionReplay` cancels that pending request. diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index 1ebc1d138..52795fdf2 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -180,9 +180,9 @@ public class PostHogReplayIntegration( @Volatile private var isSessionReplayActive: Boolean = false - // Set by any start that happens while config.sessionReplay is false — the manual API or an - // event trigger. Survives stopRecording() so an internal stop (session cleared, sampled out, - // flag off) can still resume later; only an explicit stop() or uninstall() clears it. + // Set only by an explicit start request made while config.sessionReplay is false. It survives + // stopRecording() so an internal stop (session cleared, sampled out, flag off) can still resume + // later; only an explicit stop() or uninstall() clears it. @Volatile private var startedWithAutomaticDisabled: Boolean = false @@ -2252,7 +2252,12 @@ public class PostHogReplayIntegration( } override fun start(resumeCurrent: Boolean) { - // Check if we should wait for event triggers before starting + // Remember an explicit start asked for while automatic replay is off. The event gate can + // defer it, so the intent must be recorded before checking that gate. + if (!config.sessionReplay) { + startedWithAutomaticDisabled = true + } + if (shouldWaitForEventTriggers()) { val triggers = config.remoteConfigHolder?.getEventTriggers() config.logger.log( @@ -2261,10 +2266,13 @@ public class PostHogReplayIntegration( return } + startRecording(resumeCurrent) + } + + private fun startRecording(resumeCurrent: Boolean) { val currentSessionId = postHog?.getSessionId()?.toString() resetSessionStateIfNeeded(currentSessionId, force = !resumeCurrent) - startedWithAutomaticDisabled = !config.sessionReplay isSessionReplayActive = true if (!resumeCurrent) { @@ -2306,7 +2314,7 @@ public class PostHogReplayIntegration( /** * Called when an event is captured. Checks if the event matches any configured triggers - * and starts session recording if so. + * and starts session recording if so, provided the other gates permit the session. */ override fun onEvent( event: String, @@ -2334,9 +2342,17 @@ public class PostHogReplayIntegration( synchronized(eventTriggersLock) { triggerActivatedSessionId = currentSessionId } + // A matched trigger only lifts the event-trigger gate. The master switch, the project + // flag and the sampling decision still decide, as on every other automatic start path. + if (!isRecordingPermittedForCurrentSession()) { + config.logger.log( + "[Session Replay] Event trigger matched: $event, but recording is not permitted for session $currentSessionId.", + ) + return + } config.logger.log("[Session Replay] Event trigger matched: $event. Starting replay for session $currentSessionId.") - // Start the integration now that a trigger has matched - start(resumeCurrent = true) + // Do not call start(): only an explicit request may establish manual-start provenance. + startRecording(resumeCurrent = true) } } @@ -2397,7 +2413,8 @@ public class PostHogReplayIntegration( return@post } if (isSessionReplayActive) stopRecording() - start(resumeCurrent = false) + // Do not call start(): session rotation is an automatic transition. + startRecording(resumeCurrent = false) } } @@ -2651,15 +2668,16 @@ public class PostHogReplayIntegration( if (!isSessionReplayActive) { config.logger.log("[Session Replay] Remote config enabled recording. Resuming.") mainHandler.handler.post { - if (!isSessionReplayActive) { + // Re-check at the transition so a queued automatic resume cannot use stale gates. + if (!isSessionReplayActive && isRecordingPermittedForCurrentSession()) { // Force a fresh keyframe for the resumed segment. While stopped, per-view snapshot // state is frozen and can reference a full snapshot that was never delivered (e.g. a // first-config-off opening window that was dropped), so resuming against it would emit // orphaned incremental snapshots the player can't anchor. Clear the state and force a // redraw so the resumed segment starts with meta + full snapshot — without rotating the - // session or touching the cold-start buffering state (unlike start(resumeCurrent = false)). + // session or touching the cold-start buffering state (unlike startRecording(false)). clearSnapshotStates() - start(resumeCurrent = true) + startRecording(resumeCurrent = true) synchronized(decorViews) { decorViews.keys.forEach { it.postInvalidate() } } diff --git a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt index c7c7c0fa9..b4a36c25f 100644 --- a/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt +++ b/posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt @@ -188,12 +188,13 @@ internal class PostHogReplayIntegrationTest { flagActive: Boolean, samplingPasses: Boolean, sessionReplay: Boolean = true, + triggers: Set = emptySet(), ): PostHogAndroidConfig { val remoteConfig = mock { on { isSessionReplayFlagActive() } doReturn flagActive on { makeSamplingDecision(any()) } doReturn samplingPasses - on { getEventTriggers() } doReturn emptySet() + on { getEventTriggers() } doReturn triggers on { hasRemoteConfigFetched() } doReturn true } return PostHogAndroidConfig(API_KEY).apply { @@ -573,7 +574,7 @@ internal class PostHogReplayIntegrationTest { val config = PostHogAndroidConfig(API_KEY).apply { remoteConfigHolder = remoteConfig - sessionReplay = false + sessionReplay = true } val sut = getSut(config) val postHog = mock() @@ -585,8 +586,8 @@ internal class PostHogReplayIntegrationTest { shadowOf(Looper.getMainLooper()).idle() assertTrue(sut.isActive()) - // Rotating into a session the trigger has not matched must stop recording, and the - // preserved manual intent must not let remote config resume it behind the trigger gate. + // Rotating into a session the trigger has not matched must stop recording, and remote + // config must not resume it behind the trigger gate. PostHogSessionManager.endSession() PostHogSessionManager.startSession() sut.onSessionIdChanged() @@ -602,6 +603,197 @@ internal class PostHogReplayIntegrationTest { } } + @Test + fun `event trigger starts only when all replay gates pass`() { + data class GateCase( + val name: String, + val localEnabled: Boolean = true, + val flagActive: Boolean = true, + val samplingPasses: Boolean = true, + val triggerMatches: Boolean = true, + val expectedActive: Boolean = false, + ) + + val cases = + listOf( + GateCase("flag off, trigger not matched", flagActive = false, triggerMatches = false), + GateCase("flag off, trigger matched", flagActive = false), + GateCase("flag on, trigger not matched", triggerMatches = false), + GateCase("all gates pass", expectedActive = true), + GateCase("local switch off", localEnabled = false), + GateCase("sampled out", samplingPasses = false), + ) + + for (case in cases) { + val config = + configWithSampling( + flagActive = case.flagActive, + samplingPasses = case.samplingPasses, + sessionReplay = case.localEnabled, + triggers = setOf("checkout_started"), + ) + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + sut.onEvent(if (case.triggerMatches) "checkout_started" else "product_viewed", null) + shadowOf(Looper.getMainLooper()).idle() + + assertEquals(case.expectedActive, sut.isActive(), case.name) + } finally { + sut.uninstall() + PostHogSessionManager.endSession() + } + } + } + + @Test + fun `event trigger starts a manually requested recording while the master switch is off`() { + val config = + configWithSampling( + flagActive = true, + samplingPasses = true, + sessionReplay = false, + triggers = setOf("checkout_started"), + ) + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + // The trigger gate defers this start, but the intent must survive it. + sut.start(resumeCurrent = true) + shadowOf(Looper.getMainLooper()).idle() + assertFalse(sut.isActive()) + + sut.onEvent("checkout_started", null) + shadowOf(Looper.getMainLooper()).idle() + + assertTrue(sut.isActive()) + } finally { + sut.uninstall() + } + } + + @Test + fun `event trigger activation is retained when another gate initially rejects recording`() { + val triggers = setOf("checkout_started") + val localConfig = + configWithSampling( + flagActive = true, + samplingPasses = true, + sessionReplay = false, + triggers = triggers, + ) + val linkedFlag = AtomicBoolean(false) + val linkedFlagConfig = configWithSampling(flagActive = false, samplingPasses = true, triggers = triggers) + whenever(linkedFlagConfig.remoteConfigHolder!!.isSessionReplayFlagActive()).thenAnswer { linkedFlag.get() } + val sampling = AtomicBoolean(false) + val samplingConfig = configWithSampling(flagActive = true, samplingPasses = false, triggers = triggers) + whenever(samplingConfig.remoteConfigHolder!!.makeSamplingDecision(any())).thenAnswer { sampling.get() } + val cases = + listOf( + Triple("master switch", localConfig) { localConfig.sessionReplay = true }, + Triple("linked flag", linkedFlagConfig) { linkedFlag.set(true) }, + Triple("sampling", samplingConfig) { sampling.set(true) }, + ) + + for ((name, config, openGate) in cases) { + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + sut.onEvent("checkout_started", null) + assertFalse(sut.isActive(), "started before the $name gate opened") + + openGate() + sut.onRemoteConfig() + shadowOf(Looper.getMainLooper()).idle() + + assertTrue(sut.isActive(), "did not reuse trigger activation after the $name gate opened") + } finally { + sut.uninstall() + PostHogSessionManager.endSession() + } + } + } + + @Test + fun `explicit stop cancels a manual start waiting for an event trigger`() { + val config = + configWithSampling( + flagActive = true, + samplingPasses = true, + sessionReplay = false, + triggers = setOf("checkout_started"), + ) + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + try { + PostHogSessionManager.startSession() + sut.start(resumeCurrent = true) + assertFalse(sut.isActive()) + + sut.stop() + sut.onEvent("checkout_started", null) + shadowOf(Looper.getMainLooper()).idle() + + assertFalse(sut.isActive()) + } finally { + sut.uninstall() + } + } + + @Test + fun `concurrent local disable does not give an event trigger manual start provenance`() { + val samplingStarted = CountDownLatch(1) + val continueSampling = CountDownLatch(1) + val config = + configWithSampling( + flagActive = true, + samplingPasses = true, + triggers = setOf("checkout_started"), + ) + whenever(config.remoteConfigHolder!!.makeSamplingDecision(any())).thenAnswer { + samplingStarted.countDown() + assertTrue(continueSampling.await(2, TimeUnit.SECONDS)) + true + } + val sut = getSut(config) + val postHog = mock() + whenever(postHog.getSessionId()).thenAnswer { PostHogSessionManager.peekSessionId() } + sut.install(postHog) + val eventExecutor = Executors.newSingleThreadExecutor() + try { + PostHogSessionManager.startSession() + val event = eventExecutor.submit { sut.onEvent("checkout_started", null) } + assertTrue(samplingStarted.await(2, TimeUnit.SECONDS)) + + config.sessionReplay = false + continueSampling.countDown() + event.get(2, TimeUnit.SECONDS) + assertTrue(sut.isActive()) + + // The racing automatic start may win, but it must remain automatic so the next + // reevaluation can stop it instead of preserving it as a manual recording. + sut.onRemoteConfig() + shadowOf(Looper.getMainLooper()).idle() + + assertFalse(sut.isActive()) + } finally { + continueSampling.countDown() + eventExecutor.shutdownNow() + sut.uninstall() + } + } + @Test fun `clears buffer when PostHogReplayIntegration is installed`() { val config = PostHogAndroidConfig(API_KEY) diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index 7b5ecd004..d325ec780 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -2209,14 +2209,7 @@ public class PostHog private constructor( return } - sessionReplayHandler?.let { - // already inactive - if (!it.isActive()) { - return - } - - it.stop() - } ?: run { + sessionReplayHandler?.stop() ?: run { config?.logger?.log("Session replay isn't installed.") } } diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 6376d21bf..5f8ad4f8e 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -2756,6 +2756,17 @@ internal class PostHogTest { assertTrue(integration.stopCalled) } + @Test + fun `stopSessionReplay forwards stop when replay is inactive`() { + val http = mockHttp() + val integration = PostHogSessionReplayHandlerFake(false) + val sut = getSut(http.url("/").toString(), preloadFeatureFlags = false, integration = integration) + + sut.stopSessionReplay() + + assertTrue(integration.stopCalled) + } + @Test @Suppress("DEPRECATION") fun `captureException captures exception with correct properties`() {