Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions .changeset/failed-flag-records.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"posthog": patch
"posthog-android": patch
"posthog-android-surveys-compose": patch
"posthog-server": patch
"posthog-server-openfeature": patch
---

Treat failed detailed feature flag records as unavailable values and payloads, independently of response-wide errors. Preserve successful cached values on errored client refreshes and same-context local decisions during server fallback. Report failed direct reads as evaluation failures without tracking caller defaults as evaluated responses.
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
package com.posthog.android.internal

import android.content.Context
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.posthog.android.FakeSharedPreferences
import com.posthog.android.PROJECT_TOKEN
import com.posthog.android.PostHogAndroidConfig
import com.posthog.internal.FeatureFlag
import com.posthog.internal.FeatureFlagMetadata
import com.posthog.internal.PostHogApi
import com.posthog.internal.PostHogPreferences.Companion.GROUPS
import com.posthog.internal.PostHogPreferences.Companion.STRINGIFIED_KEYS
import com.posthog.internal.PostHogRemoteConfig
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
import org.junit.runner.RunWith
import org.mockito.kotlin.any
import org.mockito.kotlin.doAnswer
Expand All @@ -26,6 +33,96 @@ internal class PostHogSharedPreferencesTests {
return PostHogSharedPreferences(context, config, sharedPreferences = FakeSharedPreferences())
}

@Test
fun `cached startup excludes pre fix failed flag placeholders after shared preferences roundtrip`() {
val appContext = ApplicationProvider.getApplicationContext<Context>()
val config = PostHogAndroidConfig("failed-cache-test")
val preferences = PostHogSharedPreferences(appContext, config)
preferences.clear()
preferences.setValue(
"flags",
mapOf(
"checkout" to FeatureFlag("checkout", false, "placeholder", FeatureFlagMetadata(1, "\"remote\"", 1), null, true),
),
)
preferences.setValue("featureFlags", mapOf("checkout" to "placeholder", "disabled" to false, "legacy" to true))
preferences.setValue("featureFlagsPayload", mapOf("checkout" to "remote", "legacy" to "valid"))
preferences.setValue("sessionReplay", mapOf("linkedFlag" to "checkout"))
val restored = PostHogSharedPreferences(appContext, config)
config.cachePreferences = restored
val executor = java.util.concurrent.Executors.newSingleThreadExecutor()
val flags = PostHogRemoteConfig(config, PostHogApi(config), executor)
try {
assertFalse(flags.isSessionReplayFlagActive())
assertNull(flags.getFeatureFlagResult("checkout"))
assertEquals(true, flags.getFeatureFlag("checkout", defaultValue = true))
assertEquals(setOf("disabled", "legacy"), flags.getFeatureFlags()?.keys)
assertEquals(false, flags.getFeatureFlagResult("disabled")?.enabled)
assertEquals("valid", flags.getFeatureFlagResult("legacy")?.payload)
assertEquals(true, flags.getFlagDetails("checkout")?.failed)
} finally {
flags.clear()
restored.clear()
executor.shutdown()
assertTrue(executor.awaitTermination(5, java.util.concurrent.TimeUnit.SECONDS))
}
}

@Test
fun `successful legacy recovery replaces failed detail provenance across shared preferences reopen`() {
val appContext = ApplicationProvider.getApplicationContext<Context>()
val http = MockWebServer()
http.enqueue(
MockResponse().setBody(
"""{"flags":{
"checkout":{"key":"checkout","enabled":false,"failed":true,"metadata":{"id":1,"version":1}},
"unresolved":{"key":"unresolved","enabled":false,"failed":true,"metadata":{"id":2,"version":1}},
"sibling":{"key":"sibling","enabled":false,"metadata":{"id":3,"version":2}}
}}""",
),
)
http.enqueue(
MockResponse().setBody(
"""{"errorsWhileComputingFlags":true,"featureFlags":{"checkout":"recovered"},
"featureFlagPayloads":{"checkout":"\"good\""}}""",
),
)
http.start()
val config = PostHogAndroidConfig("legacy-recovery-test", http.url("/").toString())
val original = PostHogSharedPreferences(appContext, config)
original.clear()
config.cachePreferences = original
val executor = java.util.concurrent.Executors.newSingleThreadExecutor()
val flags = PostHogRemoteConfig(config, PostHogApi(config), executor)
try {
repeat(2) {
val loaded = java.util.concurrent.CountDownLatch(1)
flags.loadFeatureFlags("user", null, null, onFeatureFlags = { loaded.countDown() })
assertTrue(loaded.await(5, java.util.concurrent.TimeUnit.SECONDS))
}
assertEquals("recovered", flags.getFeatureFlagResult("checkout")?.value)
assertEquals("good", flags.getFeatureFlagResult("checkout")?.payload)
config.cachePreferences = PostHogSharedPreferences(appContext, config)
val reopened = PostHogRemoteConfig(config, PostHogApi(config), executor)
assertEquals("recovered", reopened.getFeatureFlagResult("checkout")?.value)
assertEquals("good", reopened.getFeatureFlagResult("checkout")?.payload)
for (sut in listOf(flags, reopened)) {
assertNull(sut.getFlagDetails("checkout"), "Legacy success must not inherit earlier failure attribution")
assertEquals(3, sut.getFlagDetails("sibling")?.metadata?.id)
assertEquals(false, sut.getFeatureFlagResult("sibling")?.value)
assertNull(sut.getFeatureFlagResult("unresolved"))
assertEquals(true, sut.getFlagDetails("unresolved")?.failed)
}
assertEquals(2, http.requestCount, "Reopening must not fetch flags")
} finally {
executor.shutdown()
assertTrue(executor.awaitTermination(5, java.util.concurrent.TimeUnit.SECONDS))
flags.clear()
original.clear()
http.shutdown()
}
}

@Test
fun `preferences set string`() {
val sut = getSut()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package com.posthog.server.internal
internal object FeatureFlagError {
const val ERRORS_WHILE_COMPUTING: String = "errors_while_computing_flags"
const val FLAG_MISSING: String = "flag_missing"
const val EVALUATION_FAILED: String = "evaluation_failed"
const val LOCAL_EVALUATION_INCONCLUSIVE: String = "local_evaluation_inconclusive"
const val QUOTA_LIMITED: String = "quota_limited"
const val TIMEOUT: String = "timeout"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ internal class PostHogFeatureFlags(
groupProperties,
) ?: return null

if (flag.failed == true) return null
return FeatureFlagResult(key, flag.enabled, flag.variant, flag.metadata.payload)
}

Expand Down Expand Up @@ -369,28 +370,6 @@ internal class PostHogFeatureFlags(
return LocalEvaluationOutcome(localFlags, needsRemote, missingDefinitionKeys, unresolved)
}

/**
* All-or-nothing local evaluation for the deprecated [getFeatureFlags] path: a single
* unresolved flag discards the batch so the caller falls back to `/flags`.
*/
private fun getFeatureFlagsFromLocalEvaluation(
distinctId: String,
groups: Map<String, String>?,
personProperties: Map<String, Any?>?,
groupProperties: Map<String, Map<String, Any?>>?,
): Map<String, FeatureFlag>? {
val outcome =
evaluateFlagsLocally(
distinctId,
groups,
personProperties,
groupProperties,
flagKeys = null,
) ?: return null

return if (outcome.needsRemote) null else outcome.flags
}

private fun localPersonProperties(
distinctId: String,
personProperties: Map<String, Any?>?,
Expand Down Expand Up @@ -502,23 +481,29 @@ internal class PostHogFeatureFlags(

val cached = getFeatureFlagsFromCache(distinctId, groups, personProperties, groupProperties)
if (cached != null) {
return cached
val failedKeys = cached.filterValues { it.failed == true }.keys
if (failedKeys.isEmpty()) return cached
// Cache hits may use loaded definitions, but leave missing-definition retries to refresh.
val local =
if (flagDefinitions != null) {
evaluateFlagsLocally(distinctId, groups, personProperties, groupProperties, flagKeys = failedKeys.toList())
} else {
null
}
return local?.flags.orEmpty() + cached.filterKeys { it !in failedKeys }
}

// If no cached flags, try local evaluation
val localFlags =
getFeatureFlagsFromLocalEvaluation(
distinctId,
groups,
personProperties,
groupProperties,
)
if (localFlags != null) {
return localFlags
val local = evaluateFlagsLocally(distinctId, groups, personProperties, groupProperties, flagKeys = null)
if (local != null && !local.needsRemote) {
return local.flags
}

// Finally, fall back to remote fetch
return getFeatureFlagsFromRemote(distinctId, groups, personProperties, groupProperties).flags
val remoteFlags = getFeatureFlagsFromRemote(distinctId, groups, personProperties, groupProperties).flags
val successfulRemoteFlags = remoteFlags?.filterValues { it.failed != true }
if (local == null || remoteFlags == null) return successfulRemoteFlags
// Preserve local pairs only for explicitly failed records; other remote precedence is unchanged.
val retainedLocalFlags = local.flags.filterKeys { remoteFlags[it]?.failed == true }
return retainedLocalFlags + successfulRemoteFlags.orEmpty()
}

override fun clear() {
Expand Down Expand Up @@ -1180,7 +1165,7 @@ internal class PostHogFeatureFlags(
if (onlyEvaluateLocally) {
return EMPTY_EVALUATE_FLAGS_RESULT
}
val flags = entry.flags ?: EMPTY_FLAGS
val flags = entry.flags?.filterValues { it.failed != true } ?: EMPTY_FLAGS
return EvaluateFlagsResult(
flags = flags,
locallyEvaluated = flags.mapValues { isLocallyEvaluated(it.value) },
Expand Down Expand Up @@ -1266,7 +1251,8 @@ internal class PostHogFeatureFlags(
// Same precedence as posthog-python, which skips remote keys already in
// `locally_evaluated_keys`. Note a group flag evaluated without `groups` resolves locally to
// `false`, and that now beats the server's answer — pass `groups` when gating on one.
val merged = LinkedHashMap(entry?.flags ?: EMPTY_FLAGS).apply { putAll(localFlags) }
val successfulRemoteFlags = entry?.flags?.filterValues { it.failed != true } ?: EMPTY_FLAGS
val merged = LinkedHashMap(successfulRemoteFlags).apply { putAll(localFlags) }
applyDefinitionRuntimes(merged)
return EvaluateFlagsResult(
flags = merged,
Expand Down Expand Up @@ -1479,15 +1465,13 @@ internal class PostHogFeatureFlags(
return entry.error ?: FeatureFlagError.UNKNOWN_ERROR
}

// Request succeeded - check for flag_missing (key-specific, computed at query time)
val flagMissing = !entry.flags.containsKey(key)

return when {
entry.error == null && !flagMissing -> null
entry.error == null -> FeatureFlagError.FLAG_MISSING
!flagMissing -> entry.error
else -> "${entry.error},${FeatureFlagError.FLAG_MISSING}"
}
val flagError =
when {
entry.flags[key]?.failed == true -> FeatureFlagError.EVALUATION_FAILED
!entry.flags.containsKey(key) -> FeatureFlagError.FLAG_MISSING
else -> null
}
return listOfNotNull(entry.error, flagError).takeIf { it.isNotEmpty() }?.joinToString(",")
}

internal companion object {
Expand Down
Loading
Loading