From d8d13e930983edee42efbe37f7ee1133e9e42e66 Mon Sep 17 00:00:00 2001 From: "Gustavo H. Strassburger" Date: Tue, 27 Oct 2026 14:07:34 -0300 Subject: [PATCH 1/2] feat(server): expose flags that local evaluation could not resolve Implements the "Snapshot unresolved flag reporting" and "Warning for flags local evaluation never resolves" requirements of the evaluate-flags spec (PostHog/sdk-specs#85). - Add `unresolvedFlags` to the `evaluateFlags()` snapshot, with a `PostHogUnresolvedFlagReason` for each flag and the spec's fixed precedence. - Report `local_evaluation_inconclusive` instead of `flag_missing` when a snapshot read hits an unresolved flag. - Do not stop a condition at the first inconclusive filter, so a filter that does not match resolves the condition locally. - Log one warning per flag that local evaluation can never resolve, repeated only when its definition changes. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 39793b31-809e-4689-a5d8-b4359cbb22c1 --- .changeset/unresolved-local-flags.md | 11 + posthog-server/api/posthog-server.api | 12 + .../main/java/com/posthog/server/PostHog.kt | 1 + .../server/PostHogFeatureFlagEvaluations.kt | 29 +- .../server/PostHogUnresolvedFlagReason.kt | 32 + .../server/internal/EvaluateFlagsResult.kt | 3 + .../server/internal/FeatureFlagError.kt | 2 + .../posthog/server/internal/FlagEvaluator.kt | 573 ++++++++++++++---- .../internal/InconclusiveMatchException.kt | 28 +- .../server/internal/PostHogFeatureFlags.kt | 104 +++- .../server/PostHogEvaluateFlagsTest.kt | 114 ++++ .../PostHogFeatureFlagEvaluationsTest.kt | 38 ++ .../src/test/java/com/posthog/server/Utils.kt | 9 + .../server/internal/FlagEvaluatorTest.kt | 347 +++++++++++ .../internal/PostHogFeatureFlagsTest.kt | 422 +++++++++++++ 15 files changed, 1591 insertions(+), 134 deletions(-) create mode 100644 .changeset/unresolved-local-flags.md create mode 100644 posthog-server/src/main/java/com/posthog/server/PostHogUnresolvedFlagReason.kt diff --git a/.changeset/unresolved-local-flags.md b/.changeset/unresolved-local-flags.md new file mode 100644 index 000000000..33c19139d --- /dev/null +++ b/.changeset/unresolved-local-flags.md @@ -0,0 +1,11 @@ +--- +"posthog-server": minor +--- + +Add `unresolvedFlags` to the `evaluateFlags()` snapshot. It lists the requested flags that have a local definition but no value, because local evaluation could not resolve them and no `/flags` fallback filled them, for example with `onlyEvaluateLocally` or when the fallback fails. Each entry has a `PostHogUnresolvedFlagReason`: `EXPERIENCE_CONTINUITY`, `UNSUPPORTED_DEFINITION`, `UNRESOLVED_DEPENDENCY` or `MISSING_CONTEXT`. When several causes apply, the first reason in that order is reported. Snapshots from `only()` and `onlyAccessed()` have no unresolved flags. + +Behavior changes: + +- A condition with a filter that local evaluation cannot check and a filter that does not match now resolves locally as no match. Before, the flag fell back to a `/flags` request, or stayed absent with `onlyEvaluateLocally`. +- `$feature_flag_called` events from snapshot reads of an unresolved flag report `$feature_flag_error: local_evaluation_inconclusive` instead of `flag_missing`, so insights that filter on `flag_missing` count fewer events. +- When definitions load, the SDK logs a warning, also with `debug` off, that names the flags local evaluation can never resolve: flags with experience continuity, and flags that use a static cohort, an unknown operator, a malformed value, or a missing or circular dependency. Each flag is warned about once until its definition changes. diff --git a/posthog-server/api/posthog-server.api b/posthog-server/api/posthog-server.api index 51421e12d..de2f53e1a 100644 --- a/posthog-server/api/posthog-server.api +++ b/posthog-server/api/posthog-server.api @@ -273,6 +273,7 @@ public final class com/posthog/server/PostHogFeatureFlagEvaluations { public final fun getFlagPayloadAs (Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Object; public final fun getKeys ()Ljava/util/List; public final fun getRequestId ()Ljava/lang/String; + public final fun getUnresolvedFlags ()Ljava/util/Map; public final fun isEnabled (Ljava/lang/String;)Z public final fun only (Lcom/posthog/server/PostHogFeatureFlagFilter;)Lcom/posthog/server/PostHogFeatureFlagEvaluations; public final fun only (Ljava/util/Collection;)Lcom/posthog/server/PostHogFeatureFlagEvaluations; @@ -515,6 +516,17 @@ public final class com/posthog/server/PostHogRequestContextData { public final fun getSessionId ()Ljava/lang/String; } +public final class com/posthog/server/PostHogUnresolvedFlagReason : java/lang/Enum { + public static final field EXPERIENCE_CONTINUITY Lcom/posthog/server/PostHogUnresolvedFlagReason; + public static final field MISSING_CONTEXT Lcom/posthog/server/PostHogUnresolvedFlagReason; + public static final field UNRESOLVED_DEPENDENCY Lcom/posthog/server/PostHogUnresolvedFlagReason; + public static final field UNSUPPORTED_DEFINITION Lcom/posthog/server/PostHogUnresolvedFlagReason; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public final fun getValue ()Ljava/lang/String; + public static fun valueOf (Ljava/lang/String;)Lcom/posthog/server/PostHogUnresolvedFlagReason; + public static fun values ()[Lcom/posthog/server/PostHogUnresolvedFlagReason; +} + public final class com/posthog/server/otel/PostHogContextSpanProcessor : io/opentelemetry/sdk/trace/SpanProcessor { public fun (Lcom/posthog/server/PostHogInterface;)V public fun forceFlush ()Lio/opentelemetry/sdk/common/CompletableResultCode; diff --git a/posthog-server/src/main/java/com/posthog/server/PostHog.kt b/posthog-server/src/main/java/com/posthog/server/PostHog.kt index 4af51b7f7..53aad0136 100644 --- a/posthog-server/src/main/java/com/posthog/server/PostHog.kt +++ b/posthog-server/src/main/java/com/posthog/server/PostHog.kt @@ -453,6 +453,7 @@ public class PostHog : PostHogStateless(), PostHogInterface { responseError = result.responseError, host = evaluationsHost, groups = groups, + unresolvedFlags = result.unresolvedFlags, ) } diff --git a/posthog-server/src/main/java/com/posthog/server/PostHogFeatureFlagEvaluations.kt b/posthog-server/src/main/java/com/posthog/server/PostHogFeatureFlagEvaluations.kt index 5165158ea..42ce38ce8 100644 --- a/posthog-server/src/main/java/com/posthog/server/PostHogFeatureFlagEvaluations.kt +++ b/posthog-server/src/main/java/com/posthog/server/PostHogFeatureFlagEvaluations.kt @@ -16,11 +16,13 @@ import java.util.Collections * - [isEnabled] / [getFlag] fire `$feature_flag_called` (deduped by the same per-distinct-id LRU * used by [PostHogInterface.getFeatureFlag]). Empty/blank distinctId short-circuits the event. * Reads for unknown keys still fire a `$feature_flag_called` event with - * `$feature_flag_error: flag_missing` so dashboards see the lookup attempt. + * `$feature_flag_error: flag_missing` so dashboards see the lookup attempt. Reads for keys in + * [unresolvedFlags] report `local_evaluation_inconclusive` instead. * - [getFlagPayload] does not fire any event. * * Filtered clones from [onlyAccessed] / [only] are independent of the parent — accessing flags on - * the clone does not back-propagate into the parent's "accessed" set. + * the clone does not back-propagate into the parent's "accessed" set. Clones do not carry + * [unresolvedFlags]; inspect them on the original snapshot. * * @property distinctId The distinct ID the snapshot was evaluated for, or null for an empty snapshot. * @property requestId Request ID returned by the `/flags` API, when available. @@ -38,11 +40,24 @@ public class PostHogFeatureFlagEvaluations internal constructor( private val host: EvaluationsHost, initialAccessed: Set = emptySet(), groups: Map? = null, + unresolvedFlags: Map = emptyMap(), ) { private val flagMap: Map = Collections.unmodifiableMap(LinkedHashMap(flagMap)) private val locallyEvaluated: Map = Collections.unmodifiableMap(HashMap(locallyEvaluated)) private val groups: Map? = groups?.let { Collections.unmodifiableMap(HashMap(it)) } + /** + * Flags in the requested scope that have a local definition but no value in this snapshot, + * because local evaluation could not resolve them and no `/flags` fallback filled them. This + * happens with local-only evaluation, or when the fallback fails or does not return the flag. + * The flags stay absent from [keys] and read as missing. Reading this map does not fire any + * event and does not record an access. When several causes apply to a flag, the reason is the + * first one in the order of [PostHogUnresolvedFlagReason]. Always empty on snapshots returned by + * [only] and [onlyAccessed]. + */ + public val unresolvedFlags: Map = + Collections.unmodifiableMap(LinkedHashMap(unresolvedFlags)) + private val accessLock = Any() private val accessed: MutableSet = HashSet(initialAccessed) @@ -246,7 +261,15 @@ public class PostHogFeatureFlagEvaluations internal constructor( val error = buildList { responseError?.let { add(it) } - if (flag == null) add(FeatureFlagError.FLAG_MISSING) + if (flag == null) { + add( + if (key in unresolvedFlags) { + FeatureFlagError.LOCAL_EVALUATION_INCONCLUSIVE + } else { + FeatureFlagError.FLAG_MISSING + }, + ) + } } if (error.isNotEmpty()) { props["\$feature_flag_error"] = error.joinToString(",") diff --git a/posthog-server/src/main/java/com/posthog/server/PostHogUnresolvedFlagReason.kt b/posthog-server/src/main/java/com/posthog/server/PostHogUnresolvedFlagReason.kt new file mode 100644 index 000000000..98201c2a3 --- /dev/null +++ b/posthog-server/src/main/java/com/posthog/server/PostHogUnresolvedFlagReason.kt @@ -0,0 +1,32 @@ +package com.posthog.server + +/** + * Why local evaluation could not resolve a flag that has a loaded definition. Reported by + * [PostHogFeatureFlagEvaluations.unresolvedFlags]. Each reason says what the caller can do about it. + * When several causes apply to one flag, the SDK reports the first reason in the order listed here. + * + * @property value The stable string for this reason, as defined by the SDK spec. + */ +public enum class PostHogUnresolvedFlagReason(public val value: String) { + /** The flag has experience continuity enabled. Local evaluation never resolves it; change the flag. */ + EXPERIENCE_CONTINUITY("experience_continuity"), + + /** + * The flag uses something the local evaluator does not support, such as a static cohort, an + * unrecognized operator, a malformed value, a dependency with no loaded definition or a circular + * dependency. Change the flag or upgrade the SDK. + */ + UNSUPPORTED_DEFINITION("unsupported_definition"), + + /** + * A flag this flag depends on could not be resolved. Check the dependency's own reason; it appears + * in [PostHogFeatureFlagEvaluations.unresolvedFlags] only when the dependency is in the requested scope. + */ + UNRESOLVED_DEPENDENCY("unresolved_dependency"), + + /** + * The call did not supply a property or group property the flag's conditions need, or supplied + * it in a form the evaluator cannot use. Pass it. + */ + MISSING_CONTEXT("missing_context"), +} diff --git a/posthog-server/src/main/java/com/posthog/server/internal/EvaluateFlagsResult.kt b/posthog-server/src/main/java/com/posthog/server/internal/EvaluateFlagsResult.kt index 7fb3ec65e..5923fe8b7 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/EvaluateFlagsResult.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/EvaluateFlagsResult.kt @@ -1,6 +1,7 @@ package com.posthog.server.internal import com.posthog.internal.FeatureFlag +import com.posthog.server.PostHogUnresolvedFlagReason /** * The rich envelope returned by [PostHogFeatureFlags.evaluateFlags]. Holds the per-flag results @@ -18,4 +19,6 @@ internal data class EvaluateFlagsResult( * events so they match what the per-flag accessor path emits. */ val responseError: String?, + /** Flags with a local definition that have no value in [flags], with the reason why. */ + val unresolvedFlags: Map, ) diff --git a/posthog-server/src/main/java/com/posthog/server/internal/FeatureFlagError.kt b/posthog-server/src/main/java/com/posthog/server/internal/FeatureFlagError.kt index 318015cfd..686251982 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/FeatureFlagError.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/FeatureFlagError.kt @@ -10,6 +10,7 @@ package com.posthog.server.internal * Error values: * ERRORS_WHILE_COMPUTING: Server returned errorsWhileComputingFlags=true * FLAG_MISSING: Requested flag not in API response + * LOCAL_EVALUATION_INCONCLUSIVE: Flag has a local definition, but local evaluation could not resolve it * QUOTA_LIMITED: Rate/quota limit exceeded * TIMEOUT: Request timed out * CONNECTION_ERROR: Network connectivity issue @@ -20,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 LOCAL_EVALUATION_INCONCLUSIVE: String = "local_evaluation_inconclusive" const val QUOTA_LIMITED: String = "quota_limited" const val TIMEOUT: String = "timeout" const val CONNECTION_ERROR: String = "connection_error" diff --git a/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt b/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt index 4185aa807..d309439ae 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt @@ -11,6 +11,7 @@ import com.posthog.internal.PropertyGroup import com.posthog.internal.PropertyOperator import com.posthog.internal.PropertyType import com.posthog.internal.PropertyValue +import com.posthog.server.PostHogUnresolvedFlagReason import java.security.MessageDigest import java.time.Instant import java.time.ZoneId @@ -140,6 +141,21 @@ internal class FlagEvaluator( property: FlagProperty, propertyValues: Map, inCohort: Boolean, + ): Boolean = + try { + matchPropertyValue(property, propertyValues, inCohort) + } catch (e: InconclusiveMatchException) { + // An unusable filter outranks missing context, so check the filter before blaming the call. + if (e.reason == PostHogUnresolvedFlagReason.MISSING_CONTEXT) { + assertFilterSupported(property, inCohort) + } + throw e + } + + private fun matchPropertyValue( + property: FlagProperty, + propertyValues: Map, + inCohort: Boolean, ): Boolean { val key = property.key val propertyOperator = property.propertyOperator ?: PropertyOperator.EXACT @@ -147,7 +163,10 @@ internal class FlagEvaluator( // Check if property key exists in values if (!propertyValues.containsKey(key)) { - throw InconclusiveMatchException("Can't match properties without a given property value") + throw InconclusiveMatchException( + "Can't match properties without a given property value", + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ) } if (propertyOperator == PropertyOperator.IS_SET || propertyOperator == PropertyOperator.IS_NOT_SET) { @@ -173,37 +192,37 @@ internal class FlagEvaluator( PropertyOperator.ICONTAINS -> stringContains( - valueToString(overrideValue), + callerValueToString(overrideValue), valueToString(propertyValue), ) PropertyOperator.NOT_ICONTAINS -> !stringContains( - valueToString(overrideValue), + callerValueToString(overrideValue), valueToString(propertyValue), ) PropertyOperator.STARTS_WITH -> stringStartsWith( - valueToString(overrideValue), + callerValueToString(overrideValue), valueToString(propertyValue), ) PropertyOperator.NOT_STARTS_WITH -> !stringStartsWith( - valueToString(overrideValue), + callerValueToString(overrideValue), valueToString(propertyValue), ) PropertyOperator.ENDS_WITH -> stringEndsWith( - valueToString(overrideValue), + callerValueToString(overrideValue), valueToString(propertyValue), ) PropertyOperator.NOT_ENDS_WITH -> !stringEndsWith( - valueToString(overrideValue), + callerValueToString(overrideValue), valueToString(propertyValue), ) @@ -212,7 +231,10 @@ internal class FlagEvaluator( if (matches == null) { // A malformed cohort leaf must not become true when its result is negated. if (inCohort || property.negation == true) { - throw InconclusiveMatchException("Cannot negate an invalid regex criterion locally") + throw InconclusiveMatchException( + "Cannot negate an invalid regex criterion locally", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } false } else if (propertyOperator == PropertyOperator.REGEX) { @@ -245,10 +267,153 @@ internal class FlagEvaluator( propertyOperator, ) - else -> throw InconclusiveMatchException("Unknown operator: $propertyOperator") + else -> throw InconclusiveMatchException( + "Unknown operator: $propertyOperator", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } } + /** + * Throws an [PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION] error when local evaluation + * cannot use the filter, whatever the call supplies: an unknown operator, or a filter value + * that the operator cannot parse. + */ + private fun assertFilterSupported( + property: FlagProperty, + inCohort: Boolean, + ) { + val value = property.propertyValue + when (property.propertyOperator ?: PropertyOperator.EXACT) { + PropertyOperator.EXACT, PropertyOperator.IS_NOT -> (value as? List<*> ?: listOf(value)).forEach { valueToString(it) } + + PropertyOperator.IS_SET, PropertyOperator.IS_NOT_SET -> Unit + + PropertyOperator.ICONTAINS, + PropertyOperator.NOT_ICONTAINS, + PropertyOperator.STARTS_WITH, + PropertyOperator.NOT_STARTS_WITH, + PropertyOperator.ENDS_WITH, + PropertyOperator.NOT_ENDS_WITH, + -> valueToString(value) + + PropertyOperator.REGEX, PropertyOperator.NOT_REGEX -> { + val pattern = value.toString() + if (pattern.isNotEmpty() && compileRegex(pattern) == null && (inCohort || property.negation == true)) { + throw InconclusiveMatchException( + "Cannot negate an invalid regex criterion locally", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } + } + + PropertyOperator.GT, PropertyOperator.GTE, PropertyOperator.LT, PropertyOperator.LTE -> parseNumberFilter(value) + + PropertyOperator.IS_DATE_BEFORE, PropertyOperator.IS_DATE_AFTER -> parseDateFilter(value) + + PropertyOperator.SEMVER_EQ, + PropertyOperator.SEMVER_NEQ, + PropertyOperator.SEMVER_GT, + PropertyOperator.SEMVER_GTE, + PropertyOperator.SEMVER_LT, + PropertyOperator.SEMVER_LTE, + -> parseFlagConditionSemver(value.toString()) + + PropertyOperator.SEMVER_TILDE -> computeTildeBounds(value.toString()) + + PropertyOperator.SEMVER_CARET -> computeCaretBounds(value.toString()) + + PropertyOperator.SEMVER_WILDCARD -> computeWildcardFilterBounds(value.toString()) + + else -> throw InconclusiveMatchException( + "Unknown operator: ${property.propertyOperator}", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } + } + + /** + * The unresolved reason that comes from the flag's definition alone, so it is known when + * definitions load: [PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY] or + * [PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION]. Matches the reason local evaluation + * reports for the same cause. Null when the definition alone does not stop local evaluation. + */ + fun definitionUnresolvedReason( + flag: FlagDefinition, + flagsByKey: Map, + groupTypeMapping: Map, + cohortProperties: Map, + ): PostHogUnresolvedFlagReason? { + if (!flag.active) { + return null + } + if (flag.ensureExperienceContinuity) { + return PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY + } + val aggregationGroupIndex = flag.filters.aggregationGroupTypeIndex + if (aggregationGroupIndex != null && groupTypeMapping[aggregationGroupIndex.toString()] == null) { + return PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION + } + val visitedCohortIds = mutableSetOf() + val usesUnsupportedFilter = + flag.filters.groups.orEmpty().any { condition -> + condition.properties.orEmpty().any { property -> + isUnsupportedFilter(property, inCohort = false, flagsByKey, cohortProperties, visitedCohortIds) + } + } + return if (usesUnsupportedFilter) PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION else null + } + + private fun isUnsupportedFilter( + property: FlagProperty, + inCohort: Boolean, + flagsByKey: Map, + cohortProperties: Map, + visitedCohortIds: MutableSet, + ): Boolean = + when (property.type) { + PropertyType.FLAG -> { + val dependencyChain = property.dependencyChain + dependencyChain.isNullOrEmpty() || + dependencyChain.any { it !in flagsByKey } || + (property.propertyValue != null && property.propertyOperator != PropertyOperator.FLAG_EVALUATES_TO) + } + + PropertyType.COHORT -> { + val cohortId = property.propertyValue?.toString() + val cohort = cohortId?.let { cohortProperties[it] } + if (cohortId == null || cohort == null) { + // Static cohorts are absent from the local cohort map. + true + } else { + visitedCohortIds.add(cohortId) && + isUnsupportedPropertyGroup(cohort, flagsByKey, cohortProperties, visitedCohortIds) + } + } + + else -> + try { + assertFilterSupported(property, inCohort) + false + } catch (e: InconclusiveMatchException) { + true + } + } + + private fun isUnsupportedPropertyGroup( + group: PropertyGroup, + flagsByKey: Map, + cohortProperties: Map, + visitedCohortIds: MutableSet, + ): Boolean = + when (val values = group.values) { + null -> false + is PropertyValue.PropertyGroups -> + values.values.any { isUnsupportedPropertyGroup(it, flagsByKey, cohortProperties, visitedCohortIds) } + is PropertyValue.FlagProperties -> + values.values.any { isUnsupportedFilter(it, inCohort = true, flagsByKey, cohortProperties, visitedCohortIds) } + } + private fun computeExactMatch( propertyValue: Any?, overrideValue: Any?, @@ -261,7 +426,7 @@ internal class FlagEvaluator( return isTruthyPropertyValue(propertyValue) == isTruthyPropertyValue(overrideValue) } - val expectedValue = unicodeLowercase(valueToString(overrideValue)) + val expectedValue = unicodeLowercase(callerValueToString(overrideValue)) return when (propertyValue) { is List<*> -> propertyValue.any { value -> @@ -288,6 +453,14 @@ internal class FlagEvaluator( else -> false } + // A caller value that cannot be stringified is the caller's to fix, not the flag's. + private fun callerValueToString(value: Any?): String = + try { + valueToString(value) + } catch (e: InconclusiveMatchException) { + throw InconclusiveMatchException(e.message.orEmpty(), PostHogUnresolvedFlagReason.MISSING_CONTEXT, e) + } + private fun valueToString(value: Any?): String { if (value is String) { return value @@ -298,8 +471,13 @@ internal class FlagEvaluator( val jsonValue = try { JSON.toJsonTree(value) - } catch (error: IllegalArgumentException) { - throw InconclusiveMatchException("Cannot stringify JSON value locally: ${error.message}") + } catch (error: RuntimeException) { + // Gson throws several runtime exceptions for values it cannot reflect, such as java.time types. + throw InconclusiveMatchException( + "Cannot stringify JSON value locally: ${error.message}", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + error, + ) } return canonicalJson(jsonValue) } @@ -326,7 +504,10 @@ internal class FlagEvaluator( if (rawValue.matches(Regex("-?\\d+"))) { return rawValue } - throw InconclusiveMatchException("Cannot stringify floating-point JSON number locally: $rawValue") + throw InconclusiveMatchException( + "Cannot stringify floating-point JSON number locally: $rawValue", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } private fun compareJsonKeys( @@ -360,14 +541,20 @@ internal class FlagEvaluator( in '\uD800'..'\uDBFF' -> { val next = value.getOrNull(index + 1) if (next == null || next !in '\uDC00'..'\uDFFF') { - throw InconclusiveMatchException("Cannot stringify an unpaired Unicode surrogate") + throw InconclusiveMatchException( + "Cannot stringify an unpaired Unicode surrogate", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } append(character) } in '\uDC00'..'\uDFFF' -> { val previous = value.getOrNull(index - 1) if (previous == null || previous !in '\uD800'..'\uDBFF') { - throw InconclusiveMatchException("Cannot stringify an unpaired Unicode surrogate") + throw InconclusiveMatchException( + "Cannot stringify an unpaired Unicode surrogate", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } append(character) } @@ -401,32 +588,57 @@ internal class FlagEvaluator( propertyValue: String, ): Boolean? { if (pattern.isEmpty()) return true - val portablePattern = portableRegexPattern(pattern) - val regex = - try { - Regex(portablePattern, RegexOption.UNIX_LINES) - } catch (e: PatternSyntaxException) { - return null - } catch (e: RuntimeException) { - throw InconclusiveMatchException("Regex compilation requires server evaluation") - } catch (e: StackOverflowError) { - throw InconclusiveMatchException("Regex compilation requires server evaluation") - } + val regex = compileRegex(pattern) ?: return null // JVM shorthand classes and Unicode versions differ from the service's Rust engine. if (propertyValue.any { it.code > 0x7f }) { - throw InconclusiveMatchException("Regex Unicode matching requires server evaluation") + throw InconclusiveMatchException( + "Regex Unicode matching requires server evaluation", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } return try { regex.find(propertyValue) != null } catch (e: RuntimeException) { - throw InconclusiveMatchException("Regex execution requires server evaluation") + throw InconclusiveMatchException( + "Regex execution requires server evaluation", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } catch (e: StackOverflowError) { - throw InconclusiveMatchException("Regex execution requires server evaluation") + throw InconclusiveMatchException( + "Regex execution requires server evaluation", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } + } + + /** + * Returns null for a pattern the JVM rejects, which matches nothing unless it is negated. + */ + private fun compileRegex(pattern: String): Regex? { + val portablePattern = portableRegexPattern(pattern) + return try { + Regex(portablePattern, RegexOption.UNIX_LINES) + } catch (e: PatternSyntaxException) { + null + } catch (e: RuntimeException) { + throw InconclusiveMatchException( + "Regex compilation requires server evaluation", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } catch (e: StackOverflowError) { + throw InconclusiveMatchException( + "Regex compilation requires server evaluation", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } } private fun portableRegexPattern(pattern: String): String { - fun unsupported(): Nothing = throw InconclusiveMatchException("Regex syntax requires server evaluation") + fun unsupported(): Nothing = + throw InconclusiveMatchException( + "Regex syntax requires server evaluation", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) val result = StringBuilder() var classStart = -1 @@ -553,13 +765,18 @@ internal class FlagEvaluator( propertyValue: Any?, propertyOperator: PropertyOperator, ): Boolean { - val numericValue = - parseFiniteNumber(propertyValue) - ?: throw InconclusiveMatchException("The number set on the flag is not a valid finite number") + val numericValue = parseNumberFilter(propertyValue) val numericOverride = parseFiniteNumber(overrideValue) ?: return false return compareNumbers(numericOverride, numericValue, propertyOperator) } + private fun parseNumberFilter(propertyValue: Any?): Double = + parseFiniteNumber(propertyValue) + ?: throw InconclusiveMatchException( + "The number set on the flag is not a valid finite number", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + private fun parseFiniteNumber(value: Any?): Double? { val number = when (value) { @@ -589,12 +806,7 @@ internal class FlagEvaluator( propertyValue: Any?, propertyOperator: PropertyOperator, ): Boolean { - val parsedDate = - try { - parseDateValue(propertyValue.toString()) - } catch (e: Exception) { - throw InconclusiveMatchException("The date set on the flag is not a valid format") - } + val parsedDate = parseDateFilter(propertyValue) val overrideDate = when (overrideValue) { @@ -605,11 +817,17 @@ internal class FlagEvaluator( try { parseOverrideDate(overrideValue) } catch (e: Exception) { - throw InconclusiveMatchException("The date provided is not a valid format") + throw InconclusiveMatchException( + "The date provided is not a valid format", + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ) } } - else -> throw InconclusiveMatchException("The date provided must be a string or date object") + else -> throw InconclusiveMatchException( + "The date provided must be a string or date object", + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ) } return when (propertyOperator) { @@ -619,6 +837,16 @@ internal class FlagEvaluator( } } + private fun parseDateFilter(propertyValue: Any?): ZonedDateTime = + try { + parseDateValue(propertyValue.toString()) + } catch (e: Exception) { + throw InconclusiveMatchException( + "The date set on the flag is not a valid format", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } + /** * Parse date value from flag definition, supporting relative dates */ @@ -767,20 +995,32 @@ internal class FlagEvaluator( val match = REGEX_SEMVER.matchEntire(cleaned) - ?: throw InconclusiveMatchException("Invalid semver version: '$version'") + ?: throw InconclusiveMatchException( + "Invalid semver version: '$version'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) val major = parseSemverNumericIdentifier(match.groupValues[1]) - ?: throw InconclusiveMatchException("Invalid semver version: '$version'") + ?: throw InconclusiveMatchException( + "Invalid semver version: '$version'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) val minor = match.groupValues[2].takeIf { it.isNotEmpty() }?.let { parseSemverNumericIdentifier(it) - ?: throw InconclusiveMatchException("Invalid semver version: '$version'") + ?: throw InconclusiveMatchException( + "Invalid semver version: '$version'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } ?: 0 val patch = match.groupValues[3].takeIf { it.isNotEmpty() }?.let { parseSemverNumericIdentifier(it) - ?: throw InconclusiveMatchException("Invalid semver version: '$version'") + ?: throw InconclusiveMatchException( + "Invalid semver version: '$version'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } ?: 0 return SemverVersion(major, minor, patch) @@ -798,7 +1038,10 @@ internal class FlagEvaluator( return try { parseSemver(propertyValue) } catch (e: InconclusiveMatchException) { - throw InconclusiveMatchException("The flag condition value is not a valid semver: ${e.message}") + throw InconclusiveMatchException( + "The flag condition value is not a valid semver: ${e.message}", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } } @@ -811,15 +1054,10 @@ internal class FlagEvaluator( propertyValue: Any?, propertyOperator: PropertyOperator, ): Boolean { - val overrideVersion = - try { - parseSemver(overrideValue.toString()) - } catch (e: InconclusiveMatchException) { - throw InconclusiveMatchException("The person property value is not a valid semver: ${e.message}") - } - val propertyString = propertyValue.toString() + // Each branch parses the filter before the person value, so a bad filter reports an + // unsupported definition even when the person value is bad too. return when (propertyOperator) { PropertyOperator.SEMVER_EQ, PropertyOperator.SEMVER_NEQ, @@ -829,33 +1067,50 @@ internal class FlagEvaluator( PropertyOperator.SEMVER_LTE, -> { val conditionVersion = parseFlagConditionSemver(propertyString) - compareSemverVersions(overrideVersion, conditionVersion, propertyOperator) + compareSemverVersions(parsePersonSemver(overrideValue), conditionVersion, propertyOperator) } - PropertyOperator.SEMVER_TILDE -> { - val (lower, upper) = computeTildeBounds(propertyString) - overrideVersion >= lower && overrideVersion < upper - } + PropertyOperator.SEMVER_TILDE -> isPersonSemverWithin(overrideValue, computeTildeBounds(propertyString)) - PropertyOperator.SEMVER_CARET -> { - val (lower, upper) = computeCaretBounds(propertyString) - overrideVersion >= lower && overrideVersion < upper - } + PropertyOperator.SEMVER_CARET -> isPersonSemverWithin(overrideValue, computeCaretBounds(propertyString)) - PropertyOperator.SEMVER_WILDCARD -> { - val (lower, upper) = - try { - computeWildcardBounds(propertyString) - } catch (e: InconclusiveMatchException) { - throw InconclusiveMatchException("The flag condition value is not a valid semver: ${e.message}") - } - overrideVersion >= lower && overrideVersion < upper - } + PropertyOperator.SEMVER_WILDCARD -> isPersonSemverWithin(overrideValue, computeWildcardFilterBounds(propertyString)) + + else -> throw InconclusiveMatchException( + "Unknown semver operator: $propertyOperator", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } + } - else -> throw InconclusiveMatchException("Unknown semver operator: $propertyOperator") + private fun parsePersonSemver(overrideValue: Any?): SemverVersion = + try { + parseSemver(overrideValue.toString()) + } catch (e: InconclusiveMatchException) { + throw InconclusiveMatchException( + "The person property value is not a valid semver: ${e.message}", + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ) } + + private fun isPersonSemverWithin( + overrideValue: Any?, + bounds: Pair, + ): Boolean { + val overrideVersion = parsePersonSemver(overrideValue) + return overrideVersion >= bounds.first && overrideVersion < bounds.second } + private fun computeWildcardFilterBounds(propertyString: String): Pair = + try { + computeWildcardBounds(propertyString) + } catch (e: InconclusiveMatchException) { + throw InconclusiveMatchException( + "The flag condition value is not a valid semver: ${e.message}", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } + /** * Compare two parsed semver versions */ @@ -925,7 +1180,10 @@ internal class FlagEvaluator( cleaned = cleaned.trimEnd('*', '.') if (cleaned.isEmpty()) { - throw InconclusiveMatchException("Invalid wildcard version: '$propertyValue'") + throw InconclusiveMatchException( + "Invalid wildcard version: '$propertyValue'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } val parts = cleaned.split(".") @@ -935,12 +1193,18 @@ internal class FlagEvaluator( if (part.isEmpty()) continue val num = parseSemverNumericIdentifier(part) - ?: throw InconclusiveMatchException("Invalid wildcard version: '$propertyValue'") + ?: throw InconclusiveMatchException( + "Invalid wildcard version: '$propertyValue'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) components.add(num) } if (components.isEmpty()) { - throw InconclusiveMatchException("Invalid wildcard version: '$propertyValue'") + throw InconclusiveMatchException( + "Invalid wildcard version: '$propertyValue'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } return when (components.size) { @@ -971,15 +1235,19 @@ internal class FlagEvaluator( ): Boolean { val cohortId = property.propertyValue?.toString() - ?: throw InconclusiveMatchException("Cohort property missing value") + ?: throw InconclusiveMatchException("Cohort property missing value", PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION) + // Static cohorts are absent from the local cohort map, so they never resolve locally. if (!cohortProperties.containsKey(cohortId)) { - throw InconclusiveMatchException("Can't match cohort without a given cohort property value") + throw InconclusiveMatchException( + "Can't match cohort without a given cohort property value", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } val propertyGroup = cohortProperties[cohortId] - ?: throw InconclusiveMatchException("Cohort definition not found") + ?: throw InconclusiveMatchException("Cohort definition not found", PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION) return matchPropertyGroup( propertyGroup, propertyValues, @@ -1007,7 +1275,7 @@ internal class FlagEvaluator( // Empty properties always match if (properties == null || properties.isEmpty()) return true - var errorMatchingLocally = false + var inconclusiveReason: PostHogUnresolvedFlagReason? = null // Handle based on whether we have nested property groups or flag properties when (properties) { @@ -1032,12 +1300,12 @@ internal class FlagEvaluator( } } catch (e: InconclusiveMatchException) { config.logger.log("Failed to compute nested property group locally: ${e.message}") - errorMatchingLocally = true + inconclusiveReason = pickUnresolvedReason(inconclusiveReason, e.reason) } } - if (errorMatchingLocally) { - throw InconclusiveMatchException("Can't match cohort without a given cohort property value") + inconclusiveReason?.let { + throw InconclusiveMatchException("Can't match cohort without a given cohort property value", it) } // If we get here, all matched in AND case, or none matched in OR case @@ -1064,13 +1332,20 @@ internal class FlagEvaluator( evaluateFlagDependency( property, flagsByKey - ?: throw InconclusiveMatchException("Cannot evaluate flag dependencies without flagsByKey"), + ?: throw InconclusiveMatchException( + "Cannot evaluate flag dependencies without flagsByKey", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), evaluationCache ?: throw InconclusiveMatchException( "Cannot evaluate flag dependencies without evaluationCache", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, ), distinctId - ?: throw InconclusiveMatchException("Cannot evaluate flag dependencies without distinctId"), + ?: throw InconclusiveMatchException( + "Cannot evaluate flag dependencies without distinctId", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), propertyValues, cohortProperties, ) @@ -1091,12 +1366,12 @@ internal class FlagEvaluator( } } catch (e: InconclusiveMatchException) { config.logger.log("Failed to compute property ${property.key} locally: ${e.message}") - errorMatchingLocally = true + inconclusiveReason = pickUnresolvedReason(inconclusiveReason, e.reason) } } - if (errorMatchingLocally) { - throw InconclusiveMatchException("Can't match cohort without a given cohort property value") + inconclusiveReason?.let { + throw InconclusiveMatchException("Can't match cohort without a given cohort property value", it) } // If we get here, all matched in AND case, or none matched in OR case @@ -1122,32 +1397,56 @@ internal class FlagEvaluator( // Check all properties match if (conditionProperties.isNotEmpty()) { + // Filters in a condition have no defined order, so an inconclusive filter does not stop + // the loop: a later filter can still rule the condition out or carry a higher-precedence reason. + var inconclusiveReason: PostHogUnresolvedFlagReason? = null for (prop in conditionProperties) { val matches = - when (prop.type) { - PropertyType.COHORT -> - matchCohort( - prop, - properties, - cohortProperties, - flagsByKey, - evaluationCache, - distinctId, - ) + try { + when (prop.type) { + PropertyType.COHORT -> + matchCohort( + prop, + properties, + cohortProperties, + flagsByKey, + evaluationCache, + distinctId, + ) - PropertyType.FLAG -> - evaluateFlagDependency( - prop, - flagsByKey - ?: throw InconclusiveMatchException("Cannot evaluate flag dependencies without flagsByKey"), - evaluationCache - ?: throw InconclusiveMatchException("Cannot evaluate flag dependencies without evaluationCache"), - distinctId, - properties, - cohortProperties, - ) + PropertyType.FLAG -> + evaluateFlagDependency( + prop, + flagsByKey + ?: throw InconclusiveMatchException( + "Cannot evaluate flag dependencies without flagsByKey", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + evaluationCache + ?: throw InconclusiveMatchException( + "Cannot evaluate flag dependencies without evaluationCache", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + distinctId, + properties, + cohortProperties, + ) - else -> matchProperty(prop, properties) + else -> { + if (!properties.containsKey(prop.key)) { + // Same result as matchProperty for an absent key, without the cost + // of throwing for every missing filter. + assertFilterSupported(prop, inCohort = false) + inconclusiveReason = + pickUnresolvedReason(inconclusiveReason, PostHogUnresolvedFlagReason.MISSING_CONTEXT) + continue + } + matchProperty(prop, properties) + } + } + } catch (e: InconclusiveMatchException) { + inconclusiveReason = pickUnresolvedReason(inconclusiveReason, e.reason) + continue } if (!matches) { @@ -1155,6 +1454,10 @@ internal class FlagEvaluator( } } + inconclusiveReason?.let { + throw InconclusiveMatchException("Can't determine if a condition of flag '${featureFlag.key}' matches", it) + } + // All properties matched, check rollout if (rolloutPercentage == null) { return true @@ -1186,11 +1489,14 @@ internal class FlagEvaluator( evaluationCache: MutableMap? = null, ): Any? { if (flag.ensureExperienceContinuity) { - throw InconclusiveMatchException("Flag \"${flag.key}\" has experience continuity enabled") + throw InconclusiveMatchException( + "Flag \"${flag.key}\" has experience continuity enabled", + PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY, + ) } val flagConditions = flag.filters.groups ?: emptyList() - var isInconclusive = false + var inconclusiveReason: PostHogUnresolvedFlagReason? = null // Get variant keys for validation val flagVariants = flag.filters.multivariate?.variants ?: emptyList() @@ -1226,12 +1532,12 @@ internal class FlagEvaluator( return variant ?: true } } catch (e: InconclusiveMatchException) { - isInconclusive = true + inconclusiveReason = pickUnresolvedReason(inconclusiveReason, e.reason) } } - if (isInconclusive) { - throw InconclusiveMatchException("Can't determine if feature flag is enabled or not with given properties") + inconclusiveReason?.let { + throw InconclusiveMatchException("Can't determine if feature flag is enabled or not with given properties", it) } // We can only return False when either all conditions are False, or no condition was inconclusive @@ -1254,15 +1560,28 @@ internal class FlagEvaluator( if (dependencyChain == null) { throw InconclusiveMatchException( "Flag dependency property for '${property.key}' is missing required 'dependency_chain' field", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) } // Handle circular dependency (empty chain means circular) if (dependencyChain.isEmpty()) { config.logger.log("Circular dependency detected for flag: ${property.key}") - throw InconclusiveMatchException("Circular dependency detected for flag '${property.key}'") + throw InconclusiveMatchException( + "Circular dependency detected for flag '${property.key}'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } + // A dependency with no loaded definition outranks one that is only inconclusive, wherever + // it sits in the chain. + fun unresolvedChainReason(): PostHogUnresolvedFlagReason = + if (dependencyChain.any { it !in flagsByKey }) { + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION + } else { + PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY + } + // Evaluate all dependencies in the chain order for (depFlagKey in dependencyChain) { if (!evaluationCache.containsKey(depFlagKey)) { @@ -1273,6 +1592,8 @@ internal class FlagEvaluator( evaluationCache[depFlagKey] = null throw InconclusiveMatchException( "Cannot evaluate flag dependency '$depFlagKey' - flag not found in local flags", + // No entry can exist for an unloaded dependency, so the definition itself is the cause. + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) } else { // Check if the flag is active @@ -1294,7 +1615,11 @@ internal class FlagEvaluator( } catch (e: InconclusiveMatchException) { // If we can't evaluate a dependency, store null and propagate the error evaluationCache[depFlagKey] = null - throw InconclusiveMatchException("Cannot evaluate flag dependency '$depFlagKey': ${e.message}") + throw InconclusiveMatchException( + "Cannot evaluate flag dependency '$depFlagKey': ${e.message}", + unresolvedChainReason(), + e, + ) } } } @@ -1304,7 +1629,10 @@ internal class FlagEvaluator( val cachedResult = evaluationCache[depFlagKey] if (cachedResult == null) { // Previously inconclusive - raise error again - throw InconclusiveMatchException("Flag dependency '$depFlagKey' was previously inconclusive") + throw InconclusiveMatchException( + "Flag dependency '$depFlagKey' was previously inconclusive", + unresolvedChainReason(), + ) } else if (cachedResult == false && depFlagKey != property.key) { // Definitive false result for intermediate dependency - chain failed // Only return false early if this is NOT the final flag we're checking @@ -1324,14 +1652,21 @@ internal class FlagEvaluator( if (actualValue == null) { // Flag wasn't evaluated - this shouldn't happen if dependency chain is correct - throw InconclusiveMatchException("Flag '$flagKey' was not evaluated despite being in dependency chain") + throw InconclusiveMatchException( + "Flag '$flagKey' was not evaluated despite being in dependency chain", + // Every chain entry resolved, so the chain itself leaves out the flag. + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } // For flag dependencies, we need to compare the actual flag result with expected value if (propertyOperator == PropertyOperator.FLAG_EVALUATES_TO) { return matchesDependencyValue(expectedValue, actualValue) } else { - throw InconclusiveMatchException("Flag dependency property for '${property.key}' has invalid operator '$propertyOperator'") + throw InconclusiveMatchException( + "Flag dependency property for '${property.key}' has invalid operator '$propertyOperator'", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } } diff --git a/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt b/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt index 43c0a750b..603a99167 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt @@ -1,6 +1,32 @@ package com.posthog.server.internal +import com.posthog.server.PostHogUnresolvedFlagReason + /** * Exception thrown when flag evaluation cannot be determined locally */ -internal class InconclusiveMatchException(message: String) : Exception(message) +internal class InconclusiveMatchException( + message: String, + val reason: PostHogUnresolvedFlagReason, + cause: Throwable? = null, +) : Exception(message, cause) + +// When several causes apply to one flag, the spec reports the first reason in this order, so every +// SDK picks the same reason whatever order it inspects the conditions and filters in. +private val UNRESOLVED_REASON_PRECEDENCE = + listOf( + PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY, + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ) + +internal fun pickUnresolvedReason( + current: PostHogUnresolvedFlagReason?, + next: PostHogUnresolvedFlagReason, +): PostHogUnresolvedFlagReason = + if (current == null || UNRESOLVED_REASON_PRECEDENCE.indexOf(next) < UNRESOLVED_REASON_PRECEDENCE.indexOf(current)) { + next + } else { + current + } diff --git a/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt b/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt index 923081b97..b43395e8e 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt @@ -12,6 +12,7 @@ import com.posthog.internal.PostHogFeatureFlagsInterface import com.posthog.internal.PostHogFlagsResponse import com.posthog.internal.PropertyGroup import com.posthog.server.PostHogFlagDefinitionCacheProvider +import com.posthog.server.PostHogUnresolvedFlagReason import java.io.IOException import java.io.StringReader import java.io.StringWriter @@ -77,6 +78,9 @@ internal class PostHogFeatureFlags( @Volatile private var definitionsLoadedAt: Long? = null + // Flag key to the fingerprint of the definition last warned about as never locally resolvable. + private val warnedUnresolvableFlags = mutableMapOf() + @Volatile private var isLoading = false @@ -251,14 +255,14 @@ internal class PostHogFeatureFlags( /** * The result of one local evaluation pass. [flags] holds every flag that resolved, whether or - * not [needsRemote] is set, so callers can fill only the gaps from `/flags`. - * [continuityFlagKeys] holds the flags that did not resolve because experience continuity is on. + * not [needsRemote] is set, so callers can fill only the gaps from `/flags`. [unresolved] holds + * why each flag with a local definition did not resolve. */ private data class LocalEvaluationOutcome( val flags: Map, val needsRemote: Boolean, val missingDefinitionKeys: Set, - val continuityFlagKeys: Set, + val unresolved: Map, ) private class MissingFlagProbe { @@ -316,10 +320,10 @@ internal class PostHogFeatureFlags( config.logger.log("Attempting local evaluation for distinctId: $distinctId") val localFlags = mutableMapOf() + val unresolved = mutableMapOf() val props = localPersonProperties(distinctId, personProperties) val requestedKeys = flagKeys?.toHashSet() var needsRemote = false - val continuityFlagKeys = mutableSetOf() for ((key, flagDef) in currentFlagDefinitions) { if (requestedKeys != null && key !in requestedKeys) { @@ -340,10 +344,8 @@ internal class PostHogFeatureFlags( localFlags[key] = buildFeatureFlagFromResult(key, result, flagDef) } catch (e: InconclusiveMatchException) { config.logger.log("Local evaluation inconclusive for flag '$key': ${e.message}") + unresolved[key] = e.reason needsRemote = true - if (flagDef.ensureExperienceContinuity) { - continuityFlagKeys.add(key) - } } catch (e: Exception) { config.logger.log("Local evaluation failed for flag '$key': ${e.message}") needsRemote = true @@ -364,7 +366,7 @@ internal class PostHogFeatureFlags( } config.logger.log("Local evaluation resolved ${localFlags.size} flags, needsRemote=$needsRemote") - return LocalEvaluationOutcome(localFlags, needsRemote, missingDefinitionKeys, continuityFlagKeys) + return LocalEvaluationOutcome(localFlags, needsRemote, missingDefinitionKeys, unresolved) } /** @@ -777,6 +779,7 @@ internal class PostHogFeatureFlags( response: LocalEvaluationResponse, cacheData: Map?, ) { + var appliedSnapshot: DefinitionSnapshot? = null val invalidated = synchronized(missingFlagKeysLock) { synchronized(loadLock) { @@ -791,12 +794,80 @@ internal class PostHogFeatureFlags( ) cache = PostHogFeatureFlagCache(maxSize = cacheMaxSize, maxAgeMs = cacheMaxAgeMs) } + appliedSnapshot = definitionSnapshot definitionsLoaded = true definitionsLoadedAt = System.currentTimeMillis() } invalidateMissingFlagStateLocked() } invalidated.forEach { it.complete() } + appliedSnapshot?.let { warnAboutUnresolvableFlags(recordUnresolvableFlags(it)) } + } + + /** + * Returns the flags whose definition local evaluation can never resolve and that were not + * warned about at their current definition, grouped by reason, and remembers them. + * + * Only keys absent from the loaded definitions are forgotten, which keeps the map bounded. A + * flag that is inactive or resolvable for a while keeps its entry, so it warns again only when + * it returns with a different definition. + */ + private fun recordUnresolvableFlags(snapshot: DefinitionSnapshot): Map> { + val flagsByKey = snapshot.flagsByKey.orEmpty() + val groupTypeMapping = snapshot.groupTypeMapping.orEmpty() + val cohorts = snapshot.cohorts.orEmpty() + val unresolvable = linkedMapOf>() + synchronized(warnedUnresolvableFlags) { + warnedUnresolvableFlags.keys.retainAll(flagsByKey.keys) + for ((key, flag) in flagsByKey) { + val reason = + try { + snapshot.evaluator.definitionUnresolvedReason(flag, flagsByKey, groupTypeMapping, cohorts) + } catch (e: Exception) { + config.logger.log("Could not check whether flag '$key' resolves locally: ${e.message}") + null + } ?: continue + val fingerprint = definitionFingerprint(flag) + if (warnedUnresolvableFlags.put(key, fingerprint) != fingerprint) { + unresolvable.getOrPut(reason) { mutableListOf() }.add(key) + } + } + } + return unresolvable + } + + // The version alone misses edits that the endpoint serves without a version bump. + private fun definitionFingerprint(flag: FlagDefinition): String { + val json = + try { + StringWriter().also { config.serializer.serialize(flag, it) }.toString() + } catch (e: Throwable) { + "" + } + return "${flag.version}:${json.hashCode()}" + } + + private fun warnAboutUnresolvableFlags(unresolvable: Map>) { + unresolvable[PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY]?.let { keys -> + config.logger.logWarning( + "PostHog: You are using local evaluation but ${keys.size} flag(s) have experience continuity " + + "enabled and cannot be resolved locally " + + "(reason: ${PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY.value}): ${keys.joinToString(", ")}. " + + "Evaluating these flags makes a /flags request, or leaves them unresolved with local-only " + + "evaluation. To evaluate them locally, turn off experience continuity on these flags in PostHog.", + ) + } + unresolvable[PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION]?.let { keys -> + config.logger.logWarning( + "PostHog: You are using local evaluation but ${keys.size} flag(s) use something local evaluation " + + "does not support, such as a static cohort, an unknown operator, a malformed value, or a missing " + + "or circular flag dependency " + + "(reason: ${PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION.value}): ${keys.joinToString(", ")}. " + + "Evaluating these flags makes a /flags request when evaluation reaches that part, or leaves them " + + "unresolved with local-only evaluation. To evaluate them locally, change these flags in PostHog " + + "or upgrade posthog-server.", + ) + } } private fun clearKnownMissingFlagKeys() { @@ -935,7 +1006,10 @@ internal class PostHogFeatureFlags( if (groupTypeName == null) { config.logger.log("Unknown group type index $aggregationGroupIndex for flag '$key'") - throw InconclusiveMatchException("Flag has unknown group type index") + throw InconclusiveMatchException( + "Flag has unknown group type index", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) } val groupKey = groups?.get(groupTypeName) @@ -1097,6 +1171,7 @@ internal class PostHogFeatureFlags( evaluatedAt = entry.evaluatedAt, definitionsLoadedAt = definitionsLoadedAt, responseError = entry.error, + unresolvedFlags = EMPTY_UNRESOLVED_FLAGS, ) } } @@ -1120,9 +1195,11 @@ internal class PostHogFeatureFlags( } if (local != null && ((!local.needsRemote && !hasMissingKeyToProbe) || onlyEvaluateLocally)) { - if (onlyEvaluateLocally && local.continuityFlagKeys.isNotEmpty()) { + val continuityFlagKeys = + local.unresolved.filterValues { it == PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY }.keys + if (onlyEvaluateLocally && continuityFlagKeys.isNotEmpty()) { config.logger.log( - "Local-only evaluation omitted flag(s) ${local.continuityFlagKeys.joinToString(", ")}: " + + "Local-only evaluation omitted flag(s) ${continuityFlagKeys.joinToString(", ")}: " + "experience continuity is incompatible with local evaluation. " + "Allow remote evaluation or turn off experience continuity for these flags.", ) @@ -1134,6 +1211,7 @@ internal class PostHogFeatureFlags( evaluatedAt = null, definitionsLoadedAt = definitionsLoadedAt, responseError = null, + unresolvedFlags = local.unresolved, ) } @@ -1180,6 +1258,8 @@ internal class PostHogFeatureFlags( evaluatedAt = entry?.evaluatedAt, definitionsLoadedAt = definitionsLoadedAt, responseError = entry?.error, + // A flag stays unresolved only when `/flags` failed or did not return it. + unresolvedFlags = local?.unresolved?.filterKeys { it !in merged }.orEmpty(), ) } @@ -1404,6 +1484,7 @@ internal class PostHogFeatureFlags( private val EMPTY_COHORT_PROPERTIES: Map = emptyMap() private val EMPTY_FLAGS: Map = emptyMap() private val EMPTY_LOCALLY_EVALUATED: Map = emptyMap() + private val EMPTY_UNRESOLVED_FLAGS: Map = emptyMap() private val EMPTY_EVALUATE_FLAGS_RESULT = EvaluateFlagsResult( flags = EMPTY_FLAGS, @@ -1412,6 +1493,7 @@ internal class PostHogFeatureFlags( evaluatedAt = null, definitionsLoadedAt = null, responseError = null, + unresolvedFlags = EMPTY_UNRESOLVED_FLAGS, ) } } diff --git a/posthog-server/src/test/java/com/posthog/server/PostHogEvaluateFlagsTest.kt b/posthog-server/src/test/java/com/posthog/server/PostHogEvaluateFlagsTest.kt index 8cb1ea612..ee4ba0793 100644 --- a/posthog-server/src/test/java/com/posthog/server/PostHogEvaluateFlagsTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/PostHogEvaluateFlagsTest.kt @@ -777,6 +777,120 @@ internal class PostHogEvaluateFlagsTest { } } + @Test + fun `local only evaluation exposes an unresolved flag and reports it as inconclusive`() { + withLocalEvaluation(definitions = conclusiveAndGatedDefinitions()) { postHog, dispatcher, mockServer -> + val snapshot = postHog.evaluateFlags("user-1", onlyEvaluateLocally = true) + + assertEquals(mapOf("gated" to PostHogUnresolvedFlagReason.MISSING_CONTEXT), snapshot.unresolvedFlags) + assertFalse(snapshot.isEnabled("gated")) + postHog.flush() + + assertEquals(0, dispatcher.flagsCalls.get()) + val flagCalled = drainRequests(mockServer).featureFlagCalledEvents().toMap() + assertEquals("local_evaluation_inconclusive", assertNotNull(flagCalled["gated"])["\$feature_flag_error"]) + } + } + + private fun continuityDefinitions(): String = + createLocalEvaluationResponseFrom( + conclusiveFlagDefinition("beta-ui"), + conclusiveFlagDefinition("checkout", ensureExperienceContinuity = true), + ) + + private fun Map.flagErrors(): List = (this["\$feature_flag_error"] as? String)?.split(",").orEmpty() + + @Test + fun `local only evaluation lists an experience continuity flag as unresolved without reading it`() { + withLocalEvaluation(definitions = continuityDefinitions()) { postHog, dispatcher, mockServer -> + val snapshot = postHog.evaluateFlags("user-123", onlyEvaluateLocally = true) + + assertEquals(listOf("beta-ui"), snapshot.keys) + assertEquals(true, snapshot.flags["beta-ui"]?.enabled) + assertEquals(mapOf("checkout" to PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), snapshot.unresolvedFlags) + assertTrue(snapshot.onlyAccessed().keys.isEmpty(), "reading unresolvedFlags is not an access") + val filtered = snapshot.only("beta-ui", "checkout") + assertEquals(listOf("beta-ui"), filtered.keys) + assertTrue(filtered.unresolvedFlags.isEmpty()) + postHog.flush() + + assertEquals(0, dispatcher.flagsCalls.get()) + assertTrue(drainRequests(mockServer).featureFlagCalledEvents().isEmpty()) + } + } + + @Test + fun `reading an unresolved flag reports an inconclusive local evaluation`() { + withLocalEvaluation(definitions = continuityDefinitions()) { postHog, _, mockServer -> + val snapshot = postHog.evaluateFlags("user-123", onlyEvaluateLocally = true) + + assertFalse(snapshot.isEnabled("checkout")) + postHog.flush() + + val checkout = assertNotNull(drainRequests(mockServer).featureFlagCalledEvents().toMap()["checkout"]) + assertEquals(listOf("local_evaluation_inconclusive"), checkout.flagErrors()) + } + } + + @Test + fun `reading a flag the remote fallback resolved reports no inconclusive error`() { + withLocalEvaluation( + definitions = continuityDefinitions(), + flagsResponse = { jsonResponse(createMultipleFlagsResponse("checkout" to true)) }, + ) { postHog, _, mockServer -> + val snapshot = postHog.evaluateFlags("user-123") + + assertTrue(snapshot.unresolvedFlags.isEmpty()) + assertTrue(snapshot.isEnabled("checkout")) + postHog.flush() + + val checkout = assertNotNull(drainRequests(mockServer).featureFlagCalledEvents().toMap()["checkout"]) + assertEquals(true, checkout["\$feature_flag_response"]) + assertTrue(checkout.flagErrors().isEmpty()) + } + } + + @Test + fun `reading a flag the remote fallback did not resolve reports an inconclusive local evaluation`() { + val fallbacks = + listOf<() -> MockResponse>( + { MockResponse().setResponseCode(503).setBody("unavailable") }, + { jsonResponse(createEmptyFlagsResponse()) }, + ) + for (fallback in fallbacks) { + withLocalEvaluation(definitions = continuityDefinitions(), flagsResponse = fallback) { postHog, dispatcher, mockServer -> + val snapshot = postHog.evaluateFlags("user-123") + + assertFalse("checkout" in snapshot.keys) + assertEquals(mapOf("checkout" to PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), snapshot.unresolvedFlags) + assertFalse(snapshot.isEnabled("checkout")) + postHog.flush() + + assertEquals(1, dispatcher.flagsCalls.get()) + val errors = + assertNotNull(drainRequests(mockServer).featureFlagCalledEvents().toMap()["checkout"]).flagErrors() + assertTrue("local_evaluation_inconclusive" in errors, "errors: $errors") + assertFalse("flag_missing" in errors, "errors: $errors") + } + } + } + + @Test + fun `reading a key without a local definition reports a missing flag`() { + withLocalEvaluation( + definitions = createLocalEvaluationResponseFrom(conclusiveFlagDefinition("beta-ui")), + ) { postHog, _, mockServer -> + val snapshot = postHog.evaluateFlags("user-123", flagKeys = listOf("beta-ui", "typo-flag")) + + assertFalse(snapshot.isEnabled("typo-flag")) + assertTrue(snapshot.unresolvedFlags.isEmpty()) + postHog.flush() + + val typo = assertNotNull(drainRequests(mockServer).featureFlagCalledEvents().toMap()["typo-flag"]) + assertEquals(listOf("flag_missing"), typo.flagErrors()) + } + } + @Test fun `a flags outage leaves locally-resolved flags on and is not retried within the cache window`() { withLocalEvaluation( diff --git a/posthog-server/src/test/java/com/posthog/server/PostHogFeatureFlagEvaluationsTest.kt b/posthog-server/src/test/java/com/posthog/server/PostHogFeatureFlagEvaluationsTest.kt index 467ae476c..c74f7b81c 100644 --- a/posthog-server/src/test/java/com/posthog/server/PostHogFeatureFlagEvaluationsTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/PostHogFeatureFlagEvaluationsTest.kt @@ -74,6 +74,7 @@ internal class PostHogFeatureFlagEvaluationsTest { definitionsLoadedAt: Long? = null, responseError: String? = null, groups: Map? = null, + unresolvedFlags: Map = emptyMap(), ) = PostHogFeatureFlagEvaluations( distinctId = distinctId, flagMap = flags, @@ -84,6 +85,7 @@ internal class PostHogFeatureFlagEvaluationsTest { responseError = responseError, host = host, groups = groups, + unresolvedFlags = unresolvedFlags, ) @Test @@ -129,6 +131,42 @@ internal class PostHogFeatureFlagEvaluationsTest { ) } + @Test + fun `reading an unresolved flag reports local_evaluation_inconclusive alongside response errors`() { + val host = FakeHost() + val snapshot = + snapshot( + host = host, + responseError = "errors_while_computing_flags", + unresolvedFlags = mapOf("checkout" to PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), + ) + + assertFalse(snapshot.isEnabled("checkout")) + + assertEquals( + "errors_while_computing_flags,local_evaluation_inconclusive", + host.captures.single().properties["\$feature_flag_error"], + ) + } + + @Test + fun `filtered snapshots do not carry unresolved flags`() { + val host = FakeHost() + val snapshot = + snapshot( + host = host, + flags = mapOf("known" to flag("known")), + unresolvedFlags = mapOf("checkout" to PostHogUnresolvedFlagReason.MISSING_CONTEXT), + ) + + val filtered = snapshot.only("known", "checkout") + filtered.isEnabled("checkout") + + assertTrue(filtered.unresolvedFlags.isEmpty()) + assertEquals(mapOf("checkout" to PostHogUnresolvedFlagReason.MISSING_CONTEXT), snapshot.unresolvedFlags) + assertEquals("flag_missing", host.captures.single().properties["\$feature_flag_error"]) + } + @Test fun `isEnabled propagates groups to the host so dedup can include group context`() { val host = FakeHost() diff --git a/posthog-server/src/test/java/com/posthog/server/Utils.kt b/posthog-server/src/test/java/com/posthog/server/Utils.kt index 0cfb5a733..633e5666c 100644 --- a/posthog-server/src/test/java/com/posthog/server/Utils.kt +++ b/posthog-server/src/test/java/com/posthog/server/Utils.kt @@ -179,14 +179,23 @@ public fun RecordedRequest.parseBatch(): BatchRequest { public class TestLogger : PostHogLogger { public val logs: MutableList = java.util.concurrent.CopyOnWriteArrayList() + /** Messages logged through [logWarning], which shows even when debug is off. Also in [logs]. */ + public val warnings: MutableList = java.util.concurrent.CopyOnWriteArrayList() + override fun log(message: String) { logs.add(message) } + override fun logWarning(message: String) { + warnings.add(message) + logs.add(message) + } + override fun isEnabled(): Boolean = true public fun clear() { logs.clear() + warnings.clear() } public fun containsLog(substring: String): Boolean = logs.any { it.contains(substring) } diff --git a/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt b/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt index 7508223f8..cf09a6673 100644 --- a/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt @@ -8,6 +8,7 @@ import com.posthog.internal.PostHogDateProvider import com.posthog.internal.PropertyGroup import com.posthog.internal.PropertyOperator import com.posthog.internal.PropertyType +import com.posthog.server.PostHogUnresolvedFlagReason import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull @@ -3478,4 +3479,350 @@ internal class FlagEvaluatorTest { assertEquals(true, result) assertEquals(false, evaluationCache["base-flag"]) } + + private fun reasonFlag( + key: String, + groups: String, + continuity: Boolean = false, + ): FlagDefinition = + config.serializer.gson.fromJson( + """{"id": 1, "name": "$key", "key": "$key", "active": true, "ensure_experience_continuity": $continuity, + "filters": {"groups": [$groups]}, "version": 1}""", + FlagDefinition::class.java, + ) + + private fun reasonCondition(vararg properties: String): String = + """{"properties": [${properties.joinToString(",")}], "rollout_percentage": 100}""" + + private fun reasonDependency(vararg chain: String): String = + """{"key": "${chain.lastOrNull() ?: "x"}", "value": true, "operator": "flag_evaluates_to", "type": "flag", + "dependency_chain": [${chain.joinToString(",") { "\"$it\"" }}]}""" + + private fun personFilter( + key: String, + operator: String, + value: String, + ): String = """{"key": "$key", "value": $value, "operator": "$operator", "type": "person"}""" + + private val staticCohortFilter = """{"key": "id", "value": 99, "operator": "in", "type": "cohort"}""" + + @Test + internal fun testInconclusiveReasonSeparatesDefinitionFromCallerValues() { + data class Row( + val name: String, + val groups: String, + val properties: Map, + val reason: PostHogUnresolvedFlagReason, + val otherFlags: List = emptyList(), + ) + + val continuityFlag = reasonFlag("continuity-dep", reasonCondition(), continuity = true) + val contextFlag = reasonFlag("context-dep", reasonCondition(personFilter("plan", "exact", "\"pro\""))) + val rows = + listOf( + Row( + "caller float", + reasonCondition(personFilter("n", "exact", "\"1\"")), + mapOf("n" to 1.5), + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ), + Row( + "definition float", + reasonCondition(personFilter("n", "exact", "1.5")), + mapOf("n" to "x"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "caller and definition float", + reasonCondition(personFilter("n", "exact", "[1.5]")), + mapOf("n" to 2.5), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "caller date", + reasonCondition(personFilter("d", "is_date_before", "\"2024-01-01\"")), + mapOf("d" to "not-a-date"), + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ), + Row( + "definition date", + reasonCondition(personFilter("d", "is_date_before", "\"not-a-date\"")), + mapOf("d" to "2024-01-01"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "caller semver", + reasonCondition(personFilter("v", "semver_gt", "\"1.2.3\"")), + mapOf("v" to "abc"), + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ), + Row( + "definition semver", + reasonCondition(personFilter("v", "semver_gt", "\"abc\"")), + mapOf("v" to "1.2.3"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "caller and definition semver", + reasonCondition(personFilter("v", "semver_gt", "\"abc\"")), + mapOf("v" to "xyz"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "caller and definition wildcard", + reasonCondition(personFilter("v", "semver_wildcard", "\"1x.*\"")), + mapOf("v" to "xyz"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "caller tilde", + reasonCondition(personFilter("v", "semver_tilde", "\"1.2.3\"")), + mapOf("v" to "xyz"), + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ), + Row( + "unknown operator and missing property", + reasonCondition(personFilter("plan", "some_future_operator", "\"pro\"")), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "malformed date filter and missing property", + reasonCondition(personFilter("d", "is_date_after", "\"not-a-date\"")), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "malformed number filter", + reasonCondition(personFilter("n", "gt", "\"9px\"")), + mapOf("n" to 10), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "regex syntax local evaluation does not support", + reasonCondition(personFilter("email", "regex", "\"\\\\bacme\"")), + mapOf("email" to "a@acme.com"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "negated invalid regex", + reasonCondition("""{"key": "email", "value": "(", "operator": "regex", "type": "person", "negation": true}"""), + mapOf("email" to "a@acme.com"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "regex on a non-ASCII value", + reasonCondition(personFilter("name", "regex", "\"^a\"")), + mapOf("name" to "année"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "unloaded dependency", + reasonCondition(reasonDependency("gone")), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "circular dependency", + reasonCondition(reasonDependency()), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "dependency with experience continuity", + reasonCondition(reasonDependency("continuity-dep")), + emptyMap(), + PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + listOf(continuityFlag), + ), + Row( + "dependency that needs context", + reasonCondition(reasonDependency("context-dep")), + emptyMap(), + PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + listOf(contextFlag), + ), + Row( + "inconclusive dependency before an unloaded one in the chain", + reasonCondition(reasonDependency("continuity-dep", "gone")), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + listOf(continuityFlag), + ), + Row( + "missing property before an unresolved dependency", + reasonCondition(personFilter("plan", "exact", "\"pro\""), reasonDependency("continuity-dep")), + emptyMap(), + PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + listOf(continuityFlag), + ), + Row( + "missing property in one condition, static cohort in the next", + reasonCondition(personFilter("plan", "exact", "\"pro\"")) + "," + reasonCondition(staticCohortFilter), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "missing property and static cohort in one condition", + reasonCondition(personFilter("plan", "exact", "\"pro\""), staticCohortFilter), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + ) + for (row in rows) { + val flag = reasonFlag("f", row.groups) + val flagsByKey = (row.otherFlags + flag).associateBy { it.key } + val error = + kotlin.test.assertFailsWith(row.name) { + evaluator.matchFeatureFlagProperties(flag, "user-123", row.properties, emptyMap(), flagsByKey, mutableMapOf()) + } + assertEquals(row.name, row.reason, error.reason) + } + + val continuityError = + kotlin.test.assertFailsWith { + evaluator.matchFeatureFlagProperties(continuityFlag, "user-123", emptyMap()) + } + assertEquals(PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY, continuityError.reason) + } + + @Test + internal fun testPickUnresolvedReasonFollowsSpecPrecedence() { + val precedence = + listOf( + PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY, + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + PostHogUnresolvedFlagReason.MISSING_CONTEXT, + ) + for ((i, first) in precedence.withIndex()) { + assertEquals(first, pickUnresolvedReason(null, first)) + for ((j, second) in precedence.withIndex()) { + val expected = precedence[minOf(i, j)] + assertEquals("$first then $second", expected, pickUnresolvedReason(first, second)) + } + } + } + + @Test + internal fun testConditionWithAnUncheckableFilterAndAFailingFilterDoesNotMatch() { + val planIsFree = personFilter("plan", "exact", "\"free\"") + val missingEmail = personFilter("email", "icontains", "\"@acme.com\"") + val unknownOperator = personFilter("plan", "some_future_operator", "\"pro\"") + val properties = mapOf("plan" to "pro") + for (uncheckable in listOf(missingEmail, staticCohortFilter, unknownOperator, reasonDependency("gone"))) { + for (filters in listOf(arrayOf(uncheckable, planIsFree), arrayOf(planIsFree, uncheckable))) { + val flag = reasonFlag("f", reasonCondition(*filters)) + assertEquals( + filters.joinToString(), + false, + evaluator.matchFeatureFlagProperties(flag, "user-123", properties, emptyMap(), mapOf("f" to flag), mutableMapOf()), + ) + } + } + } + + @Test + internal fun testConditionWithAnUncheckableFilterLetsALaterConditionMatch() { + val missingEmail = personFilter("email", "icontains", "\"@acme.com\"") + val planIsPro = personFilter("plan", "exact", "\"pro\"") + for (uncheckable in listOf(missingEmail, staticCohortFilter)) { + val flag = reasonFlag("f", reasonCondition(uncheckable, planIsPro) + "," + reasonCondition(planIsPro)) + assertEquals( + uncheckable, + true, + evaluator.matchFeatureFlagProperties( + flag, + "user-123", + mapOf("plan" to "pro"), + emptyMap(), + mapOf("f" to flag), + mutableMapOf(), + ), + ) + } + } + + @Test + internal fun testInactiveDependencyWithExperienceContinuityResolvesToFalse() { + val dependency = + config.serializer.gson.fromJson( + """{"id": 1, "name": "dep", "key": "dep", "active": false, "ensure_experience_continuity": true, "version": 1, + "filters": {"groups": [${reasonCondition()}]}}""", + FlagDefinition::class.java, + ) + val flag = reasonFlag("f", reasonCondition(reasonDependency("dep"))) + + assertEquals( + false, + evaluator.matchFeatureFlagProperties( + flag, + "user-123", + emptyMap(), + emptyMap(), + mapOf("dep" to dependency, "f" to flag), + mutableMapOf(), + ), + ) + } + + @Test + internal fun testDefinitionUnresolvedReason() { + val cohorts = + mapOf( + "1" to + config.serializer.gson.fromJson( + """{"type": "AND", "values": [{"type": "AND", "values": [ + {"key": "plan", "value": "pro", "operator": "some_future_operator", "type": "person"}]}]}""", + PropertyGroup::class.java, + ), + "2" to + config.serializer.gson.fromJson( + """{"type": "OR", "values": [{"key": "plan", "value": "pro", "operator": "exact", "type": "person"}]}""", + PropertyGroup::class.java, + ), + ) + val dependency = reasonFlag("dependency", reasonCondition(), continuity = true) + val unknownGroupType = + config.serializer.gson.fromJson( + """{"id": 1, "name": "g", "key": "g", "active": true, "version": 1, + "filters": {"aggregation_group_type_index": 7, "groups": [${reasonCondition()}]}}""", + FlagDefinition::class.java, + ) + val inactive = + config.serializer.gson.fromJson( + """{"id": 1, "name": "i", "key": "i", "active": false, "ensure_experience_continuity": true, "version": 1, + "filters": {"groups": [${reasonCondition(staticCohortFilter)}]}}""", + FlagDefinition::class.java, + ) + val rows = + listOf( + reasonFlag("f", reasonCondition(), continuity = true) to PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY, + reasonFlag("f", reasonCondition(staticCohortFilter)) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + reasonFlag("f", reasonCondition(personFilter("plan", "some_future_operator", "\"pro\""))) to + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + reasonFlag("f", reasonCondition(personFilter("v", "semver_caret", "\"x.y\""))) to + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + reasonFlag("f", reasonCondition(personFilter("n", "lt", "\"ten\""))) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + reasonFlag("f", reasonCondition(reasonDependency("gone"))) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + reasonFlag("f", reasonCondition(reasonDependency())) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + reasonFlag("f", reasonCondition("""{"key": "id", "value": 1, "operator": "in", "type": "cohort"}""")) to + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + unknownGroupType to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + inactive to null, + reasonFlag("f", reasonCondition()) to null, + reasonFlag("f", reasonCondition(personFilter("plan", "exact", "\"pro\""))) to null, + reasonFlag("f", reasonCondition("""{"key": "id", "value": 2, "operator": "in", "type": "cohort"}""")) to null, + reasonFlag("f", reasonCondition(reasonDependency("dependency"))) to null, + reasonFlag("f", reasonCondition(personFilter("email", "regex", "\"(\""))) to null, + ) + for ((index, row) in rows.withIndex()) { + val (flag, reason) = row + assertEquals( + "row $index", + reason, + evaluator.definitionUnresolvedReason(flag, mapOf("dependency" to dependency, flag.key to flag), emptyMap(), cohorts), + ) + } + } } diff --git a/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt b/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt index 926b38344..81f0998ee 100644 --- a/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt @@ -4,6 +4,7 @@ import com.posthog.internal.PostHogApi import com.posthog.server.CountingDispatcher import com.posthog.server.PostHogBlockingFlagDefinitionCacheProvider import com.posthog.server.PostHogFlagDefinitionCacheProvider +import com.posthog.server.PostHogUnresolvedFlagReason import com.posthog.server.TestLogger import com.posthog.server.conclusiveFlagDefinition import com.posthog.server.createEmptyFlagsResponse @@ -19,6 +20,7 @@ import com.posthog.server.errorResponse import com.posthog.server.jsonResponse import com.posthog.server.jsonResponseWithEtag import com.posthog.server.notModifiedResponse +import com.posthog.server.throwingFlagDefinition import com.posthog.server.unGzip import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse @@ -4230,4 +4232,424 @@ internal class PostHogFeatureFlagsTest { } } } + + private fun unresolvedTestFlag( + key: String, + properties: String = "[]", + continuity: Boolean = false, + active: Boolean = true, + version: Int = 1, + rolloutPercentage: Int = 100, + ): String = + """ + { + "id": 1, + "name": "$key", + "key": "$key", + "active": $active, + "ensure_experience_continuity": $continuity, + "filters": {"groups": [{"properties": $properties, "rollout_percentage": $rolloutPercentage}]}, + "version": $version + } + """.trimIndent() + + private fun dependsOn(key: String): String = + """{"key": "$key", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["$key"]}""" + + private val planIsPro = """{"key": "plan", "value": "pro", "operator": "exact", "type": "person"}""" + private val staticCohort = """{"key": "id", "value": 99, "operator": "in", "type": "cohort"}""" + + private fun localFeatureFlags( + mockServer: MockWebServer, + logger: TestLogger = TestLogger(), + provider: PostHogFlagDefinitionCacheProvider? = null, + ): PostHogFeatureFlags { + val config = createTestConfig(logger, mockServer.url("/").toString()) + cleanup.add { mockServer.shutdown() } + return PostHogFeatureFlags( + config, + PostHogApi(config), + 60000, + 100, + localEvaluation = true, + personalApiKey = "test-personal-key", + pollerEnabled = false, + flagDefinitionCacheProvider = provider, + ).also { cleanup.add { it.shutDown() } } + } + + private fun PostHogFeatureFlags.evaluate( + onlyEvaluateLocally: Boolean, + flagKeys: List? = null, + personProperties: Map? = null, + ): EvaluateFlagsResult = + evaluateFlags( + distinctId = "user-123", + groups = null, + personProperties = personProperties, + groupProperties = null, + flagKeys = flagKeys, + onlyEvaluateLocally = onlyEvaluateLocally, + disableGeoip = false, + ) + + @Test + fun `local only evaluation reports an experience continuity flag as unresolved`() { + val mockServer = + createMockHttp( + jsonResponse( + createLocalEvaluationResponseFrom( + unresolvedTestFlag("beta-ui"), + unresolvedTestFlag("checkout", continuity = true), + ), + ), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = true) + + assertEquals(setOf("beta-ui"), result.flags.keys) + assertEquals(true, result.flags["beta-ui"]?.enabled) + assertEquals(mapOf("checkout" to PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), result.unresolvedFlags) + assertEquals(1, mockServer.requestCount, "only the definitions load is allowed") + } + + @Test + fun `each inconclusive cause reports its unresolved reason`() { + // The spec's device-id row does not apply: this SDK has no device-id bucketing. + data class Row( + val name: String, + val properties: String, + val personProperties: Map?, + val reason: PostHogUnresolvedFlagReason, + ) + val rows = + listOf( + Row("missing person property", "[$planIsPro]", emptyMap(), PostHogUnresolvedFlagReason.MISSING_CONTEXT), + Row( + "unknown operator", + """[{"key": "plan", "value": "pro", "operator": "some_future_operator", "type": "person"}]""", + mapOf("plan" to "enterprise"), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row("static cohort", "[$staticCohort]", null, PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION), + Row( + "static cohort and missing person property", + "[$staticCohort, $planIsPro]", + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "missing person property and static cohort", + "[$planIsPro, $staticCohort]", + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + ) + + for (row in rows) { + val mockServer = + createMockHttp(jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", row.properties)))) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = true, personProperties = row.personProperties) + + assertNull(result.flags["checkout"], row.name) + assertEquals(mapOf("checkout" to row.reason), result.unresolvedFlags, row.name) + } + } + + @Test + fun `a flag blocked by an unresolved dependency reports the dependency`() { + val mockServer = + createMockHttp( + jsonResponse( + createLocalEvaluationResponseFrom( + unresolvedTestFlag("checkout", continuity = true), + unresolvedTestFlag("beta-ui", "[${dependsOn("checkout")}]"), + ), + ), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = true, flagKeys = listOf("beta-ui")) + + assertNull(result.flags["beta-ui"]) + assertEquals(mapOf("beta-ui" to PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY), result.unresolvedFlags) + } + + @Test + fun `a flag that depends on a flag with no loaded definition reports an unsupported definition`() { + val mockServer = + createMockHttp( + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("beta-ui", "[${dependsOn("checkout")}]"))), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = true, flagKeys = listOf("beta-ui")) + + assertNull(result.flags["beta-ui"]) + assertEquals(mapOf("beta-ui" to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION), result.unresolvedFlags) + } + + @Test + fun `a flag resolved by remote fallback is not unresolved`() { + val mockServer = + createMockHttp( + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true))), + jsonResponse(createFlagsResponse("checkout", enabled = true)), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = false) + + assertEquals(true, result.flags["checkout"]?.enabled) + assertTrue(result.unresolvedFlags.isEmpty()) + } + + @Test + fun `a remote fallback that does not resolve the flag leaves it unresolved`() { + val definitions = createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true)) + for (fallback in listOf(errorResponse(500), jsonResponse(createEmptyFlagsResponse()))) { + val mockServer = createMockHttp(jsonResponse(definitions), fallback) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = false) + + assertNull(result.flags["checkout"]) + assertEquals(mapOf("checkout" to PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), result.unresolvedFlags) + assertEquals(2, mockServer.requestCount, "the fallback was attempted") + } + } + + @Test + fun `a key without a local definition is missing, not unresolved`() { + val mockServer = + createMockHttp( + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("beta-ui"))), + jsonResponse(createEmptyFlagsResponse()), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = false, flagKeys = listOf("beta-ui", "typo-flag")) + + assertEquals(true, result.flags["beta-ui"]?.enabled) + assertTrue(result.unresolvedFlags.isEmpty()) + } + + @Test + fun `a flag outside the requested keys is not unresolved`() { + val mockServer = + createMockHttp( + jsonResponse( + createLocalEvaluationResponseFrom( + unresolvedTestFlag("beta-ui"), + unresolvedTestFlag("checkout", continuity = true), + ), + ), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = false, flagKeys = listOf("beta-ui")) + + assertEquals(setOf("beta-ui"), result.flags.keys) + assertTrue(result.unresolvedFlags.isEmpty()) + assertEquals(1, mockServer.requestCount, "no remote feature flag evaluation request") + } + + @Test + fun `an inactive flag is resolved as false, not unresolved`() { + val mockServer = + createMockHttp( + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true, active = false))), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = true) + + assertEquals(false, result.flags["checkout"]?.enabled) + assertTrue(result.unresolvedFlags.isEmpty()) + } + + @Test + fun `an unexpected evaluation error leaves the flag missing, not unresolved`() { + val mockServer = + createMockHttp( + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("beta-ui"), throwingFlagDefinition("broken"))), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = true) + + assertEquals(setOf("beta-ui"), result.flags.keys) + assertTrue(result.unresolvedFlags.isEmpty()) + } + + @Test + fun `a condition with an uncheckable filter and a filter that does not match resolves locally`() { + val planIsFree = """{"key": "plan", "value": "free", "operator": "exact", "type": "person"}""" + val email = """{"key": "email", "value": "@acme.com", "operator": "icontains", "type": "person"}""" + val mockServer = + createMockHttp( + jsonResponse( + createLocalEvaluationResponseFrom( + unresolvedTestFlag("static-cohort-first", "[$staticCohort, $planIsFree]"), + unresolvedTestFlag("missing-property-first", "[$email, $planIsFree]"), + unresolvedTestFlag("matching-filter", "[$email, $planIsPro]"), + ), + ), + jsonResponse(createFlagsResponse("matching-filter", enabled = true)), + ) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = false, personProperties = mapOf("plan" to "pro")) + + assertEquals(false, result.flags["static-cohort-first"]?.enabled) + assertEquals(false, result.flags["missing-property-first"]?.enabled) + assertEquals(true, result.locallyEvaluated["static-cohort-first"]) + assertEquals(true, result.locallyEvaluated["missing-property-first"]) + assertEquals(false, result.locallyEvaluated["matching-filter"], "the filters that match leave it inconclusive") + assertTrue(result.unresolvedFlags.isEmpty()) + } + + private fun TestLogger.unresolvableWarnings( + key: String, + reason: PostHogUnresolvedFlagReason, + ): Int = warnings.count { it.contains(key) && it.contains("(reason: ${reason.value})") } + + @Test + fun `loading a definition local evaluation never resolves logs one warning`() { + val definitions = createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true)) + val refreshes = + listOf( + "unchanged definitions" to listOf(jsonResponse(definitions), jsonResponse(definitions)), + "304 not modified" to listOf(jsonResponseWithEtag(definitions, "\"v1\""), notModifiedResponse("\"v1\"")), + ) + for ((name, responses) in refreshes) { + val logger = TestLogger() + val mockServer = createMockHttp(*responses.toTypedArray()) + val featureFlags = localFeatureFlags(mockServer, logger) + + featureFlags.loadFeatureFlagDefinitions() + featureFlags.loadFeatureFlagDefinitions() + + assertEquals(1, logger.unresolvableWarnings("checkout", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), name) + assertEquals(2, mockServer.requestCount, "$name: only the definitions loads") + } + } + + @Test + fun `a shared-cache load of the same definition does not repeat the warning`() { + val logger = TestLogger() + val mockServer = + createMockHttp(jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true)))) + val provider = TestFlagDefinitionCacheProvider(shouldFetch = true) + val featureFlags = localFeatureFlags(mockServer, logger, provider) + + featureFlags.loadFeatureFlagDefinitions() + provider.cacheData = roundTripFlagDefinitionCacheData(createTestConfig(), provider.cacheData) + provider.shouldFetch = false + featureFlags.loadFeatureFlagDefinitions() + featureFlags.loadFeatureFlagDefinitions() + + assertEquals(2, provider.getCalls, "both refreshes loaded from the shared cache") + assertEquals(1, logger.unresolvableWarnings("checkout", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY)) + assertEquals(1, mockServer.requestCount, "no remote feature flag evaluation request") + + val followerLogger = TestLogger() + val follower = localFeatureFlags(MockWebServer().apply { start() }, followerLogger, provider) + follower.loadFeatureFlagDefinitions() + follower.loadFeatureFlagDefinitions() + assertEquals(1, followerLogger.unresolvableWarnings("checkout", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY)) + } + + @Test + fun `a changed definition local evaluation never resolves logs the warning again`() { + val logger = TestLogger() + val mockServer = + createMockHttp( + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true))), + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true, version = 2))), + jsonResponse( + createLocalEvaluationResponseFrom( + unresolvedTestFlag("checkout", continuity = true, version = 2, rolloutPercentage = 50), + ), + ), + ) + val featureFlags = localFeatureFlags(mockServer, logger) + + featureFlags.loadFeatureFlagDefinitions() + featureFlags.loadFeatureFlagDefinitions() + assertEquals(2, logger.unresolvableWarnings("checkout", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY)) + + featureFlags.loadFeatureFlagDefinitions() + assertEquals( + 3, + logger.unresolvableWarnings("checkout", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), + "a changed definition warns again even when the version is the same", + ) + } + + @Test + fun `loading definitions local evaluation does not support logs one unsupported_definition warning`() { + val logger = TestLogger() + val definitions = + createLocalEvaluationResponseFrom( + unresolvedTestFlag("static-cohort", "[$staticCohort]"), + unresolvedTestFlag( + "unknown-operator", + """[{"key": "plan", "value": "pro", "operator": "some_future_operator", "type": "person"}]""", + ), + unresolvedTestFlag( + "malformed-value", + """[{"key": "version", "value": "not-a-version", "operator": "semver_gt", "type": "person"}]""", + ), + unresolvedTestFlag("missing-dependency", "[${dependsOn("gone")}]"), + unresolvedTestFlag( + "circular-dependency", + """[{"key": "x", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": []}]""", + ), + unresolvedTestFlag("continuity", continuity = true), + unresolvedTestFlag("needs-context", "[$planIsPro]"), + unresolvedTestFlag("needs-dependency", "[${dependsOn("continuity")}]"), + unresolvedTestFlag("disabled", "[$staticCohort]", active = false), + unresolvedTestFlag("resolvable"), + ) + val mockServer = createMockHttp(jsonResponse(definitions), jsonResponse(definitions)) + val featureFlags = localFeatureFlags(mockServer, logger) + + featureFlags.loadFeatureFlagDefinitions() + featureFlags.loadFeatureFlagDefinitions() + + val unsupported = "(reason: ${PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION.value}): " + val warning = logger.warnings.single { it.contains(unsupported) } + assertEquals( + setOf("static-cohort", "unknown-operator", "malformed-value", "missing-dependency", "circular-dependency"), + warning.substringAfter(unsupported).substringBefore(". ").split(", ").toSet(), + ) + assertEquals(1, logger.unresolvableWarnings("continuity", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY)) + assertEquals(2, logger.warnings.size) + } + + @Test + fun `a flag that is inactive for one load does not warn again, a removed flag does when it returns`() { + val logger = TestLogger() + val checkout = unresolvedTestFlag("checkout", continuity = true) + val mockServer = + createMockHttp( + jsonResponse(createLocalEvaluationResponseFrom(checkout)), + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("checkout", continuity = true, active = false))), + jsonResponse(createLocalEvaluationResponseFrom(checkout)), + jsonResponse(createLocalEvaluationResponseFrom(unresolvedTestFlag("beta-ui"))), + jsonResponse(createLocalEvaluationResponseFrom(checkout)), + ) + val featureFlags = localFeatureFlags(mockServer, logger) + + repeat(3) { featureFlags.loadFeatureFlagDefinitions() } + assertEquals(1, logger.unresolvableWarnings("checkout", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY)) + + // Removed keys are forgotten, so the warning history stays bounded by the loaded definitions. + repeat(2) { featureFlags.loadFeatureFlagDefinitions() } + assertEquals(2, logger.unresolvableWarnings("checkout", PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY)) + } } From 987b468824f9033916d9dc3a68d90953e309fefb Mon Sep 17 00:00:00 2001 From: "Gustavo H. Strassburger" Date: Wed, 28 Oct 2026 00:55:25 -0300 Subject: [PATCH 2/2] fix(server): address review findings for unresolved local flags --- .../posthog/server/internal/FlagEvaluator.kt | 129 ++++++++++-------- .../internal/InconclusiveMatchException.kt | 6 +- .../server/internal/PostHogFeatureFlags.kt | 24 +++- .../server/internal/FlagEvaluatorTest.kt | 105 +++++++++++++- .../internal/PostHogFeatureFlagsTest.kt | 28 ++++ 5 files changed, 227 insertions(+), 65 deletions(-) diff --git a/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt b/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt index d309439ae..5284a1d6c 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/FlagEvaluator.kt @@ -59,6 +59,14 @@ internal class FlagEvaluator( } } + /** + * Keys of the active flags whose top-level property filters all passed [assertFilterSupported] + * when definitions loaded, so evaluation does not check them again for each absent property. + * Empty until that check runs, so evaluation checks every filter until then. + */ + @Volatile + var flagsWithSupportedFilters: Set = emptySet() + private data class VariantLookupEntry( val key: String, val valueMin: Double, @@ -229,13 +237,7 @@ internal class FlagEvaluator( PropertyOperator.REGEX, PropertyOperator.NOT_REGEX -> { val matches = matchesRegex(propertyValue.toString(), overrideValue.toString()) if (matches == null) { - // A malformed cohort leaf must not become true when its result is negated. - if (inCohort || property.negation == true) { - throw InconclusiveMatchException( - "Cannot negate an invalid regex criterion locally", - PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, - ) - } + assertInvalidRegexNotNegated(property, inCohort) false } else if (propertyOperator == PropertyOperator.REGEX) { matches @@ -299,11 +301,8 @@ internal class FlagEvaluator( PropertyOperator.REGEX, PropertyOperator.NOT_REGEX -> { val pattern = value.toString() - if (pattern.isNotEmpty() && compileRegex(pattern) == null && (inCohort || property.negation == true)) { - throw InconclusiveMatchException( - "Cannot negate an invalid regex criterion locally", - PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, - ) + if (pattern.isNotEmpty() && compileRegex(pattern) == null) { + assertInvalidRegexNotNegated(property, inCohort) } } @@ -325,13 +324,54 @@ internal class FlagEvaluator( PropertyOperator.SEMVER_WILDCARD -> computeWildcardFilterBounds(value.toString()) - else -> throw InconclusiveMatchException( + // Listed by name, not `else`, so a new operator does not compile until this check handles it. + PropertyOperator.UNKNOWN, PropertyOperator.IN, PropertyOperator.FLAG_EVALUATES_TO -> throw InconclusiveMatchException( "Unknown operator: ${property.propertyOperator}", PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) } } + // A malformed cohort leaf must not become true when its result is negated. + private fun assertInvalidRegexNotNegated( + property: FlagProperty, + inCohort: Boolean, + ) { + if (inCohort || property.negation == true) { + throw InconclusiveMatchException( + "Cannot negate an invalid regex criterion locally", + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ) + } + } + + /** + * Throws an [PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION] error when a flag dependency + * filter is unusable whatever the chain evaluates to: no chain, a circular chain, or an operator + * other than `flag_evaluates_to`. + */ + private fun assertDependencySupported(property: FlagProperty) { + val chain = property.dependencyChain + val problem = + when { + chain == null -> "Flag dependency property for '${property.key}' is missing required 'dependency_chain' field" + chain.isEmpty() -> "Circular dependency detected for flag '${property.key}'" + property.propertyValue != null && + (property.propertyOperator ?: PropertyOperator.EXACT) != PropertyOperator.FLAG_EVALUATES_TO -> + "Flag dependency property for '${property.key}' has invalid operator '${property.propertyOperator}'" + else -> return + } + throw InconclusiveMatchException(problem, PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION) + } + + private inline fun throwsInconclusive(check: () -> Unit): Boolean = + try { + check() + false + } catch (e: InconclusiveMatchException) { + true + } + /** * The unresolved reason that comes from the flag's definition alone, so it is known when * definitions load: [PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY] or @@ -372,12 +412,9 @@ internal class FlagEvaluator( visitedCohortIds: MutableSet, ): Boolean = when (property.type) { - PropertyType.FLAG -> { - val dependencyChain = property.dependencyChain - dependencyChain.isNullOrEmpty() || - dependencyChain.any { it !in flagsByKey } || - (property.propertyValue != null && property.propertyOperator != PropertyOperator.FLAG_EVALUATES_TO) - } + PropertyType.FLAG -> + throwsInconclusive { assertDependencySupported(property) } || + property.dependencyChain.orEmpty().any { it !in flagsByKey } PropertyType.COHORT -> { val cohortId = property.propertyValue?.toString() @@ -391,13 +428,7 @@ internal class FlagEvaluator( } } - else -> - try { - assertFilterSupported(property, inCohort) - false - } catch (e: InconclusiveMatchException) { - true - } + else -> throwsInconclusive { assertFilterSupported(property, inCohort) } } private fun isUnsupportedPropertyGroup( @@ -1434,9 +1465,11 @@ internal class FlagEvaluator( else -> { if (!properties.containsKey(prop.key)) { - // Same result as matchProperty for an absent key, without the cost - // of throwing for every missing filter. - assertFilterSupported(prop, inCohort = false) + // Same result as matchProperty for an absent key. Filters that passed the + // load-time check skip it, since it can compile a regex or parse a date. + if (featureFlag.key !in flagsWithSupportedFilters) { + assertFilterSupported(prop, inCohort = false) + } inconclusiveReason = pickUnresolvedReason(inconclusiveReason, PostHogUnresolvedFlagReason.MISSING_CONTEXT) continue @@ -1447,6 +1480,12 @@ internal class FlagEvaluator( } catch (e: InconclusiveMatchException) { inconclusiveReason = pickUnresolvedReason(inconclusiveReason, e.reason) continue + } catch (e: Exception) { + // An earlier inconclusive filter already stops this condition, so an unexpected + // error in a later filter must not fail the whole flag. + if (inconclusiveReason == null) throw e + config.logger.log("Failed to compute property ${prop.key} locally: ${e.message}") + break } if (!matches) { @@ -1555,23 +1594,9 @@ internal class FlagEvaluator( properties: Map, cohortProperties: Map, ): Boolean { - // Check if dependency_chain is present - val dependencyChain = property.dependencyChain - if (dependencyChain == null) { - throw InconclusiveMatchException( - "Flag dependency property for '${property.key}' is missing required 'dependency_chain' field", - PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, - ) - } - - // Handle circular dependency (empty chain means circular) - if (dependencyChain.isEmpty()) { - config.logger.log("Circular dependency detected for flag: ${property.key}") - throw InconclusiveMatchException( - "Circular dependency detected for flag '${property.key}'", - PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, - ) - } + // A definition problem outranks whatever the chain evaluates to, so check it first. + assertDependencySupported(property) + val dependencyChain = property.dependencyChain.orEmpty() // A dependency with no loaded definition outranks one that is only inconclusive, wherever // it sits in the chain. @@ -1644,7 +1669,6 @@ internal class FlagEvaluator( // Now check if the final flag value matches the expected value in the property val flagKey = property.key val expectedValue = property.propertyValue - val propertyOperator = property.propertyOperator ?: PropertyOperator.EXACT if (expectedValue != null) { // Get the actual value of the flag we're checking @@ -1659,15 +1683,8 @@ internal class FlagEvaluator( ) } - // For flag dependencies, we need to compare the actual flag result with expected value - if (propertyOperator == PropertyOperator.FLAG_EVALUATES_TO) { - return matchesDependencyValue(expectedValue, actualValue) - } else { - throw InconclusiveMatchException( - "Flag dependency property for '${property.key}' has invalid operator '$propertyOperator'", - PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, - ) - } + // assertDependencySupported already rejected every operator but flag_evaluates_to. + return matchesDependencyValue(expectedValue, actualValue) } // If no value check needed, return True (all dependencies passed) diff --git a/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt b/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt index 603a99167..94246e4cb 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/InconclusiveMatchException.kt @@ -3,13 +3,15 @@ package com.posthog.server.internal import com.posthog.server.PostHogUnresolvedFlagReason /** - * Exception thrown when flag evaluation cannot be determined locally + * Exception thrown when flag evaluation cannot be determined locally. + * + * It is a control-flow signal that callers log by message only, so it skips the stack trace. */ internal class InconclusiveMatchException( message: String, val reason: PostHogUnresolvedFlagReason, cause: Throwable? = null, -) : Exception(message, cause) +) : Exception(message, cause, false, false) // When several causes apply to one flag, the spec reports the first reason in this order, so every // SDK picks the same reason whatever order it inspects the conditions and filters in. diff --git a/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt b/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt index b43395e8e..fdc19a2ec 100644 --- a/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt +++ b/posthog-server/src/main/java/com/posthog/server/internal/PostHogFeatureFlags.kt @@ -793,8 +793,9 @@ internal class PostHogFeatureFlags( cacheData, ) cache = PostHogFeatureFlagCache(maxSize = cacheMaxSize, maxAgeMs = cacheMaxAgeMs) + // Unchanged definitions cannot add a warning, so only a new snapshot is checked. + appliedSnapshot = definitionSnapshot } - appliedSnapshot = definitionSnapshot definitionsLoaded = true definitionsLoadedAt = System.currentTimeMillis() } @@ -806,7 +807,8 @@ internal class PostHogFeatureFlags( /** * Returns the flags whose definition local evaluation can never resolve and that were not - * warned about at their current definition, grouped by reason, and remembers them. + * warned about at their current definition, grouped by reason, and remembers them. Also tells + * the snapshot's evaluator which flags passed the check. * * Only keys absent from the loaded definitions are forgotten, which keeps the map bounded. A * flag that is inactive or resolvable for a while keeps its entry, so it warns again only when @@ -817,6 +819,7 @@ internal class PostHogFeatureFlags( val groupTypeMapping = snapshot.groupTypeMapping.orEmpty() val cohorts = snapshot.cohorts.orEmpty() val unresolvable = linkedMapOf>() + val flagsWithSupportedFilters = mutableSetOf() synchronized(warnedUnresolvableFlags) { warnedUnresolvableFlags.keys.retainAll(flagsByKey.keys) for ((key, flag) in flagsByKey) { @@ -825,14 +828,20 @@ internal class PostHogFeatureFlags( snapshot.evaluator.definitionUnresolvedReason(flag, flagsByKey, groupTypeMapping, cohorts) } catch (e: Exception) { config.logger.log("Could not check whether flag '$key' resolves locally: ${e.message}") - null - } ?: continue + continue + } + if (reason == null) { + // An inactive flag skips the check, so its filters are unknown. + if (flag.active) flagsWithSupportedFilters.add(key) + continue + } val fingerprint = definitionFingerprint(flag) if (warnedUnresolvableFlags.put(key, fingerprint) != fingerprint) { unresolvable.getOrPut(reason) { mutableListOf() }.add(key) } } } + snapshot.evaluator.flagsWithSupportedFilters = flagsWithSupportedFilters return unresolvable } @@ -1008,7 +1017,12 @@ internal class PostHogFeatureFlags( config.logger.log("Unknown group type index $aggregationGroupIndex for flag '$key'") throw InconclusiveMatchException( "Flag has unknown group type index", - PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + // Experience continuity outranks every other reason, as in the load-time warning. + if (flag.ensureExperienceContinuity) { + PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY + } else { + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION + }, ) } diff --git a/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt b/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt index cf09a6673..ac515c201 100644 --- a/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/internal/FlagEvaluatorTest.kt @@ -1550,7 +1550,7 @@ internal class FlagEvaluatorTest { evaluator.matchFeatureFlagProperties(flag, "user-123", properties, cohortProperties) assertTrue("Should have thrown InconclusiveMatchException", false) } catch (e: InconclusiveMatchException) { - // Expected + assertEquals(PostHogUnresolvedFlagReason.MISSING_CONTEXT, e.reason) } } @@ -3514,8 +3514,29 @@ internal class FlagEvaluatorTest { val properties: Map, val reason: PostHogUnresolvedFlagReason, val otherFlags: List = emptyList(), + val cohorts: Map = emptyMap(), ) + val unknownOperatorLeaf = personFilter("plan", "some_future_operator", "\"pro\"") + val missingEmailLeaf = personFilter("email", "exact", "\"a@acme.com\"") + val cohortFilter = """{"key": "id", "value": 5, "operator": "in", "type": "cohort"}""" + val flatCohort = + mapOf( + "5" to + config.serializer.gson.fromJson( + """{"type": "OR", "values": [$unknownOperatorLeaf, $missingEmailLeaf]}""", + PropertyGroup::class.java, + ), + ) + val nestedCohort = + mapOf( + "5" to + config.serializer.gson.fromJson( + """{"type": "OR", "values": [{"type": "OR", "values": [$unknownOperatorLeaf]}, + {"type": "OR", "values": [$missingEmailLeaf]}]}""", + PropertyGroup::class.java, + ), + ) val continuityFlag = reasonFlag("continuity-dep", reasonCondition(), continuity = true) val contextFlag = reasonFlag("context-dep", reasonCondition(personFilter("plan", "exact", "\"pro\""))) val rows = @@ -3668,13 +3689,50 @@ internal class FlagEvaluatorTest { emptyMap(), PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, ), + Row( + "static cohort in one condition, missing property in the next", + reasonCondition(staticCohortFilter) + "," + reasonCondition(personFilter("plan", "exact", "\"pro\"")), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), + Row( + "unresolved dependency referenced by two conditions", + reasonCondition(reasonDependency("continuity-dep")) + "," + reasonCondition(reasonDependency("continuity-dep")), + emptyMap(), + PostHogUnresolvedFlagReason.UNRESOLVED_DEPENDENCY, + listOf(continuityFlag), + ), + Row( + "dependency with an invalid operator on an unresolved flag", + reasonCondition( + """{"key": "continuity-dep", "value": true, "operator": "exact", "type": "flag", + "dependency_chain": ["continuity-dep"]}""", + ), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + listOf(continuityFlag), + ), + Row( + "cohort with an unknown operator leaf before a missing property leaf", + reasonCondition(cohortFilter), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + cohorts = flatCohort, + ), + Row( + "cohort with an unknown operator group before a missing property group", + reasonCondition(cohortFilter), + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + cohorts = nestedCohort, + ), ) for (row in rows) { val flag = reasonFlag("f", row.groups) val flagsByKey = (row.otherFlags + flag).associateBy { it.key } val error = kotlin.test.assertFailsWith(row.name) { - evaluator.matchFeatureFlagProperties(flag, "user-123", row.properties, emptyMap(), flagsByKey, mutableMapOf()) + evaluator.matchFeatureFlagProperties(flag, "user-123", row.properties, row.cohorts, flagsByKey, mutableMapOf()) } assertEquals(row.name, row.reason, error.reason) } @@ -3743,6 +3801,43 @@ internal class FlagEvaluatorTest { } } + @Test + internal fun testUnexpectedErrorAfterAnUncheckableFilterLetsALaterConditionMatch() { + val missingEmail = personFilter("email", "icontains", "\"@acme.com\"") + val planRegex = personFilter("plan", "regex", "\"^pro\"") + val unprintable = + object { + override fun toString(): String = throw IllegalStateException("unprintable") + } + val properties = mapOf("plan" to unprintable) + val catchAll = reasonCondition() + + val afterUncheckable = reasonFlag("f", reasonCondition(missingEmail, planRegex) + "," + catchAll) + assertEquals( + true, + evaluator.matchFeatureFlagProperties( + afterUncheckable, + "user-123", + properties, + emptyMap(), + mapOf("f" to afterUncheckable), + mutableMapOf(), + ), + ) + + val beforeUncheckable = reasonFlag("f", reasonCondition(planRegex, missingEmail) + "," + catchAll) + kotlin.test.assertFailsWith { + evaluator.matchFeatureFlagProperties( + beforeUncheckable, + "user-123", + properties, + emptyMap(), + mapOf("f" to beforeUncheckable), + mutableMapOf(), + ) + } + } + @Test internal fun testInactiveDependencyWithExperienceContinuityResolvesToFalse() { val dependency = @@ -3806,6 +3901,12 @@ internal class FlagEvaluatorTest { reasonFlag("f", reasonCondition(personFilter("n", "lt", "\"ten\""))) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, reasonFlag("f", reasonCondition(reasonDependency("gone"))) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, reasonFlag("f", reasonCondition(reasonDependency())) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + reasonFlag( + "f", + reasonCondition( + """{"key": "dependency", "value": true, "operator": "exact", "type": "flag", "dependency_chain": ["dependency"]}""", + ), + ) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, reasonFlag("f", reasonCondition("""{"key": "id", "value": 1, "operator": "in", "type": "cohort"}""")) to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, unknownGroupType to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, diff --git a/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt b/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt index 81f0998ee..56c67cb86 100644 --- a/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt +++ b/posthog-server/src/test/java/com/posthog/server/internal/PostHogFeatureFlagsTest.kt @@ -4332,6 +4332,12 @@ internal class PostHogFeatureFlagsTest { mapOf("plan" to "enterprise"), PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, ), + Row( + "unknown operator on a missing person property", + """[{"key": "plan", "value": "pro", "operator": "some_future_operator", "type": "person"}]""", + emptyMap(), + PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION, + ), Row("static cohort", "[$staticCohort]", null, PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION), Row( "static cohort and missing person property", @@ -4392,6 +4398,28 @@ internal class PostHogFeatureFlagsTest { assertEquals(mapOf("beta-ui" to PostHogUnresolvedFlagReason.UNSUPPORTED_DEFINITION), result.unresolvedFlags) } + @Test + fun `a group flag with experience continuity and an unknown group type reports experience continuity`() { + val flag = + """ + { + "id": 1, + "name": "org-feature", + "key": "org-feature", + "active": true, + "ensure_experience_continuity": true, + "filters": {"aggregation_group_type_index": 99, "groups": [{"properties": [], "rollout_percentage": 100}]}, + "version": 1 + } + """.trimIndent() + val mockServer = createMockHttp(jsonResponse(createLocalEvaluationResponseFrom(flag))) + val featureFlags = localFeatureFlags(mockServer) + + val result = featureFlags.evaluate(onlyEvaluateLocally = true) + + assertEquals(mapOf("org-feature" to PostHogUnresolvedFlagReason.EXPERIENCE_CONTINUITY), result.unresolvedFlags) + } + @Test fun `a flag resolved by remote fallback is not unresolved`() { val mockServer =