diff --git a/.changeset/before-send-once.md b/.changeset/before-send-once.md new file mode 100644 index 000000000..a64308f68 --- /dev/null +++ b/.changeset/before-send-once.md @@ -0,0 +1,6 @@ +--- +'posthog': patch +'posthog-android': patch +--- + +Fix `beforeSend` running twice for each captured event, which discarded the hook's changes to `distinctId`, `timestamp` and `uuid` and compounded random sampling diff --git a/posthog/src/main/java/com/posthog/PostHog.kt b/posthog/src/main/java/com/posthog/PostHog.kt index 20745f58c..281503ecb 100644 --- a/posthog/src/main/java/com/posthog/PostHog.kt +++ b/posthog/src/main/java/com/posthog/PostHog.kt @@ -1037,7 +1037,9 @@ public class PostHog private constructor( }, ) - val postHogEvent = buildEvent(event, newDistinctId, mergedProperties, timestamp) + val eventProperties = minimizeIfFeatureFlagCalled(event, mergedProperties) + + val postHogEvent = buildEvent(event, newDistinctId, eventProperties, timestamp) if (postHogEvent == null) { val originalMessage = "PostHog event $event was dropped" val message = @@ -1060,18 +1062,15 @@ public class PostHog private constructor( if (isSnapshotEvent) { replayQueue?.add(postHogEvent) } else { - val queued = - captureStatelessInternal( - postHogEvent.event, - newDistinctId, - postHogEvent.properties ?: emptyMap(), - userProperties, - userPropertiesSetOnce, - groups, - timestamp, - ) - if (queued && claimedReplayDebugBundle) { - commitReplayDebugPropertiesWindow() + val queue = queue + // beforeSend may have opted out + if (isOptedOut()) { + config?.logger?.log("PostHog is in OptOut state.") + } else if (queue != null) { + queue.add(postHogEvent) + if (claimedReplayDebugBundle) { + commitReplayDebugPropertiesWindow() + } } // Notify surveys integration about the event surveysHandler?.onEvent(event, mergedProperties) diff --git a/posthog/src/main/java/com/posthog/PostHogStateless.kt b/posthog/src/main/java/com/posthog/PostHogStateless.kt index dd858b6f8..bc54145ad 100644 --- a/posthog/src/main/java/com/posthog/PostHogStateless.kt +++ b/posthog/src/main/java/com/posthog/PostHogStateless.kt @@ -241,26 +241,14 @@ public open class PostHogStateless protected constructor( groups: Map?, timestamp: Date?, ) { - captureStatelessInternal(event, distinctId, properties, userProperties, userPropertiesSetOnce, groups, timestamp) - } - - internal fun captureStatelessInternal( - event: String, - distinctId: String, - properties: Map?, - userProperties: Map?, - userPropertiesSetOnce: Map?, - groups: Map?, - timestamp: Date?, - ): Boolean { try { if (!isEnabled()) { - return false + return } if (config?.optOut == true) { config?.logger?.log("PostHog is in OptOut state.") - return false + return } var groupIdentify = false @@ -277,18 +265,13 @@ public open class PostHogStateless protected constructor( appendGroups = !groupIdentify, ) - val eventProperties = - if (event == PostHogEventName.FEATURE_FLAG_CALLED.event) { - minimizeFeatureFlagCalledProperties(mergedProperties) - } else { - mergedProperties - } + val eventProperties = minimizeIfFeatureFlagCalled(event, mergedProperties.toMutableMap()) val postHogEvent = buildEvent( event, distinctId, - eventProperties.toMutableMap(), + eventProperties, timestamp, ) if (postHogEvent == null) { @@ -300,15 +283,12 @@ public open class PostHogStateless protected constructor( originalMessage } config?.logger?.log(message) - return false + return } - val queue = queue ?: return false - queue.add(postHogEvent) - return true + queue?.add(postHogEvent) } catch (e: Throwable) { config?.logger?.log("Capture failed: $e.") - return false } } @@ -551,9 +531,15 @@ public open class PostHogStateless protected constructor( * enabled minimal flag-called events for this team AND the evaluated flag is not linked to an * experiment. Any missing signal (gate absent from the response or cache, `has_experiment` * unknown) keeps the full legacy shape, and experiment-linked flags always send the full - * envelope because experiment exposure analysis reads it. + * envelope because experiment exposure analysis reads it. Other events pass through unchanged. */ - private fun minimizeFeatureFlagCalledProperties(properties: Map): Map { + internal fun minimizeIfFeatureFlagCalled( + event: String, + properties: MutableMap, + ): MutableMap { + if (event != PostHogEventName.FEATURE_FLAG_CALLED.event) { + return properties + } if (!isMinimalFlagCalledEventsEnabled()) { return properties } @@ -561,7 +547,7 @@ public open class PostHogStateless protected constructor( return properties } - return properties.filterKeys { it in MINIMAL_FEATURE_FLAG_CALLED_PROPERTIES } + return properties.filterKeys { it in MINIMAL_FEATURE_FLAG_CALLED_PROPERTIES }.toMutableMap() } /** diff --git a/posthog/src/test/java/com/posthog/PostHogTest.kt b/posthog/src/test/java/com/posthog/PostHogTest.kt index 0846b04bc..01286caba 100644 --- a/posthog/src/test/java/com/posthog/PostHogTest.kt +++ b/posthog/src/test/java/com/posthog/PostHogTest.kt @@ -2517,6 +2517,100 @@ internal class PostHogTest { http.shutdown() } + @Test + fun `beforeSend runs once per capture`() { + val http = mockHttp() + val url = http.url("/") + var calls = 0 + + val sut = + getSut(url.toString(), preloadFeatureFlags = false, beforeSend = { + calls++ + it + }) + + sut.capture("event") + + queueExecutor.shutdownAndAwaitTermination() + + assertEquals(1, calls) + + sut.close() + http.shutdown() + } + + @Test + fun `an event edited by beforeSend is sent as edited`() { + val http = mockHttp() + val url = http.url("/") + var calls = 0 + + val sut = + getSut(url.toString(), preloadFeatureFlags = false, beforeSend = { + it.copy(distinctId = "hooked-${++calls}") + }) + + sut.capture("event") + + queueExecutor.shutdownAndAwaitTermination() + + val batch = serializer.deserialize(http.takeRequest().body.unGzip().reader()) + assertEquals("hooked-1", batch.batch.first().distinctId) + + sut.close() + http.shutdown() + } + + @Test + fun `an event is dropped when beforeSend opts out`() { + val http = mockHttp() + val url = http.url("/") + lateinit var sut: PostHogInterface + + sut = + getSut(url.toString(), preloadFeatureFlags = false, beforeSend = { + sut.optOut() + it + }) + + sut.capture("event") + + queueExecutor.shutdownAndAwaitTermination() + + assertEquals(0, http.requestCount) + + sut.close() + http.shutdown() + } + + @Test + fun `beforeSend sees the minimal feature flag called event`() { + val file = File("src/test/resources/json/basic-flags-minimal-flag-called-events.json") + val http = mockHttp(response = MockResponse().setBody(file.readText())) + http.enqueue(MockResponse().setBody("")) + val url = http.url("/") + val seenProperties = mutableListOf>() + + val sut = + getSut(url.toString(), preloadFeatureFlags = false, beforeSend = { + if (it.event == "\$feature_flag_called") seenProperties.add(it.properties.orEmpty().keys.toSet()) + it + }) + sut.register("myCustomProp", "value") + + sut.reloadFeatureFlags() + remoteConfigExecutor.shutdownAndAwaitTermination() + + sut.getFeatureFlag("IAmInactive") + queueExecutor.shutdownAndAwaitTermination() + + assertEquals(1, seenProperties.size) + assertFalse(seenProperties.first().contains("myCustomProp")) + + sut.close() + http.shutdown() + } + @Test fun `creates a group`() { val http = mockHttp() @@ -3683,10 +3777,8 @@ internal class PostHogTest { fun `an SDK event captured from inside beforeSend does not also carry the bundle`() { val http = mockHttp() lateinit var sut: PostHogInterface - // beforeSend runs twice per stateful capture (PostHog.capture, then captureStateless), so the - // nested capture fires twice. sut = - replayDebugSut(http, flushAt = 3, beforeSend = { + replayDebugSut(http, flushAt = 2, beforeSend = { if (it.event == "\$outer") sut.capture("\$inner") it })