fix: keep the personless option over a legacy property and drop a 2xx without results

.changeset/android-v4-major.md

@@ -24,7 +24,7 @@
 - **Breaking:** add an `options` argument to `capture`, `screen`, `identify`, `setPersonProperties`, `alias`, `group` and `captureException` for per-event capture options such as `cookieless_mode`, sent apart from the properties and editable in `beforeSend` through `PostHogEvent.options`; Java callers pass the trailing argument or `null`, and classes that implement `PostHogInterface` add the parameter
 - **Breaking:** move `identify(distinctId, userProperties, userPropertiesSetOnce)` from `PostHogCoreInterface` to `PostHogStatelessInterface`; `PostHogInterface` declares `identify` with the trailing `options` parameter
 - **Breaking:** registered super properties now override the properties the SDK sets itself, such as `$os_name`; super and event properties also override `$is_identified`, `$process_person_profile` and the session replay debug properties, which the SDK no longer re-applies after the caller's values
-- **Breaking:** the SDK sets the `process_person_profile` option to `false` only when person processing is off, and no longer adds a `$process_person_profile` property to every event
+- **Breaking:** the SDK sets the `process_person_profile` option to `false` only when person processing is off, and no longer adds a `$process_person_profile` property to every event; a passed or registered `$process_person_profile` property no longer overrides that `false`, so pass the `process_person_profile` option instead
 - **Breaking:** keep the uuid of its own capture on the event `beforeSend` returns, so a uuid set in `beforeSend`, for example with `copy(uuid = …)`, is replaced
 - **Breaking:** add `captureAi` to send AI events such as `$ai_generation` to PostHog's AI endpoint (`/i/v1/ai/events`), which accepts events whose properties are up to 8 MiB, on its own disk queue; it returns the event uuid, or `null` when the event isn't queued, and classes that implement `PostHogInterface` add the method
 - use the current distinct ID when the `distinctId` argument to `capture` is empty or whitespace-only, instead of dropping the event

docs/migration-3.x-to-4.0.md

@@ -54,6 +54,7 @@ If you send events through your own reverse proxy, update it before you upgrade:
 - Forward `/i/v1/analytics/events` and `/i/v1/ai/events` to PostHog.
 - Pass `Authorization` (the project token, sent as `Bearer <token>`), every `PostHog-*` header, `Content-Type`, `Content-Encoding` and `User-Agent` through unchanged. PostHog answers `401` without `Authorization` and `400` without the others, and the events are dropped.
 - Accept request bodies up to 20 MB. AI requests are often larger than 1 MB, the default limit in nginx and ingress-nginx. A `413` from the proxy makes the SDK halve the batch until it fits, and drops an event that doesn't fit on its own.
+- Pass PostHog's response body through unchanged. A 2xx response whose body isn't PostHog's `{"results": {...}}` drops the events instead of retrying them.
 
 PostHog's managed reverse proxy already does this. See the [proxy guides](https://posthog.com/docs/advanced/proxy).
 
@@ -149,7 +150,7 @@ When the event is sent, the SDK removes each one from `properties` and uses it f
 
 - An option you pass to any of these methods or set in `beforeSend` wins over its legacy property. An option set to `null` counts as unset, so the legacy property fills it.
 - To change person processing in `beforeSend`, edit `event.options`. A `$process_person_profile` property the hook adds only fills an option that is still unset, so it doesn't override the `false` the SDK set for an anonymous event.
-- When `personProfiles` turns person processing off, the SDK sets the `process_person_profile` option to `false`. Otherwise it sends neither, because PostHog processes person profiles by default. In 3.x, every event carried a `$process_person_profile` property. To change person processing for one event, pass the `process_person_profile` option or the `$process_person_profile` property. A registered `$process_person_profile` changes it for every event.
+- When `personProfiles` turns person processing off, the SDK sets the `process_person_profile` option to `false`. Otherwise it sends neither, because PostHog processes person profiles by default. In 3.x, every event carried a `$process_person_profile` property. To change person processing for one event, pass the `process_person_profile` option. A `$process_person_profile` property, passed or registered, only fills that option when it's unset, so it can't override the `false` the SDK sets when person processing is off.
 - A `$set` sent with `process_person_profile: false` doesn't update the person, so it no longer suppresses a later `setPersonProperties` or `identify` call with the same properties as a duplicate.
 - The SDK sends option values as given. PostHog reads common forms of a value, such as `"false"` or `0` for a boolean, and drops an event whose option value it can't read.
 - Other option keys are sent unchanged, and PostHog ignores keys it doesn't know.

posthog/src/main/java/com/posthog/PostHog.kt

@@ -809,7 +809,8 @@ public class PostHog private constructor(
             props["\$is_identified"] = isIdentified
             // No `$process_person_profile`: person processing is sent as the `process_person_profile`
             // option, only when it's off (see personProcessingOptions). A registered or caller
-            // `$process_person_profile` wins over it when the send hoists the property.
+            // `$process_person_profile` only fills that option when the send hoists the property and
+            // the option is still unset.
         }
 
         // Session replay should have the SDK info as well
@@ -977,17 +978,14 @@ public class PostHog private constructor(
 
     /**
      * Adds `process_person_profile: false` when the SDK doesn't process person profiles for this
-     * event, unless the caller set that option or [properties] carry a non-null legacy
-     * `$process_person_profile` property (per call or registered). Nothing is added otherwise:
-     * PostHog processes person profiles by default. Returns a copy, so `beforeSend` edits never
-     * reach the caller's map.
+     * event, unless the caller set that option. A legacy `$process_person_profile` property doesn't
+     * prevent it: the send hoists that property only into an unset option. Nothing is added
+     * otherwise: PostHog processes person profiles by default. Returns a copy, so `beforeSend`
+     * edits never reach the caller's map.
      */
-    private fun personProcessingOptions(
-        options: Map<String, Any?>?,
-        properties: Map<String, Any>,
-    ): MutableMap<String, Any?>? {
+    private fun personProcessingOptions(options: Map<String, Any?>?): MutableMap<String, Any?>? {
         val eventOptions = options?.toMutableMap()
-        if (hasPersonProcessing() || properties["\$process_person_profile"] != null) {
+        if (hasPersonProcessing()) {
             return eventOptions
         }
         val result = eventOptions ?: mutableMapOf()
@@ -1186,7 +1184,7 @@ public class PostHog private constructor(
 
             // Snapshots don't carry the SDK's person-processing option.
             val eventOptions =
-                if (isSnapshotEvent) options?.toMutableMap() else personProcessingOptions(options, mergedProperties)
+                if (isSnapshotEvent) options?.toMutableMap() else personProcessingOptions(options)
 
             // This capture's identity, kept through beforeSend.
             val uuid = TimeBasedEpochGenerator.generate()

posthog/src/main/java/com/posthog/internal/PostHogApi.kt

@@ -174,15 +174,17 @@ public class PostHogApi(
      * Sends analytics events to capture v1 (`POST /i/v1/analytics/events`).
      *
      * Returns the uuids of the events the 2xx response asked to retry, with the `Retry-After` it
-     * sent; every other event in [events] was delivered or dropped and must not be sent again. A 2xx
-     * whose per-event results can't be read, including a transfer that failed after the status
-     * arrived, counts as delivered: the server already stored its events and a resend would store
-     * them again.
+     * sent; every other event in [events] was delivered or dropped and must not be sent again.
+     *
+     * A 2xx whose body can't be read as `{"results": {...}}`, including a transfer that failed after
+     * the status arrived, is a terminal failure of the whole batch: it throws a [PostHogApiError]
+     * with that 2xx status, which the queue doesn't retry.
      *
      * Only a 307 or 308 to the origin of [PostHogConfig.host] is followed, at most 5 times, with
      * the same body and headers; any other redirect is a terminal [PostHogApiError].
      *
-     * Throws [PostHogApiError] for a non-2xx status and [IOException] for a transport failure.
+     * Throws [PostHogApiError] for a non-2xx status or unreadable results, and [IOException] for a
+     * transport failure before the status.
      */
     @Throws(PostHogApiError::class, IOException::class)
     internal fun captureV1(events: List<PostHogEvent>): SendResult = postCaptureV1(CAPTURE_V1_ANALYTICS_PATH, events, captureClient)
@@ -302,7 +304,8 @@ public class PostHogApi(
 
     /**
      * Reads the per-event results of a 2xx and records which events, if any, the next request
-     * retries under the same identity. A non-2xx is thrown; a terminal one also drops the identity.
+     * retries under the same identity. A non-2xx is thrown; a terminal one also drops the identity,
+     * as does a 2xx without readable results, which is thrown as a terminal failure.
      */
     @Throws(PostHogApiError::class)
     private fun captureV1Result(
@@ -318,11 +321,13 @@ public class PostHogApi(
             throw PostHogApiError(response.code, response.message, response.body, parseRetryAfter(response))
         }
 
+        // Counts only: the body may name event uuids.
         val results = readCaptureV1Results(response)
         if (results == null) {
-            config.logger.log(
-                "Capture returned ${response.code} without readable per-event results, treating ${events.size} events as delivered.",
-            )
+            clearCaptureV1Retry(path, identity)
+            val message = "Capture returned ${response.code} without readable per-event results, dropping ${events.size} events."
+            config.logger.log(message)
+            throw PostHogApiError(response.code, message, null)
         }
 
         val retryUuids = mutableSetOf<String>()
@@ -331,7 +336,7 @@ public class PostHogApi(
         // Events missing from the results count as delivered. The uuids are read once, so a repeated
         // uuid's later copies (`duplicates_dropped`) are counted, not given the kept copy's verdict.
         for (uuid in events.mapNotNullTo(LinkedHashSet()) { it.uuid?.toString() }) {
-            val entry = results?.get(uuid) as? Map<*, *> ?: continue
+            val entry = results[uuid] as? Map<*, *> ?: continue
             when (entry["result"]) {
                 "retry" -> retryUuids.add(uuid)
                 "drop" -> dropped++

posthog/src/test/java/com/posthog/PostHogCaptureOptionsTest.kt

@@ -218,15 +218,17 @@ internal class PostHogCaptureOptionsTest {
     }
 
     @Test
-    fun `a registered legacy person processing property suppresses the personless option`() {
-        val sut = getSut(personProfiles = PersonProfiles.NEVER)
+    fun `a registered legacy person processing property doesn't override the personless option`() {
+        val http = httpServers.mockHttp()
+        val sut = getSut(http.url("/").toString(), personProfiles = PersonProfiles.NEVER, beforeSend = { it })
         sut.register("\$process_person_profile", true)
 
         sut.capture("test_event")
+        queueExecutor.shutdownAndAwaitTermination()
 
-        val event = captured.single()
-        assertNull(event.options)
-        assertEquals(true, event.properties!!["\$process_person_profile"])
+        val event = sentEvents(http).single()
+        assertEquals<Map<String, Any?>?>(mapOf("process_person_profile" to false), event.options)
+        assertFalse(event.properties!!.containsKey("\$process_person_profile"))
         sut.close()
     }
 

posthog/src/test/java/com/posthog/PostHogPersonProfilesTest.kt

@@ -119,7 +119,7 @@ internal class PostHogPersonProfilesTest {
     }
 
     @Test
-    fun `caller person processing properties win if identified only and not identified`() {
+    fun `caller person processing properties don't override the personless option if identified only and not identified`() {
         val http = mockHttp()
         val url = http.url("/")
 
@@ -138,8 +138,8 @@ internal class PostHogPersonProfilesTest {
 
         val event = batch.batch.first()
 
-        // the caller's legacy property is hoisted into the option when the event is sent
-        assertEquals(true, event.processPersonProfile)
+        // the SDK's personless option wins; the caller's legacy property is removed when sent
+        assertEquals(false, event.processPersonProfile)
         assertFalse(event.properties!!.containsKey("\$process_person_profile"))
         assertEquals(true, event.properties!!["\$is_identified"] as Boolean)
 
@@ -424,7 +424,7 @@ internal class PostHogPersonProfilesTest {
     }
 
     @Test
-    fun `caller process person override wins if never`() {
+    fun `caller process person property doesn't override never`() {
         val http = mockHttp()
         val url = http.url("/")
 
@@ -440,7 +440,7 @@ internal class PostHogPersonProfilesTest {
 
         val event = batch.batch.first()
 
-        assertEquals(true, event.processPersonProfile)
+        assertEquals(false, event.processPersonProfile)
         assertFalse(event.properties!!.containsKey("\$process_person_profile"))
 
         sut.close()