diff --git a/.changeset/unresolved-local-flags.md b/.changeset/unresolved-local-flags.md new file mode 100644 index 0000000..aa09845 --- /dev/null +++ b/.changeset/unresolved-local-flags.md @@ -0,0 +1,14 @@ +--- +"posthog-php": minor +--- + +Report the flags local evaluation could not resolve on `evaluateFlags()` snapshots. `FeatureFlagEvaluations::getUnresolvedFlags()` maps each such flag key to an `UnresolvedFlagReason` value: `experience_continuity`, `unsupported_definition`, `unresolved_dependency` or `missing_context`. + +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. An inconclusive flag dependency in one condition no longer stops a later condition from resolving the flag. So a flag that went to the server before can now resolve locally. A dependency that needs server data, such as a static cohort, still sends the flag to the server. +- An inactive flag with experience continuity enabled now resolves to `false` locally instead of going to the server. +- A flag that depends on a flag with experience continuity enabled no longer resolves locally. It goes to the server, or stays unresolved with `onlyEvaluateLocally`. +- A malformed date in a flag filter or a person property now makes its condition inconclusive, like other malformed values, instead of failing local evaluation of the whole flag. +- `$feature_flag_called` events from snapshot reads of unresolved flags report `local_evaluation_inconclusive` instead of `flag_missing`. +- When a fresh flag definitions response (HTTP 200) loads, 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 per client until its definition changes. Definitions from a cache provider or a 304 response do not log the warning. Without a `flag_definition_cache_provider`, each new client fetches fresh definitions, so under PHP-FPM the warning is logged on every request. Configure a cache provider to log it only when definitions are fetched. diff --git a/api/public-api.json b/api/public-api.json index 826ef5b..81b148b 100644 --- a/api/public-api.json +++ b/api/public-api.json @@ -2166,6 +2166,54 @@ } ] }, + "getDefinitionUnresolvedReason": { + "static": true, + "abstract": false, + "final": false, + "returnType": "?string", + "parameters": [ + { + "name": "flag", + "type": null, + "byReference": false, + "variadic": false, + "optional": false, + "default": null, + "defaultConstant": null, + "hasDefault": false + }, + { + "name": "flagsByKey", + "type": null, + "byReference": false, + "variadic": false, + "optional": false, + "default": null, + "defaultConstant": null, + "hasDefault": false + }, + { + "name": "cohorts", + "type": null, + "byReference": false, + "variadic": false, + "optional": false, + "default": null, + "defaultConstant": null, + "hasDefault": false + }, + { + "name": "groupTypeMapping", + "type": null, + "byReference": false, + "variadic": false, + "optional": false, + "default": null, + "defaultConstant": null, + "hasDefault": false + } + ] + }, "matchCohort": { "static": true, "abstract": false, @@ -2554,6 +2602,10 @@ "type": "string", "value": "flag_missing" }, + "LOCAL_EVALUATION_INCONCLUSIVE": { + "type": "string", + "value": "local_evaluation_inconclusive" + }, "QUOTA_LIMITED": { "type": "string", "value": "quota_limited" @@ -2704,6 +2756,16 @@ "default": null, "defaultConstant": null, "hasDefault": true + }, + { + "name": "unresolvedFlags", + "type": "array", + "byReference": false, + "variadic": false, + "optional": true, + "default": [], + "defaultConstant": null, + "hasDefault": true } ] }, @@ -2757,6 +2819,13 @@ "returnType": "array", "parameters": [] }, + "getUnresolvedFlags": { + "static": false, + "abstract": false, + "final": false, + "returnType": "array", + "parameters": [] + }, "isEnabled": { "static": false, "abstract": false, @@ -3405,12 +3474,67 @@ "constants": [], "properties": [], "methods": { + "__construct": { + "static": false, + "abstract": false, + "final": false, + "returnType": null, + "parameters": [ + { + "name": "message", + "type": "string", + "byReference": false, + "variadic": false, + "optional": true, + "default": "", + "defaultConstant": null, + "hasDefault": true + }, + { + "name": "code", + "type": "int", + "byReference": false, + "variadic": false, + "optional": true, + "default": 0, + "defaultConstant": null, + "hasDefault": true + }, + { + "name": "previous", + "type": "?Throwable", + "byReference": false, + "variadic": false, + "optional": true, + "default": null, + "defaultConstant": null, + "hasDefault": true + }, + { + "name": "reason", + "type": "string", + "byReference": false, + "variadic": false, + "optional": true, + "default": "unsupported_definition", + "defaultConstant": "PostHog\\UnresolvedFlagReason::UNSUPPORTED_DEFINITION", + "hasDefault": true + } + ] + }, "errorMessage": { "static": false, "abstract": false, "final": false, "returnType": null, "parameters": [] + }, + "getReason": { + "static": false, + "abstract": false, + "final": false, + "returnType": "string", + "parameters": [] } } }, @@ -4571,12 +4695,67 @@ "constants": [], "properties": [], "methods": { + "__construct": { + "static": false, + "abstract": false, + "final": false, + "returnType": null, + "parameters": [ + { + "name": "message", + "type": "string", + "byReference": false, + "variadic": false, + "optional": true, + "default": "", + "defaultConstant": null, + "hasDefault": true + }, + { + "name": "code", + "type": "int", + "byReference": false, + "variadic": false, + "optional": true, + "default": 0, + "defaultConstant": null, + "hasDefault": true + }, + { + "name": "previous", + "type": "?Throwable", + "byReference": false, + "variadic": false, + "optional": true, + "default": null, + "defaultConstant": null, + "hasDefault": true + }, + { + "name": "reason", + "type": "string", + "byReference": false, + "variadic": false, + "optional": true, + "default": "unsupported_definition", + "defaultConstant": "PostHog\\UnresolvedFlagReason::UNSUPPORTED_DEFINITION", + "hasDefault": true + } + ] + }, "errorMessage": { "static": false, "abstract": false, "final": false, "returnType": null, "parameters": [] + }, + "getReason": { + "static": false, + "abstract": false, + "final": false, + "returnType": "string", + "parameters": [] } } }, @@ -4726,6 +4905,34 @@ } } }, + "PostHog\\UnresolvedFlagReason": { + "type": "class", + "abstract": false, + "final": false, + "readonly": false, + "extends": null, + "implements": [], + "constants": { + "EXPERIENCE_CONTINUITY": { + "type": "string", + "value": "experience_continuity" + }, + "MISSING_CONTEXT": { + "type": "string", + "value": "missing_context" + }, + "UNRESOLVED_DEPENDENCY": { + "type": "string", + "value": "unresolved_dependency" + }, + "UNSUPPORTED_DEFINITION": { + "type": "string", + "value": "unsupported_definition" + } + }, + "properties": [], + "methods": [] + }, "PostHog\\Uuid": { "type": "class", "abstract": false, diff --git a/lib/Client.php b/lib/Client.php index ecc1ce0..a857ca3 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -243,6 +243,13 @@ class Client implements FeatureFlagEvaluationsHost */ private array $missingDistinctIdWarnings = []; + /** + * Definition fingerprint per flag key already warned about as never locally resolvable. + * + * @var array + */ + private array $warnedUnresolvableFlags = []; + /** * Create a new PostHog client with your project's API key. * @@ -1135,7 +1142,9 @@ public function getAllFlags( * FeatureFlagEvaluations snapshot. When distinctId is omitted, the current request context * distinctId is used if available. Reads on the snapshot do not trigger additional /flags * requests; access via isEnabled() or getFlag() fires a deduped $feature_flag_called event the - * first time each key is touched. + * first time each key is touched. Flags with a local definition that neither local evaluation nor + * the remote fallback resolved are absent from the snapshot; getUnresolvedFlags() lists them with + * a reason. * * @param string|null $distinctId Defaults to the current request context distinctId, when set. * @param array $groups Group identifiers for group-based flags. @@ -1189,6 +1198,7 @@ public function evaluateFlags( ); $records = []; + $unresolvedFlags = []; $requestId = null; $evaluatedAt = null; $errorsWhileComputing = false; @@ -1222,11 +1232,9 @@ public function evaluateFlags( $personProperties, $groupProperties ); - } catch (RequiresServerEvaluationException $e) { - $fallbackToRemote = true; - continue; - } catch (InconclusiveMatchException $e) { + } catch (RequiresServerEvaluationException | InconclusiveMatchException $e) { $fallbackToRemote = true; + $unresolvedFlags[$key] = $e->getReason(); continue; } catch (Exception $e) { $fallbackToRemote = true; @@ -1337,6 +1345,9 @@ public function evaluateFlags( } } + // A flag the remote fallback resolved has a value, so it is not unresolved. + $unresolvedFlags = array_diff_key($unresolvedFlags, $records); + return new FeatureFlagEvaluations( $distinctId, $records, @@ -1348,6 +1359,7 @@ public function evaluateFlags( $errorsWhileComputing, $quotaLimited, $disableGeoip, + $unresolvedFlags, ); } @@ -1478,14 +1490,17 @@ private function computeFlagLocally( // Create evaluation cache for flag dependencies $evaluationCache = []; - if ($featureFlag["ensure_experience_continuity"] ?? false) { - throw new InconclusiveMatchException("Flag has experience continuity enabled"); - } - if (!$featureFlag["active"]) { return false; } + if ($featureFlag["ensure_experience_continuity"] ?? false) { + throw new InconclusiveMatchException( + "Flag has experience continuity enabled", + reason: UnresolvedFlagReason::EXPERIENCE_CONTINUITY + ); + } + $flagFilters = $featureFlag["filters"] ?? []; $aggregationGroupTypeIndex = $flagFilters["aggregation_group_type_index"] ?? null; @@ -1493,7 +1508,10 @@ private function computeFlagLocally( $groupName = $definitionSnapshot['group_type_mapping'][strval($aggregationGroupTypeIndex)] ?? null; if (is_null($groupName)) { - throw new InconclusiveMatchException("Flag has unknown group type index"); + throw new InconclusiveMatchException( + "Flag has unknown group type index", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } if (!array_key_exists($groupName, $groups)) { @@ -1699,9 +1717,93 @@ private function fetchAndApplyFlagDefinitionsFromApi(): void $data = $this->normalizeFlagDefinitionData($payload); $this->applyFlagDefinitions($data); + // Only a fresh response warns: PHP builds a client per request, and a 304 or a shared-cache + // load carries definitions that a fresh response already warned about. + $this->warnAboutUnresolvableFlags(); $this->storeFlagDefinitionsInCacheProvider($data); } + /** + * Warn about flags whose definition local evaluation cannot resolve. Each flag is warned about + * once per client until its definition changes. + * + * @return void + */ + private function warnAboutUnresolvableFlags(): void + { + $flagKeysByReason = [ + UnresolvedFlagReason::EXPERIENCE_CONTINUITY => [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION => [], + ]; + // Rebuilt from the current definitions, so a flag that is deleted or becomes resolvable is + // forgotten and warns again if it later returns to an unresolvable definition. + $warnedFlags = []; + $uncheckedFlags = []; + foreach ($this->featureFlags as $flag) { + if (!is_array($flag) || !is_string($flag['key'] ?? null)) { + continue; + } + try { + $reason = FeatureFlag::getDefinitionUnresolvedReason( + $flag, + $this->featureFlagsByKey, + $this->cohorts, + $this->groupTypeMapping + ); + } catch (Throwable $throwable) { + // A malformed definition must not stop the load or the warning for the other flags. + $uncheckedFlags[] = $flag['key']; + continue; + } + if ($reason === null) { + continue; + } + $fingerprint = ($flag['version'] ?? '') . ':' . sha1(serialize($flag)) . ':' . $reason; + $warnedFlags[$flag['key']] = $fingerprint; + if (($this->warnedUnresolvableFlags[$flag['key']] ?? null) !== $fingerprint) { + $flagKeysByReason[$reason][] = $flag['key']; + } + } + $this->warnedUnresolvableFlags = $warnedFlags; + + $experienceContinuityFlags = $flagKeysByReason[UnresolvedFlagReason::EXPERIENCE_CONTINUITY]; + if (count($experienceContinuityFlags) > 0) { + $this->logWarning(sprintf( + 'You are using local evaluation but %d flag(s) have experience continuity enabled and cannot be ' + . 'resolved locally (reason: %s): %s. Experience continuity is incompatible with local evaluation ' + . 'and causes a server request on every evaluation of these flags. To evaluate them locally, ' + . 'disable experience continuity on these flags in PostHog; with onlyEvaluateLocally: true they ' + . 'are left unresolved.', + count($experienceContinuityFlags), + UnresolvedFlagReason::EXPERIENCE_CONTINUITY, + implode(', ', $experienceContinuityFlags) + )); + } + + $unsupportedDefinitionFlags = $flagKeysByReason[UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + if (count($unsupportedDefinitionFlags) > 0) { + $this->logWarning(sprintf( + 'You are using local evaluation but %d 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: %s): %s. Evaluating these flags falls back to a server request when ' + . 'evaluation reaches that part. To evaluate them locally, change these flags in PostHog or ' + . 'upgrade posthog-php.', + count($unsupportedDefinitionFlags), + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + implode(', ', $unsupportedDefinitionFlags) + )); + } + + if (count($uncheckedFlags) > 0) { + $this->logWarning(sprintf( + 'You are using local evaluation but the SDK could not check the definition of %d flag(s) ' + . 'because it is malformed: %s. Evaluating these flags can fall back to a server request.', + count($uncheckedFlags), + implode(', ', $uncheckedFlags) + )); + } + } + /** * Normalize API flag definition data using SDK defaults for optional fields. * diff --git a/lib/FeatureFlag.php b/lib/FeatureFlag.php index af4dc1a..7b36a37 100644 --- a/lib/FeatureFlag.php +++ b/lib/FeatureFlag.php @@ -13,6 +13,15 @@ class FeatureFlag { private const LONG_SCALE = 0xfffffffffffffff; + // 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 filters in. + private const UNRESOLVED_REASON_PRECEDENCE = [ + UnresolvedFlagReason::EXPERIENCE_CONTINUITY, + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, + UnresolvedFlagReason::MISSING_CONTEXT, + ]; + /** * Match a single property filter against provided property values. * @@ -23,13 +32,93 @@ class FeatureFlag * @throws InconclusiveMatchException When the property cannot be evaluated locally. */ public static function matchProperty($property, $propertyValues, $propertyMatchingVersion = 1) + { + try { + return FeatureFlag::matchPropertyValue($property, $propertyValues, $propertyMatchingVersion); + } catch (InconclusiveMatchException $e) { + // An unusable filter outranks missing context, so check the filter before blaming the context. + if ($e->getReason() === UnresolvedFlagReason::MISSING_CONTEXT) { + FeatureFlag::assertFilterSupported($property); + } + throw $e; + } + } + + /** + * Throw an unsupported_definition error when local evaluation cannot use the filter, whatever + * the context holds: an unknown operator, or a filter value that the operator cannot parse. + * + * @param array $property Feature flag property filter. + * @throws InconclusiveMatchException When local evaluation cannot use the filter. + */ + private static function assertFilterSupported($property): void + { + $operator = $property["operator"] ?? "exact"; + $value = $property["value"] ?? null; + + if ( + in_array($operator, [ + "exact", "is_not", "is_set", "is_not_set", "icontains", "not_icontains", + "starts_with", "not_starts_with", "ends_with", "not_ends_with", + ]) + ) { + return; + } + + if (in_array($operator, ["regex", "not_regex"])) { + $pattern = FeatureFlag::valueToString($value); + FeatureFlag::assertPortableRegex($pattern); + FeatureFlag::isRegularExpression(FeatureFlag::prepareValueForRegex($pattern)); + return; + } + + if (in_array($operator, ["gt", "gte", "lt", "lte"])) { + FeatureFlag::parseNumericFilter($value); + return; + } + + if (in_array($operator, ["is_date_before", "is_date_after"])) { + FeatureFlag::parseDateFilter($value); + return; + } + + if (in_array($operator, ["semver_eq", "semver_neq", "semver_gt", "semver_gte", "semver_lt", "semver_lte"])) { + FeatureFlag::parseSemver($value); + return; + } + + if ($operator === "semver_tilde") { + FeatureFlag::tildeBounds($value); + return; + } + + if ($operator === "semver_caret") { + FeatureFlag::caretBounds($value); + return; + } + + if ($operator === "semver_wildcard") { + FeatureFlag::wildcardBounds($value); + return; + } + + throw new InconclusiveMatchException( + "Unknown operator: " . $operator, + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + private static function matchPropertyValue($property, $propertyValues, $propertyMatchingVersion) { $key = $property["key"]; $operator = $property["operator"] ?? "exact"; $value = $property["value"]; if (!array_key_exists($key, $propertyValues)) { - throw new InconclusiveMatchException("Can't match properties without a given property value"); + throw new InconclusiveMatchException( + "Can't match properties without a given property value", + reason: UnresolvedFlagReason::MISSING_CONTEXT + ); } if ($operator == "is_not_set") { @@ -85,22 +174,25 @@ public static function matchProperty($property, $propertyValues, $propertyMatchi $input = FeatureFlag::valueToString($overrideValue); // Rust and PCRE disagree on word membership for join controls, marks, numbers and symbols. if ($usesWordClass && preg_match('/[^\x00-\x7f\p{L}\p{Nd}]/u', $input) === 1) { - throw new InconclusiveMatchException("Regex Unicode word class requires server evaluation"); + throw new InconclusiveMatchException( + "Regex Unicode word class requires server evaluation", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $matched = preg_match($regexValue, $input); if ($matched === false) { - throw new InconclusiveMatchException("Regex execution failed: " . preg_last_error_msg()); + throw new InconclusiveMatchException( + "Regex execution failed: " . preg_last_error_msg(), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } return ($matched === 1) === ($operator == "regex"); } if (in_array($operator, ["gt", "gte", "lt", "lte"])) { - $parsedValue = FeatureFlag::parseNumericValue($value); - if ($parsedValue === null) { - throw new InconclusiveMatchException("The number set on the flag is not a valid format"); - } + $parsedValue = FeatureFlag::parseNumericFilter($value); $parsedOverrideValue = FeatureFlag::parseNumericValue($overrideValue); if ($parsedOverrideValue === null) { @@ -111,17 +203,9 @@ public static function matchProperty($property, $propertyValues, $propertyMatchi } if (in_array($operator, ["is_date_before", "is_date_after"])) { - $parsedDate = FeatureFlag::relativeDateParseForFeatureFlagMatching($value); + $parsedDate = FeatureFlag::parseDateFilter($value); - if (is_null($parsedDate)) { - $parsedDate = FeatureFlag::convertToDateTime($value); - } - - if (is_null($parsedDate)) { - throw new InconclusiveMatchException("The date set on the flag is not a valid format"); - } - - $overrideDate = FeatureFlag::convertToDateTime($overrideValue); + $overrideDate = FeatureFlag::convertToDateTime($overrideValue, UnresolvedFlagReason::MISSING_CONTEXT); if ($operator == 'is_date_before') { return $overrideDate < $parsedDate; } else { @@ -129,10 +213,11 @@ public static function matchProperty($property, $propertyValues, $propertyMatchi } } - // Semver operators + // Semver operators. The filter is parsed before the context, so a bad filter reports + // unsupported_definition even when the context value is bad too. if (in_array($operator, ["semver_eq", "semver_neq", "semver_gt", "semver_gte", "semver_lt", "semver_lte"])) { - $overrideTuple = FeatureFlag::parseSemver($overrideValue); $valueTuple = FeatureFlag::parseSemver($value); + $overrideTuple = FeatureFlag::parseContextSemver($overrideValue); $comparison = FeatureFlag::compareSemverTuples($overrideTuple, $valueTuple); @@ -152,30 +237,33 @@ public static function matchProperty($property, $propertyValues, $propertyMatchi } if ($operator === "semver_tilde") { - $overrideTuple = FeatureFlag::parseSemver($overrideValue); list($lower, $upper) = FeatureFlag::tildeBounds($value); + $overrideTuple = FeatureFlag::parseContextSemver($overrideValue); return FeatureFlag::compareSemverTuples($overrideTuple, $lower) >= 0 && FeatureFlag::compareSemverTuples($overrideTuple, $upper) < 0; } if ($operator === "semver_caret") { - $overrideTuple = FeatureFlag::parseSemver($overrideValue); list($lower, $upper) = FeatureFlag::caretBounds($value); + $overrideTuple = FeatureFlag::parseContextSemver($overrideValue); return FeatureFlag::compareSemverTuples($overrideTuple, $lower) >= 0 && FeatureFlag::compareSemverTuples($overrideTuple, $upper) < 0; } if ($operator === "semver_wildcard") { - $overrideTuple = FeatureFlag::parseSemver($overrideValue); list($lower, $upper) = FeatureFlag::wildcardBounds($value); + $overrideTuple = FeatureFlag::parseContextSemver($overrideValue); return FeatureFlag::compareSemverTuples($overrideTuple, $lower) >= 0 && FeatureFlag::compareSemverTuples($overrideTuple, $upper) < 0; } - throw new InconclusiveMatchException("Unknown operator: " . $operator); + throw new InconclusiveMatchException( + "Unknown operator: " . $operator, + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } /** @@ -219,7 +307,7 @@ private static function matchCohortForIdentity( $propertyValues, $cohortProperties, $flagsByKey, - $evaluationCache, + &$evaluationCache, $distinctId, $propertyMatchingVersion, $identityAggregation @@ -286,7 +374,7 @@ private static function matchPropertyGroupForIdentity( $propertyValues, $cohortProperties, $flagsByKey, - $evaluationCache, + &$evaluationCache, $distinctId, $propertyMatchingVersion, $identityAggregation @@ -303,7 +391,7 @@ private static function matchPropertyGroupForIdentity( return true; } - $errorMatchingLocally = false; + $inconclusiveReason = null; if (array_key_exists("values", $properties[0])) { // a nested property group @@ -333,12 +421,15 @@ private static function matchPropertyGroupForIdentity( // Immediately propagate - this condition requires server-side data throw $err; } catch (InconclusiveMatchException $err) { - $errorMatchingLocally = true; + $inconclusiveReason = FeatureFlag::pickUnresolvedReason($inconclusiveReason, $err->getReason()); } } - if ($errorMatchingLocally) { - throw new InconclusiveMatchException("Can't match cohort without a given cohort property value"); + if ($inconclusiveReason !== null) { + throw new InconclusiveMatchException( + "Can't match cohort without a given cohort property value", + reason: $inconclusiveReason + ); } // if we get here, all matched in AND case, or none matched in OR case return $propertyGroupType === 'AND'; @@ -396,16 +487,15 @@ private static function matchPropertyGroupForIdentity( // Immediately propagate - this condition requires server-side data throw $err; } catch (InconclusiveMatchException $err) { - // If this is a flag dependency error, preserve the original message - if ($propType === 'flag') { - throw $err; - } - $errorMatchingLocally = true; + $inconclusiveReason = FeatureFlag::pickUnresolvedReason($inconclusiveReason, $err->getReason()); } } - if ($errorMatchingLocally) { - throw new InconclusiveMatchException("can't match cohort without a given cohort property value"); + if ($inconclusiveReason !== null) { + throw new InconclusiveMatchException( + "can't match cohort without a given cohort property value", + reason: $inconclusiveReason + ); } // if we get here, all matched in AND case, or none matched in OR case @@ -461,10 +551,16 @@ private static function parseSemverNumeric(string $part, string $component, $val { // Semver 2.0.0 ยง2: numeric identifiers MUST NOT include leading zeros. if ($part === "" || !ctype_digit($part)) { - throw new InconclusiveMatchException("Cannot parse semver: invalid {$component} version in {$value}"); + throw new InconclusiveMatchException( + "Cannot parse semver: invalid {$component} version in {$value}", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } if (strlen($part) > 1 && $part[0] === "0") { - throw new InconclusiveMatchException("Cannot parse semver: {$component} version has leading zero in {$value}"); + throw new InconclusiveMatchException( + "Cannot parse semver: {$component} version has leading zero in {$value}", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } return intval($part); } @@ -489,20 +585,36 @@ private static function parseSemverNumeric(string $part, string $component, $val public static function parseSemver($value): array { if ($value === null || $value === "") { - throw new InconclusiveMatchException("Cannot parse empty or null value as semver"); + throw new InconclusiveMatchException( + "Cannot parse empty or null value as semver", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + if (!is_scalar($value)) { + throw new InconclusiveMatchException( + "Cannot parse a non-scalar value as semver", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $text = trim(strval($value)); if ($text === "") { - throw new InconclusiveMatchException("Cannot parse empty value as semver"); + throw new InconclusiveMatchException( + "Cannot parse empty value as semver", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } // Strip v/V prefix $text = ltrim($text, "vV"); if ($text === "") { - throw new InconclusiveMatchException("Cannot parse semver: only prefix found"); + throw new InconclusiveMatchException( + "Cannot parse semver: only prefix found", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } // Strip pre-release and build metadata (split on - or +, take first part) @@ -510,7 +622,10 @@ public static function parseSemver($value): array // Check for leading dot if (str_starts_with($text, ".")) { - throw new InconclusiveMatchException("Cannot parse semver with leading dot: {$value}"); + throw new InconclusiveMatchException( + "Cannot parse semver with leading dot: {$value}", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } // Split on dots @@ -601,7 +716,17 @@ private static function caretBounds($value): array private static function wildcardBounds($value): array { if ($value === null || $value === "") { - throw new InconclusiveMatchException("Cannot parse empty or null value as semver wildcard"); + throw new InconclusiveMatchException( + "Cannot parse empty or null value as semver wildcard", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + if (!is_scalar($value)) { + throw new InconclusiveMatchException( + "Cannot parse a non-scalar value as semver wildcard", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $text = trim(strval($value)); @@ -618,7 +743,10 @@ private static function wildcardBounds($value): array } if (count($parts) === 0) { - throw new InconclusiveMatchException("Cannot parse semver wildcard: no version components found in {$value}"); + throw new InconclusiveMatchException( + "Cannot parse semver wildcard: no version components found in {$value}", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $major = FeatureFlag::parseSemverNumeric($parts[0], "major", $value); @@ -637,21 +765,67 @@ private static function wildcardBounds($value): array return [$lower, $upper]; } - private static function convertToDateTime($value) + private static function parseDateFilter($value): \DateTime + { + $parsedDate = is_string($value) ? FeatureFlag::relativeDateParseForFeatureFlagMatching($value) : null; + + return $parsedDate ?? FeatureFlag::convertToDateTime($value, UnresolvedFlagReason::UNSUPPORTED_DEFINITION); + } + + /** + * @param string $reason Reason for an unusable value: unsupported_definition for a filter value, + * missing_context for a value the evaluation call passed. + */ + private static function convertToDateTime($value, string $reason): \DateTime { if ($value instanceof \DateTime) { return $value; - } elseif (is_string($value)) { - try { - $date = new \DateTime($value); - if (!is_nan($date->getTimestamp())) { - return $date; - } - } catch (Exception $e) { - throw new InconclusiveMatchException("{$value} is in an invalid date format"); - } - } else { - throw new InconclusiveMatchException("The date provided {$value} must be a string or date object"); + } + + if (!is_string($value)) { + throw new InconclusiveMatchException( + "The date provided " . FeatureFlag::valueToString($value) . " must be a string or date object", + reason: $reason + ); + } + + try { + return new \DateTime($value); + } catch (\Exception $e) { + throw new InconclusiveMatchException( + "{$value} is in an invalid date format: " . $e->getMessage(), + 0, + $e, + $reason + ); + } + } + + private static function parseNumericFilter($value): float + { + $parsedValue = FeatureFlag::parseNumericValue($value); + if ($parsedValue === null) { + throw new InconclusiveMatchException( + "The number set on the flag is not a valid format", + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + return $parsedValue; + } + + /** + * Parse a semver value read from the evaluation call: failing to use it is missing context, + * not an unsupported definition. + * + * @return array{int, int, int} + */ + private static function parseContextSemver($value): array + { + try { + return FeatureFlag::parseSemver($value); + } catch (InconclusiveMatchException $e) { + throw new InconclusiveMatchException($e->getMessage(), 0, $e, UnresolvedFlagReason::MISSING_CONTEXT); } } @@ -1062,7 +1236,7 @@ private static function matchFeatureFlagPropertiesForIdentity( $properties, $cohorts, $flagsByKey, - $evaluationCache, + &$evaluationCache, $groups, $groupProperties, $groupTypeMapping, @@ -1080,7 +1254,8 @@ private static function matchFeatureFlagPropertiesForIdentity( $flagConditions = $flagFilters["groups"] ?? []; $flagAggregation = $flagFilters["aggregation_group_type_index"] ?? null; $earlyExitEnabled = $flagFilters["early_exit"] ?? false; - $isInconclusive = false; + $inconclusiveReason = null; + $groupEvaluationCaches = []; foreach ($flagConditions as $condition) { try { @@ -1095,6 +1270,7 @@ private static function matchFeatureFlagPropertiesForIdentity( $effectiveProperties = $properties; $effectiveBucketing = $distinctId; $effectiveAggregation = $identityAggregation; + $conditionEvaluationCache = &$evaluationCache; // Mixed-override path: condition-level aggregation differs from flag-level. // This assumes flag-level aggregation is null for mixed flags. @@ -1105,12 +1281,18 @@ private static function matchFeatureFlagPropertiesForIdentity( continue; } if (!array_key_exists($groupName, $groupProperties)) { - $isInconclusive = true; + $inconclusiveReason = FeatureFlag::pickUnresolvedReason( + $inconclusiveReason, + UnresolvedFlagReason::MISSING_CONTEXT + ); continue; } $effectiveProperties = $groupProperties[$groupName]; $effectiveBucketing = $groups[$groupName]; $effectiveAggregation = $conditionAggregation; + // Dependency results depend on the bucketing identity, so each group keeps its own. + $groupEvaluationCaches[$groupName] ??= is_null($evaluationCache) ? null : []; + $conditionEvaluationCache = &$groupEvaluationCaches[$groupName]; } } @@ -1121,7 +1303,7 @@ private static function matchFeatureFlagPropertiesForIdentity( $effectiveProperties, $cohorts, $flagsByKey, - $evaluationCache, + $conditionEvaluationCache, $propertyMatchingVersion, $effectiveAggregation ); @@ -1146,28 +1328,40 @@ private static function matchFeatureFlagPropertiesForIdentity( // evaluating (early exit still applies to later groups) and let the // post-loop inconclusive check fall back to server evaluation. A throw here // would be swallowed by the catch below and evaluation would wrongly continue. - if ($isInconclusive) { + if ($inconclusiveReason !== null) { break; } return false; } } catch (RequiresServerEvaluationException $e) { - // Immediately propagate - this condition requires server-side data - throw $e; - } catch (InconclusiveMatchException $e) { - // If this is a flag dependency error, preserve the original message - if ( - strpos($e->getMessage(), "Cannot evaluate flag dependency") !== false || - strpos($e->getMessage(), "Circular dependency detected") !== false - ) { - throw $e; + // Immediately propagate - this condition requires server-side data. Evaluation stops + // before it visits every filter of the flag, including those inside cohorts, so the + // definition of the whole flag still counts toward the reason. + $reason = FeatureFlag::pickUnresolvedReason($inconclusiveReason, $e->getReason()); + try { + $definitionReason = FeatureFlag::conditionsDefinitionReason( + $flagConditions, + $flagsByKey ?? [], + $cohorts ?? [] + ); + } catch (\Throwable $lookupError) { + // A malformed filter must not stop the server fallback, so keep the reason found so far. + $definitionReason = null; } - $isInconclusive = true; + if ($definitionReason !== null) { + $reason = FeatureFlag::pickUnresolvedReason($reason, $definitionReason); + } + throw new RequiresServerEvaluationException($e->getMessage(), 0, $e, $reason); + } catch (InconclusiveMatchException $e) { + $inconclusiveReason = FeatureFlag::pickUnresolvedReason($inconclusiveReason, $e->getReason()); } } - if ($isInconclusive) { - throw new InconclusiveMatchException("Can't determine if feature flag is enabled or not with given properties"); //phpcs:ignore + if ($inconclusiveReason !== null) { + throw new InconclusiveMatchException( + "Can't determine if feature flag is enabled or not with given properties", + reason: $inconclusiveReason + ); } return false; @@ -1180,40 +1374,64 @@ private static function isConditionMatch( $properties, $cohorts, $flagsByKey, - $evaluationCache, + &$evaluationCache, $propertyMatchingVersion, $identityAggregation ): ConditionMatch { $rolloutPercentage = array_key_exists("rollout_percentage", $condition) ? $condition["rollout_percentage"] : null; if (count($condition['properties'] ?? []) > 0) { + // 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. + $inconclusiveReason = null; + $serverEvaluationError = null; foreach ($condition['properties'] as $property) { - $matches = false; $propertyType = $property['type'] ?? null; - if ($propertyType == 'cohort') { - $matches = FeatureFlag::matchCohortForIdentity( - $property, - $properties, - $cohorts, - $flagsByKey, - $evaluationCache, - $distinctId, - $propertyMatchingVersion, - $identityAggregation - ); - } elseif ($propertyType == 'flag') { - $matches = FeatureFlag::evaluateFlagDependencyForIdentity( - $property, - $flagsByKey, - $evaluationCache, - $distinctId, - $properties, - $cohorts, - $propertyMatchingVersion, - $identityAggregation - ); - } else { - $matches = FeatureFlag::matchProperty($property, $properties, $propertyMatchingVersion); + try { + if ($propertyType == 'cohort') { + $matches = FeatureFlag::matchCohortForIdentity( + $property, + $properties, + $cohorts, + $flagsByKey, + $evaluationCache, + $distinctId, + $propertyMatchingVersion, + $identityAggregation + ); + } elseif ($propertyType == 'flag') { + $matches = FeatureFlag::evaluateFlagDependencyForIdentity( + $property, + $flagsByKey, + $evaluationCache, + $distinctId, + $properties, + $cohorts, + $propertyMatchingVersion, + $identityAggregation + ); + } else { + $matches = FeatureFlag::matchProperty($property, $properties, $propertyMatchingVersion); + } + } catch (RequiresServerEvaluationException $e) { + // Fall back to the server only when this is the first filter that could not be + // checked, as when the loop stopped there. After another inconclusive filter, + // later conditions must still be able to resolve the flag locally. + if ($inconclusiveReason === null) { + $serverEvaluationError = $e; + } + $inconclusiveReason = FeatureFlag::pickUnresolvedReason($inconclusiveReason, $e->getReason()); + continue; + } catch (InconclusiveMatchException $e) { + $inconclusiveReason = FeatureFlag::pickUnresolvedReason($inconclusiveReason, $e->getReason()); + continue; + } catch (\Throwable $e) { + // After an inconclusive filter the condition is already inconclusive, so an unexpected + // error must not fail the whole flag and stop later conditions from resolving it. + if ($inconclusiveReason === null) { + throw $e; + } + break; } if (!$matches) { @@ -1221,6 +1439,24 @@ private static function isConditionMatch( } } + // Rethrow the server error type: the caller stops at it and falls back to the server + // instead of trying later conditions. Later filters in this condition can still + // outrank its reason. + if ($serverEvaluationError !== null) { + throw new RequiresServerEvaluationException( + $serverEvaluationError->getMessage(), + 0, + $serverEvaluationError, + $inconclusiveReason + ); + } + if ($inconclusiveReason !== null) { + throw new InconclusiveMatchException( + sprintf("Can't determine if a condition of flag '%s' matches", $featureFlag["key"] ?? "unknown"), + reason: $inconclusiveReason + ); + } + if (is_null($rolloutPercentage)) { return ConditionMatch::Match; } @@ -1233,6 +1469,23 @@ private static function isConditionMatch( return ConditionMatch::Match; } + /** + * Pick the reason to report when two causes apply to one flag. + * + * @param string|null $current Reason found so far, if any. + * @param string $next Reason of the cause just found. + * @return string + */ + private static function pickUnresolvedReason(?string $current, string $next): string + { + if ($current === null) { + return $next; + } + + $precedence = array_flip(self::UNRESOLVED_REASON_PRECEDENCE); + return $precedence[$next] < $precedence[$current] ? $next : $current; + } + private static function isRegularExpression($string) { if ($string === null) { @@ -1246,7 +1499,10 @@ private static function isRegularExpression($string) restore_error_handler(); if (!$isRegularExpression && $error !== PREG_INTERNAL_ERROR) { // Preflight also executes the pattern; execution limits do not mean invalid syntax. - throw new InconclusiveMatchException("Regex execution failed: " . $errorMessage); + throw new InconclusiveMatchException( + "Regex execution failed: " . $errorMessage, + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } return $isRegularExpression; } @@ -1267,7 +1523,10 @@ private static function assertPortableRegex(string $pattern): bool } if ($escaped === 'x') { if (!preg_match('/^[0-9a-fA-F]{2}$/D', substr($pattern, $index + 1, 2))) { - throw new InconclusiveMatchException('Regex escape requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex escape requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $index += 2; } elseif ( @@ -1277,20 +1536,29 @@ private static function assertPortableRegex(string $pattern): bool && !preg_match('/^[\x20-\x2f:;=?@\x5b-\x60\x7b-\x7e]$/D', $escaped) ) { // In particular, word boundaries can exhaust the service's 10,000-step budget. - throw new InconclusiveMatchException('Regex escape requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex escape requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } if ( $inClass && !str_contains('dDsSwWbfnrtx\\^-[]', $escaped) ) { - throw new InconclusiveMatchException('Regex class escape requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex class escape requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } if ( $inClass && str_contains('dDsSwW', $escaped) && (($pattern[$index - 2] ?? '') === '-' || (($pattern[$index + 1] ?? '') === '-' && ($pattern[$index + 2] ?? '') !== ']')) ) { - throw new InconclusiveMatchException('Regex class range requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex class range requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } if ($escaped === 'w' || $escaped === 'W') { $usesWordClass = true; @@ -1302,20 +1570,29 @@ private static function assertPortableRegex(string $pattern): bool if ($character === '[') { // PCRE accepts POSIX/nested/set classes with semantics different from Rust's. if ($inClass) { - throw new InconclusiveMatchException('Regex nested class requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex nested class requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $inClass = true; $classStart = $index + (($pattern[$index + 1] ?? '') === '^' ? 2 : 1); } elseif ($character === ']' && $inClass) { if ($index === $classStart) { - throw new InconclusiveMatchException('Regex empty class requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex empty class requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $inClass = false; $previousToken = 'literal'; continue; } elseif ($inClass) { if (in_array(substr($pattern, $index, 2), ['&&', '--', '~~'], true)) { - throw new InconclusiveMatchException('Regex set operation requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex set operation requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } continue; } elseif ($character === '(' && ($pattern[$index + 1] ?? '') === '?') { @@ -1325,10 +1602,16 @@ private static function assertPortableRegex(string $pattern): bool $index += 3; continue; } else { - throw new InconclusiveMatchException('Regex group requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex group requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } } elseif ($character === '(' && ($pattern[$index + 1] ?? '') === '*') { - throw new InconclusiveMatchException('Regex verb requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex verb requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } elseif ($character === '{') { if ( !preg_match('/^\{(\d+)(?:,(\d*))?\}/', substr($pattern, $index), $repetition) @@ -1337,16 +1620,25 @@ private static function assertPortableRegex(string $pattern): bool || (isset($repetition[2]) && (float) $repetition[2] > 65535) ) { // PCRE's repetition count limit is smaller than the service's. - throw new InconclusiveMatchException('Regex repetition requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex repetition requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } } elseif ( str_contains('*+?}', $character) && in_array($pattern[$index + 1] ?? '', ['+', '{'], true) ) { - throw new InconclusiveMatchException('Regex stacked repetition requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex stacked repetition requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } if (str_contains('*+?{', $character) && in_array($previousToken, ['^', '$', 'empty group'], true)) { - throw new InconclusiveMatchException('Regex assertion repetition requires server evaluation'); + throw new InconclusiveMatchException( + 'Regex assertion repetition requires server evaluation', + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } $previousToken = $character === ')' && in_array($previousToken, ['(', 'empty group'], true) ? 'empty group' : $character; @@ -1404,7 +1696,7 @@ public static function evaluateFlagDependency( private static function evaluateFlagDependencyForIdentity( $property, $flagsByKey, - $evaluationCache, + &$evaluationCache, $distinctId, $properties, $cohortProperties, @@ -1412,95 +1704,90 @@ private static function evaluateFlagDependencyForIdentity( $identityAggregation ) { if ($flagsByKey === null || $evaluationCache === null) { - throw new InconclusiveMatchException(sprintf( - "Cannot evaluate flag dependency on '%s' without flags_by_key and evaluation_cache", - $property["key"] ?? "unknown" - )); - } - - // Check if dependency_chain is present - it should always be provided for flag dependencies - if (!array_key_exists("dependency_chain", $property)) { - throw new InconclusiveMatchException(sprintf( - "Cannot evaluate flag dependency on '%s' without dependency_chain", - $property["key"] ?? "unknown" - )); + throw new InconclusiveMatchException( + sprintf( + "Cannot evaluate flag dependency on '%s' without flags_by_key and evaluation_cache", + $property["key"] ?? "unknown" + ), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); } - $dependencyChain = $property["dependency_chain"]; - - // Handle circular dependency (empty chain means circular) - if (count($dependencyChain) === 0) { - throw new InconclusiveMatchException(sprintf( - "Circular dependency detected for flag '%s'", - $property["key"] ?? "unknown" - )); - } - - // The flag key to evaluate is in the "key" field - $depFlagKey = $property["key"] ?? null; - if (!$depFlagKey) { - throw new InconclusiveMatchException(sprintf( - "Flag dependency missing 'key' field: %s", - json_encode($property) - )); - } + // An unsupported dependency outranks one that is only inconclusive, so check it before + // evaluating any of it. + FeatureFlag::assertFlagDependencySupported($property, $flagsByKey, $evaluationCache); + $depFlagKey = $property["key"]; // Check if we've already evaluated this flag if (!array_key_exists($depFlagKey, $evaluationCache)) { - // Need to evaluate this dependency first - $depFlag = $flagsByKey[$depFlagKey] ?? null; - if (!$depFlag) { - // Missing flag dependency - cannot evaluate locally + $depFlag = $flagsByKey[$depFlagKey]; + // Same order as Client::computeFlagLocally: an inactive flag is false whatever its + // experience continuity setting. + if (!($depFlag["active"] ?? false)) { + $evaluationCache[$depFlagKey] = false; + } elseif ($depFlag["ensure_experience_continuity"] ?? false) { $evaluationCache[$depFlagKey] = null; - throw new InconclusiveMatchException(sprintf( - "Cannot evaluate flag dependency '%s' - flag not found in local flags", - $depFlagKey - )); + throw new InconclusiveMatchException( + sprintf( + "Cannot evaluate flag dependency '%s' - flag has experience continuity enabled", + $depFlagKey + ), + reason: UnresolvedFlagReason::UNRESOLVED_DEPENDENCY + ); } else { - // Check if the flag is active (same check as in Client::computeFlagLocally) - if (!($depFlag["active"] ?? false)) { - $evaluationCache[$depFlagKey] = false; - } else { - // Holdout bucketing uses the evaluated identity, which can't be resolved for a - // dependency aggregated differently, so defer to server evaluation. - $depFilters = $depFlag["filters"] ?? []; - if ( - $identityAggregation !== false && - FeatureFlag::hasActiveHoldout($depFilters["holdout"] ?? null) && - FeatureFlag::aggregationKey($depFilters["aggregation_group_type_index"] ?? null) !== - FeatureFlag::aggregationKey($identityAggregation) - ) { - throw new InconclusiveMatchException(sprintf( + // Holdout bucketing uses the evaluated identity, which can't be resolved for a + // dependency aggregated differently, so defer to server evaluation. + $depFilters = $depFlag["filters"] ?? []; + if ( + $identityAggregation !== false && + FeatureFlag::hasActiveHoldout($depFilters["holdout"] ?? null) && + FeatureFlag::aggregationKey($depFilters["aggregation_group_type_index"] ?? null) !== + FeatureFlag::aggregationKey($identityAggregation) + ) { + throw new InconclusiveMatchException( + sprintf( "Cannot evaluate flag dependency '%s' with a holdout aggregated differently from the dependent flag", //phpcs:ignore $depFlagKey - )); - } + ), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } - // Recursively evaluate the dependency - try { - $depResult = FeatureFlag::matchFeatureFlagPropertiesForIdentity( - $depFlag, - $distinctId, - $properties, - $cohortProperties, - $flagsByKey, - $evaluationCache, - [], - [], - [], - $propertyMatchingVersion, - $identityAggregation - ); - $evaluationCache[$depFlagKey] = $depResult; - } catch (InconclusiveMatchException $e) { - // If we can't evaluate a dependency, store null and propagate the error - $evaluationCache[$depFlagKey] = null; - throw new InconclusiveMatchException(sprintf( - "Cannot evaluate flag dependency '%s': %s", - $depFlagKey, - $e->getMessage() - )); - } + // Recursively evaluate the dependency + try { + $depResult = FeatureFlag::matchFeatureFlagPropertiesForIdentity( + $depFlag, + $distinctId, + $properties, + $cohortProperties, + $flagsByKey, + $evaluationCache, + [], + [], + [], + $propertyMatchingVersion, + $identityAggregation + ); + $evaluationCache[$depFlagKey] = $depResult; + } catch (RequiresServerEvaluationException $e) { + // Keep the server error type so the dependent flag falls back to the server instead + // of letting a later condition resolve it locally. Nothing is cached, so a later + // check of this dependency needs the server too. + throw new RequiresServerEvaluationException( + sprintf("Cannot evaluate flag dependency '%s': %s", $depFlagKey, $e->getMessage()), + 0, + $e, + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY + ); + } catch (InconclusiveMatchException $e) { + // If we can't evaluate a dependency, store null and propagate the error + $evaluationCache[$depFlagKey] = null; + throw new InconclusiveMatchException( + sprintf("Cannot evaluate flag dependency '%s': %s", $depFlagKey, $e->getMessage()), + 0, + $e, + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY + ); } } } @@ -1509,35 +1796,78 @@ private static function evaluateFlagDependencyForIdentity( $flagValue = $evaluationCache[$depFlagKey]; if ($flagValue === null) { // Previously inconclusive - raise error again - throw new InconclusiveMatchException(sprintf( - "Flag dependency '%s' was previously inconclusive", - $depFlagKey - )); + throw new InconclusiveMatchException( + sprintf("Flag dependency '%s' was previously inconclusive", $depFlagKey), + reason: UnresolvedFlagReason::UNRESOLVED_DEPENDENCY + ); } // Now check if the flag value matches the expected value in the property $expectedValue = $property["value"] ?? null; - $operator = $property["operator"] ?? "exact"; - if ($expectedValue !== null) { - // For flag dependencies, we need to compare the actual flag result with expected value - // using the flag_evaluates_to operator logic - if ($operator === "flag_evaluates_to") { - return FeatureFlag::matchesDependencyValue($expectedValue, $flagValue); - } else { - // This should never happen, but just to be defensive - throw new InconclusiveMatchException(sprintf( - "Flag dependency property for '%s' has invalid operator '%s'", - $depFlagKey, - $operator - )); - } + return FeatureFlag::matchesDependencyValue($expectedValue, $flagValue); } // If no value check needed, return true (all dependencies passed) return true; } + /** + * Throw unsupported_definition when local evaluation can never evaluate a flag dependency filter. + * + * @param array $evaluationCache Dependency results already known, keyed by flag key. + * @throws InconclusiveMatchException + */ + private static function assertFlagDependencySupported($property, $flagsByKey, array $evaluationCache): void + { + // Check if dependency_chain is present - it should always be provided for flag dependencies + if (!is_array($property["dependency_chain"] ?? null)) { + throw new InconclusiveMatchException( + sprintf( + "Cannot evaluate flag dependency on '%s' without dependency_chain", + $property["key"] ?? "unknown" + ), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + $dependencyChain = $property["dependency_chain"]; + + // Handle circular dependency (empty chain means circular) + if (count($dependencyChain) === 0) { + throw new InconclusiveMatchException( + sprintf("Circular dependency detected for flag '%s'", $property["key"] ?? "unknown"), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + // The flag key to evaluate is in the "key" field + $depFlagKey = $property["key"] ?? null; + if (!$depFlagKey) { + throw new InconclusiveMatchException( + sprintf("Flag dependency missing 'key' field: %s", json_encode($property)), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + $operator = $property["operator"] ?? "exact"; + if (($property["value"] ?? null) !== null && $operator !== "flag_evaluates_to") { + throw new InconclusiveMatchException( + sprintf("Flag dependency property for '%s' has invalid operator '%s'", $depFlagKey, $operator), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + + foreach (array_merge($dependencyChain, [$depFlagKey]) as $chainFlagKey) { + if (!array_key_exists($chainFlagKey, $evaluationCache) && empty($flagsByKey[$chainFlagKey])) { + throw new InconclusiveMatchException( + sprintf("Cannot evaluate flag dependency '%s' - flag not found in local flags", $chainFlagKey), + reason: UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ); + } + } + } + /** * Compare an expected flag dependency value with an evaluated flag value. * @@ -1566,4 +1896,117 @@ public static function matchesDependencyValue($expectedValue, $actualValue) // Default case return false; } + + /** + * The unresolved reason that comes from the flag's definition alone, so it is known when + * definitions load. Matches the reason local evaluation reports for the same cause. + * + * @param array $flag Feature flag definition. + * @param array> $flagsByKey Local feature flag definitions keyed by key. + * @param array $cohorts Local cohort definitions keyed by cohort ID. + * @param array $groupTypeMapping Mapping from group type index to group type name. + * @return string|null UnresolvedFlagReason::EXPERIENCE_CONTINUITY or + * UnresolvedFlagReason::UNSUPPORTED_DEFINITION, or null when the definition alone does not + * stop local evaluation. + */ + public static function getDefinitionUnresolvedReason($flag, $flagsByKey, $cohorts, $groupTypeMapping): ?string + { + if (!($flag["active"] ?? false)) { + return null; + } + + if ($flag["ensure_experience_continuity"] ?? false) { + return UnresolvedFlagReason::EXPERIENCE_CONTINUITY; + } + + $flagFilters = $flag["filters"] ?? []; + $aggregation = $flagFilters["aggregation_group_type_index"] ?? null; + if (!is_null($aggregation) && is_null($groupTypeMapping[strval($aggregation)] ?? null)) { + return UnresolvedFlagReason::UNSUPPORTED_DEFINITION; + } + + return FeatureFlag::conditionsDefinitionReason($flagFilters["groups"] ?? [], $flagsByKey, $cohorts); + } + + /** + * UnresolvedFlagReason::UNSUPPORTED_DEFINITION when local evaluation can never check a filter in + * one of the conditions, null otherwise. + * + * @param array> $conditions Release conditions of a feature flag. + */ + private static function conditionsDefinitionReason(array $conditions, $flagsByKey, $cohorts): ?string + { + foreach ($conditions as $condition) { + foreach ($condition["properties"] ?? [] as $property) { + $visitedCohortIds = []; + if (FeatureFlag::isUnsupportedProperty($property, $flagsByKey, $cohorts, $visitedCohortIds)) { + return UnresolvedFlagReason::UNSUPPORTED_DEFINITION; + } + } + } + + return null; + } + + /** + * @param array $visitedCohortIds + */ + private static function isUnsupportedProperty($property, $flagsByKey, $cohorts, array &$visitedCohortIds): bool + { + $propertyType = $property["type"] ?? null; + + if ($propertyType === "flag") { + try { + FeatureFlag::assertFlagDependencySupported($property, $flagsByKey, []); + return false; + } catch (InconclusiveMatchException $e) { + return true; + } + } + + if ($propertyType === "cohort") { + $cohortId = strval($property["value"] ?? ""); + if (!array_key_exists($cohortId, $cohorts)) { + return true; + } + if (isset($visitedCohortIds[$cohortId])) { + return false; + } + $visitedCohortIds[$cohortId] = true; + return FeatureFlag::isUnsupportedPropertyGroup( + $cohorts[$cohortId], + $flagsByKey, + $cohorts, + $visitedCohortIds + ); + } + + try { + FeatureFlag::assertFilterSupported($property); + return false; + } catch (InconclusiveMatchException $e) { + return true; + } + } + + /** + * @param array $visitedCohortIds + */ + private static function isUnsupportedPropertyGroup( + $propertyGroup, + $flagsByKey, + $cohorts, + array &$visitedCohortIds + ): bool { + foreach ($propertyGroup["values"] ?? [] as $value) { + $unsupported = is_array($value) && array_key_exists("values", $value) + ? FeatureFlag::isUnsupportedPropertyGroup($value, $flagsByKey, $cohorts, $visitedCohortIds) + : FeatureFlag::isUnsupportedProperty($value, $flagsByKey, $cohorts, $visitedCohortIds); + if ($unsupported) { + return true; + } + } + + return false; + } } diff --git a/lib/FeatureFlagError.php b/lib/FeatureFlagError.php index 215912e..a470ec5 100644 --- a/lib/FeatureFlagError.php +++ b/lib/FeatureFlagError.php @@ -19,6 +19,11 @@ class FeatureFlagError */ public const FLAG_MISSING = 'flag_missing'; + /** + * Flag has a loaded local definition but local evaluation could not resolve it + */ + public const LOCAL_EVALUATION_INCONCLUSIVE = 'local_evaluation_inconclusive'; + /** * Rate/quota limit exceeded */ diff --git a/lib/FeatureFlagEvaluations.php b/lib/FeatureFlagEvaluations.php index b69d493..5ac53e8 100644 --- a/lib/FeatureFlagEvaluations.php +++ b/lib/FeatureFlagEvaluations.php @@ -27,6 +27,8 @@ class FeatureFlagEvaluations * @param bool $errorsWhileComputing Whether the remote response reported computation errors. * @param bool $quotaLimited Whether the remote response was quota limited. * @param bool|null $disableGeoip Override GeoIP enrichment on access events; null uses the client setting. + * @param array $unresolvedFlags Flags local evaluation could not resolve, mapped to an + * UnresolvedFlagReason value. */ public function __construct( private readonly string $distinctId, @@ -39,6 +41,7 @@ public function __construct( private readonly bool $errorsWhileComputing = false, private readonly bool $quotaLimited = false, private readonly ?bool $disableGeoip = null, + private readonly array $unresolvedFlags = [], ) { $this->accessed = $accessed ?? []; } @@ -53,6 +56,21 @@ public function getKeys(): array return array_keys($this->flags); } + /** + * Flags that have a loaded local definition but that neither local evaluation nor a remote + * fallback resolved, mapped to the reason local evaluation was inconclusive (an + * UnresolvedFlagReason value). These flags are absent from getKeys(), and their accessors + * return the missing-flag results. Reading this does not record access or fire an event. + * + * Always empty on snapshots returned by only() and onlyAccessed(). + * + * @return array + */ + public function getUnresolvedFlags(): array + { + return $this->unresolvedFlags; + } + /** * Whether the flag is enabled for the snapshot's distinct id. Returns false for unknown keys. * @@ -214,7 +232,8 @@ private function recordAccess(string $key, ?EvaluatedFlagRecord $record): void } // Build a comma-joined $feature_flag_error matching the single-flag path's granularity: - // response-level errors combine with per-flag errors so consumers can filter by type. + // response-level errors combine with per-flag errors (flag-missing or + // local-evaluation-inconclusive) so consumers can filter by type. $errors = []; if ($this->errorsWhileComputing) { $errors[] = FeatureFlagError::ERRORS_WHILE_COMPUTING_FLAGS; @@ -223,7 +242,9 @@ private function recordAccess(string $key, ?EvaluatedFlagRecord $record): void $errors[] = FeatureFlagError::QUOTA_LIMITED; } if ($record === null) { - $errors[] = FeatureFlagError::FLAG_MISSING; + $errors[] = array_key_exists($key, $this->unresolvedFlags) + ? FeatureFlagError::LOCAL_EVALUATION_INCONCLUSIVE + : FeatureFlagError::FLAG_MISSING; } if (!empty($errors)) { $properties['$feature_flag_error'] = implode(',', $errors); @@ -246,6 +267,7 @@ private function cloneWith(array $flags): self { // Filtered views start with an empty access set so reads on the child don't propagate back // into the parent's view. PHP's value-copy semantics on arrays already give us isolation. + // Unresolved flags are left out on purpose: filtered views carry no unresolved entries. return new self( $this->distinctId, $flags, diff --git a/lib/InconclusiveMatchException.php b/lib/InconclusiveMatchException.php index cd27029..120f695 100644 --- a/lib/InconclusiveMatchException.php +++ b/lib/InconclusiveMatchException.php @@ -3,6 +3,7 @@ namespace PostHog; use Exception; +use Throwable; /** * Raised when a feature flag cannot be evaluated conclusively with local data. @@ -11,6 +12,31 @@ */ class InconclusiveMatchException extends Exception { + /** + * @param string $message Exception message. + * @param int $code Exception code. + * @param Throwable|null $previous Previous exception. + * @param string $reason Why local evaluation is inconclusive, one of the UnresolvedFlagReason values. + */ + public function __construct( + string $message = "", + int $code = 0, + ?Throwable $previous = null, + private readonly string $reason = UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ) { + parent::__construct($message, $code, $previous); + } + + /** + * Why local evaluation is inconclusive, one of the UnresolvedFlagReason values. + * + * @return string + */ + public function getReason(): string + { + return $this->reason; + } + /** * Format the exception as an HTML-ish error message. * diff --git a/lib/RequiresServerEvaluationException.php b/lib/RequiresServerEvaluationException.php index 3d5c3ab..ec40100 100644 --- a/lib/RequiresServerEvaluationException.php +++ b/lib/RequiresServerEvaluationException.php @@ -3,6 +3,7 @@ namespace PostHog; use Exception; +use Throwable; /** * Raised when feature flag evaluation requires server-side data. @@ -11,6 +12,32 @@ */ class RequiresServerEvaluationException extends Exception { + /** + * @param string $message Exception message. + * @param int $code Exception code. + * @param Throwable|null $previous Previous exception. + * @param string $reason Why local evaluation is inconclusive, one of the UnresolvedFlagReason values. + * Server-side data is never available locally, so by default the definition is unsupported. + */ + public function __construct( + string $message = "", + int $code = 0, + ?Throwable $previous = null, + private readonly string $reason = UnresolvedFlagReason::UNSUPPORTED_DEFINITION + ) { + parent::__construct($message, $code, $previous); + } + + /** + * Why local evaluation is inconclusive, one of the UnresolvedFlagReason values. + * + * @return string + */ + public function getReason(): string + { + return $this->reason; + } + /** * Format the exception as an HTML-ish error message. * diff --git a/lib/UnresolvedFlagReason.php b/lib/UnresolvedFlagReason.php new file mode 100644 index 0000000..b3077eb --- /dev/null +++ b/lib/UnresolvedFlagReason.php @@ -0,0 +1,35 @@ +http_client = new MockedHttpClient( 'app.posthog.com', flagEndpointResponse: $localEvaluationResponse, - flagsEndpointResponse: $flagsEndpointResponse + flagsEndpointResponse: $flagsEndpointResponse, + flagsEndpointResponseCode: $flagsEndpointResponseCode ); $this->client = new Client( self::FAKE_API_KEY, @@ -1157,4 +1160,427 @@ public function testIsFeatureEnabledDoesNotCascadeDeprecationWarnings(): void $this->assertCount(1, $messages); } + + private static function localFlag( + string $key, + array $properties = [], + bool $continuity = false, + bool $active = true + ): array { + return [ + 'id' => 1, + 'key' => $key, + 'active' => $active, + 'ensure_experience_continuity' => $continuity, + 'filters' => ['groups' => [['properties' => $properties, 'rollout_percentage' => 100]]], + ]; + } + + private static function localDefinitions(array ...$flags): array + { + return ['flags' => $flags, 'group_type_mapping' => [], 'cohorts' => []]; + } + + private static function planFilter(string $operator = 'exact', string $value = 'pro'): array + { + return ['key' => 'plan', 'type' => 'person', 'operator' => $operator, 'value' => $value]; + } + + private static function staticCohortFilter(): array + { + return ['key' => 'id', 'type' => 'cohort', 'value' => 999]; + } + + private static function dependsOnFilter(string $key): array + { + return [ + 'key' => $key, + 'type' => 'flag', + 'operator' => 'flag_evaluates_to', + 'value' => true, + 'dependency_chain' => [$key], + ]; + } + + private function makeLocalClient( + array $localEvaluationResponse, + array $flagsEndpointResponse = ['flags' => []], + int $flagsEndpointResponseCode = 200 + ): void { + $this->makeClient( + flagsEndpointResponse: $flagsEndpointResponse, + localEvaluationResponse: $localEvaluationResponse, + personalApiKey: 'test-personal-key', + flagsEndpointResponseCode: $flagsEndpointResponseCode + ); + } + + /** @return list> Properties of each delivered $feature_flag_called event for $key. */ + private function flagCalledEvents(string $key): array + { + $this->client->flush(); + $events = []; + foreach ($this->batchRequests() as $batch) { + foreach ($batch['batch'] as $event) { + if ($event['event'] === '$feature_flag_called' && $event['properties']['$feature_flag'] === $key) { + $events[] = $event['properties']; + } + } + } + return $events; + } + + public function testLocalOnlyEvaluationReportsExperienceContinuityFlagAsUnresolved(): void + { + $this->makeLocalClient(self::localDefinitions( + self::localFlag('beta-ui'), + self::localFlag('checkout', continuity: true), + )); + + $snapshot = $this->client->evaluateFlags('user-123', onlyEvaluateLocally: true); + + $this->assertSame(['beta-ui'], $snapshot->getKeys()); + $this->assertSame(['checkout' => UnresolvedFlagReason::EXPERIENCE_CONTINUITY], $snapshot->getUnresolvedFlags()); + $this->assertSame(0, $this->flagsRequestCount()); + $this->client->flush(); + $this->assertSame([], $this->batchRequests()); + $this->assertSame([], $snapshot->onlyAccessed()->getKeys()); + $this->assertSame([], $snapshot->onlyAccessed()->getUnresolvedFlags()); + $this->assertSame([], $snapshot->only(['beta-ui', 'checkout'])->getUnresolvedFlags()); + $this->assertSame(true, $snapshot->getFlag('beta-ui')); + } + + #[DataProvider('unresolvedReasonProvider')] + public function testEachInconclusiveCauseReportsItsUnresolvedReason( + array $properties, + array $personProperties, + string $reason + ): void { + $this->makeLocalClient(self::localDefinitions(self::localFlag('checkout', $properties))); + + $snapshot = $this->client->evaluateFlags( + 'user-123', + personProperties: $personProperties, + onlyEvaluateLocally: true + ); + + $this->assertNotContains('checkout', $snapshot->getKeys()); + $this->assertSame(['checkout' => $reason], $snapshot->getUnresolvedFlags()); + } + + public static function unresolvedReasonProvider(): iterable + { + // The device-bucketed row of the spec is not applicable: this SDK has no device id bucketing. + yield 'person property not passed' => [[self::planFilter()], [], UnresolvedFlagReason::MISSING_CONTEXT]; + yield 'unknown operator' => [ + [self::planFilter('not_a_real_operator')], + ['plan' => 'enterprise'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'static cohort' => [[self::staticCohortFilter()], [], UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + yield 'static cohort and person property not passed' => [ + [self::staticCohortFilter(), self::planFilter()], + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'person property not passed and static cohort' => [ + [self::planFilter(), self::staticCohortFilter()], + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'malformed semver person property' => [ + [self::planFilter('semver_gt', '1.2.3')], + ['plan' => 'not-a-version'], + UnresolvedFlagReason::MISSING_CONTEXT, + ]; + yield 'malformed semver filter' => [ + [self::planFilter('semver_gt', 'not-a-version')], + ['plan' => '1.2.3'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + } + + public function testFlagBlockedByUnresolvedDependencyReportsTheDependency(): void + { + $this->makeLocalClient(self::localDefinitions( + self::localFlag('checkout', continuity: true), + self::localFlag('beta-ui', [self::dependsOnFilter('checkout')]), + )); + + $snapshot = $this->client->evaluateFlags('user-123', onlyEvaluateLocally: true, flagKeys: ['beta-ui']); + + $this->assertSame([], $snapshot->getKeys()); + $this->assertSame(['beta-ui' => UnresolvedFlagReason::UNRESOLVED_DEPENDENCY], $snapshot->getUnresolvedFlags()); + } + + public function testFlagDependingOnFlagWithNoLoadedDefinitionReportsUnsupportedDefinition(): void + { + $this->makeLocalClient(self::localDefinitions( + self::localFlag('beta-ui', [self::dependsOnFilter('checkout')]), + )); + + $snapshot = $this->client->evaluateFlags('user-123', onlyEvaluateLocally: true, flagKeys: ['beta-ui']); + + $this->assertSame([], $snapshot->getKeys()); + $this->assertSame(['beta-ui' => UnresolvedFlagReason::UNSUPPORTED_DEFINITION], $snapshot->getUnresolvedFlags()); + } + + /** A flag whose first condition depends on `dep` and whose second condition matches everyone by default. */ + private static function flagWithDependencyAndCatchAll(array $laterConditionFilters = []): array + { + return [ + 'id' => 2, + 'key' => 'target', + 'active' => true, + 'filters' => [ + 'multivariate' => ['variants' => [ + ['key' => 'a', 'rollout_percentage' => 50], + ['key' => 'b', 'rollout_percentage' => 50], + ]], + 'groups' => [ + ['properties' => [self::dependsOnFilter('dep')], 'rollout_percentage' => 100, 'variant' => 'a'], + ['properties' => $laterConditionFilters, 'rollout_percentage' => 100, 'variant' => 'b'], + ], + ], + ]; + } + + #[DataProvider('dependencyThatNeedsServerProvider')] + public function testFlagWithDependencyThatNeedsServerFallsBackToServer(?array $flagKeys): void + { + $this->makeLocalClient( + self::localDefinitions( + self::localFlag('dep', [self::staticCohortFilter()]), + self::flagWithDependencyAndCatchAll(), + ), + ['flags' => ['target' => ['key' => 'target', 'enabled' => true, 'variant' => 'a']]] + ); + + $snapshot = $this->client->evaluateFlags('user-123', flagKeys: $flagKeys); + + $this->assertSame(1, $this->flagsRequestCount()); + $this->assertSame('a', $snapshot->getFlag('target')); + $this->assertArrayNotHasKey('target', $snapshot->getUnresolvedFlags()); + } + + public static function dependencyThatNeedsServerProvider(): iterable + { + yield 'flag requested alone' => [['target']]; + yield 'all flags requested' => [null]; + } + + public function testFlagWithDependencyThatNeedsServerFallsBackToServerDespiteAMalformedLaterCondition(): void + { + global $errorMessages; + $malformedDependency = array_merge(self::dependsOnFilter('dep'), ['dependency_chain' => [['dep']]]); + $this->makeLocalClient( + self::localDefinitions( + self::localFlag('dep', [self::staticCohortFilter()]), + self::flagWithDependencyAndCatchAll([$malformedDependency]), + self::localFlag('everyone'), + ), + ['flags' => ['target' => ['key' => 'target', 'enabled' => true, 'variant' => 'a']]] + ); + $loadMessages = $errorMessages; + $errorMessages = []; + + $snapshot = $this->client->evaluateFlags('user-123', flagKeys: ['target', 'everyone']); + + $this->assertSame( + [1, 'a', true, []], + [ + $this->flagsRequestCount(), + $snapshot->getFlag('target'), + $snapshot->getFlag('everyone'), + $errorMessages, + ] + ); + $this->assertCount(1, array_filter( + $loadMessages, + fn (string $message) => str_contains($message, 'could not check the definition') && + str_contains($message, 'target') + )); + } + + public function testFlagWithDependencyThatNeedsServerIsUnresolvedLocally(): void + { + $this->makeLocalClient(self::localDefinitions( + self::localFlag('dep', [self::staticCohortFilter()]), + self::flagWithDependencyAndCatchAll(), + )); + + $snapshot = $this->client->evaluateFlags('user-123', onlyEvaluateLocally: true, flagKeys: ['target']); + + $this->assertSame([], $snapshot->getKeys()); + $this->assertSame(['target' => UnresolvedFlagReason::UNRESOLVED_DEPENDENCY], $snapshot->getUnresolvedFlags()); + } + + public function testFlagWithLocallyResolvedDependencyMatchesTheDependencyCondition(): void + { + $this->makeLocalClient(self::localDefinitions( + self::localFlag('dep', [self::planFilter()]), + self::flagWithDependencyAndCatchAll(), + )); + + $snapshot = $this->client->evaluateFlags('user-123', personProperties: ['plan' => 'pro'], flagKeys: ['target']); + + $this->assertSame(0, $this->flagsRequestCount()); + $this->assertSame('a', $snapshot->getFlag('target')); + } + + public function testFlagResolvedByRemoteFallbackIsNotUnresolved(): void + { + $this->makeLocalClient( + self::localDefinitions(self::localFlag('checkout', continuity: true)), + ['flags' => ['checkout' => ['key' => 'checkout', 'enabled' => true, 'variant' => null]]] + ); + + $snapshot = $this->client->evaluateFlags('user-123'); + + $this->assertSame(['checkout'], $snapshot->getKeys()); + $this->assertSame([], $snapshot->getUnresolvedFlags()); + $this->assertTrue($snapshot->isEnabled('checkout')); + + $events = $this->flagCalledEvents('checkout'); + $this->assertCount(1, $events); + $this->assertTrue($events[0]['$feature_flag_response']); + $this->assertArrayNotHasKey('$feature_flag_error', $events[0]); + } + + #[DataProvider('unresolvingFallbackProvider')] + public function testRemoteFallbackThatDoesNotResolveTheFlagLeavesItUnresolved( + array $flagsEndpointResponse, + int $flagsEndpointResponseCode + ): void { + $this->makeLocalClient( + self::localDefinitions(self::localFlag('checkout', continuity: true)), + $flagsEndpointResponse, + $flagsEndpointResponseCode + ); + + $snapshot = $this->client->evaluateFlags('user-123'); + + $this->assertSame(1, $this->flagsRequestCount()); + $this->assertSame([], $snapshot->getKeys()); + $this->assertSame(['checkout' => UnresolvedFlagReason::EXPERIENCE_CONTINUITY], $snapshot->getUnresolvedFlags()); + $this->assertFalse($snapshot->isEnabled('checkout')); + + $events = $this->flagCalledEvents('checkout'); + $this->assertCount(1, $events); + $this->assertSame('local_evaluation_inconclusive', $events[0]['$feature_flag_error']); + } + + public static function unresolvingFallbackProvider(): iterable + { + yield 'request fails' => [['error' => 'unavailable'], 500]; + yield 'response has no flags' => [['flags' => []], 200]; + yield 'flag failed remotely' => [ + ['flags' => ['checkout' => ['key' => 'checkout', 'enabled' => false, 'failed' => true]]], + 200, + ]; + } + + public function testKeyWithoutLocalDefinitionIsMissingNotUnresolved(): void + { + $this->makeLocalClient(self::localDefinitions(self::localFlag('beta-ui'))); + + $snapshot = $this->client->evaluateFlags('user-123', flagKeys: ['beta-ui', 'typo-flag']); + $snapshot->isEnabled('typo-flag'); + + $this->assertSame([], $snapshot->getUnresolvedFlags()); + $events = $this->flagCalledEvents('typo-flag'); + $this->assertCount(1, $events); + $this->assertSame('flag_missing', $events[0]['$feature_flag_error']); + } + + public function testFlagOutsideRequestedKeysIsNotUnresolved(): void + { + $this->makeLocalClient(self::localDefinitions( + self::localFlag('beta-ui'), + self::localFlag('checkout', continuity: true), + )); + + $snapshot = $this->client->evaluateFlags('user-123', flagKeys: ['beta-ui']); + + $this->assertSame(['beta-ui'], $snapshot->getKeys()); + $this->assertTrue($snapshot->isEnabled('beta-ui')); + $this->assertSame([], $snapshot->getUnresolvedFlags()); + $this->assertSame(0, $this->flagsRequestCount()); + } + + public function testInactiveFlagIsResolvedAsFalseNotUnresolved(): void + { + $this->makeLocalClient(self::localDefinitions( + self::localFlag('checkout', continuity: true, active: false), + )); + + $snapshot = $this->client->evaluateFlags('user-123', onlyEvaluateLocally: true); + + $this->assertSame(['checkout'], $snapshot->getKeys()); + $this->assertFalse($snapshot->getFlag('checkout')); + $this->assertSame([], $snapshot->getUnresolvedFlags()); + } + + public function testReadingUnresolvedFlagReportsInconclusiveLocalEvaluation(): void + { + $this->makeLocalClient(self::localDefinitions(self::localFlag('checkout', continuity: true))); + + $snapshot = $this->client->evaluateFlags('user-123', onlyEvaluateLocally: true); + + $this->assertFalse($snapshot->isEnabled('checkout')); + $this->assertNull($snapshot->getFlag('checkout')); + $this->assertSame(['checkout' => UnresolvedFlagReason::EXPERIENCE_CONTINUITY], $snapshot->getUnresolvedFlags()); + $events = $this->flagCalledEvents('checkout'); + $this->assertCount(1, $events); + $this->assertNull($events[0]['$feature_flag_response']); + $this->assertSame('local_evaluation_inconclusive', $events[0]['$feature_flag_error']); + } + + public function testInconclusiveErrorCombinesWithResponseLevelErrors(): void + { + $this->makeLocalClient( + self::localDefinitions(self::localFlag('checkout', continuity: true)), + ['flags' => [], 'errorsWhileComputingFlags' => true] + ); + + $this->client->evaluateFlags('user-123')->isEnabled('checkout'); + + $events = $this->flagCalledEvents('checkout'); + $this->assertCount(1, $events); + $this->assertSame( + 'errors_while_computing_flags,local_evaluation_inconclusive', + $events[0]['$feature_flag_error'] + ); + } + + public function testFilteredSnapshotReadOfUnresolvedFlagReportsFlagMissing(): void + { + $this->makeLocalClient(self::localDefinitions(self::localFlag('checkout', continuity: true))); + + $snapshot = $this->client->evaluateFlags('user-123', onlyEvaluateLocally: true); + $snapshot->only(['checkout'])->isEnabled('checkout'); + + $events = $this->flagCalledEvents('checkout'); + $this->assertCount(1, $events); + $this->assertSame('flag_missing', $events[0]['$feature_flag_error']); + } + + public function testUnexpectedLocalEvaluationErrorIsNotUnresolved(): void + { + $this->makeLocalClient(self::localDefinitions(self::localFlag('checkout', [self::planFilter()]))); + + // A property value that cannot be encoded as JSON fails outside the inconclusive paths. + $snapshot = $this->client->evaluateFlags( + 'user-123', + personProperties: ['plan' => ["\xB1"]], + onlyEvaluateLocally: true + ); + $snapshot->isEnabled('checkout'); + + $this->assertSame([], $snapshot->getKeys()); + $this->assertSame([], $snapshot->getUnresolvedFlags()); + $events = $this->flagCalledEvents('checkout'); + $this->assertCount(1, $events); + $this->assertSame('flag_missing', $events[0]['$feature_flag_error']); + } } diff --git a/test/FeatureFlagLocalEvaluationTest.php b/test/FeatureFlagLocalEvaluationTest.php index 839a41e..e24a7c7 100644 --- a/test/FeatureFlagLocalEvaluationTest.php +++ b/test/FeatureFlagLocalEvaluationTest.php @@ -13,6 +13,8 @@ use PostHog\PostHog; use PostHog\Test\Assets\MockedResponses; use PostHog\InconclusiveMatchException; +use PostHog\RequiresServerEvaluationException; +use PostHog\UnresolvedFlagReason; use PostHog\SizeLimitedHash; class FeatureFlagLocalEvaluationTest extends TestCase @@ -68,6 +70,13 @@ public function checkEmptyErrorLogs(): void $this->assertTrue(empty($errorMessages), "Error logs are not empty: " . implode("\n", $errorMessages)); } + private function checkOnlyUnsupportedDefinitionWarning(string $flagKey): void + { + global $errorMessages; + $this->assertCount(1, $errorMessages, "Error logs: " . implode("\n", $errorMessages)); + $this->assertStringContainsString("(reason: unsupported_definition): {$flagKey}.", $errorMessages[0]); + } + private function assertHttpCallsEqual(array $expectedCalls): void { $this->assertEquals( @@ -1963,7 +1972,7 @@ public function testFlagFallbackToDecide() $this->assertEquals(PostHog::getFeatureFlag('feature-1', 'some-distinct'), 'decide-fallback-value'); $this->assertEquals(PostHog::getFeatureFlag('feature-2', 'some-distinct'), 'decide-fallback-value'); - $this->checkEmptyErrorLogs(); + $this->checkOnlyUnsupportedDefinitionWarning('feature-1'); } public function testFlagFallbackToDecideWithFalseFlag() @@ -1990,7 +1999,7 @@ public function testFlagFallbackToDecideWithFalseFlag() array_column($this->http_client->calls, 'path') ); - $this->checkEmptyErrorLogs(); + $this->checkOnlyUnsupportedDefinitionWarning('feature-1'); } public function testFeatureFlagDefaultsComeIntoPlayOnlyWhenDecideErrorsOut() @@ -4645,7 +4654,7 @@ public function testFeatureFlagsWithFlagDependencies(): void try { FeatureFlag::matchPropertyGroup($propertyGroup, $properties, []); } catch (InconclusiveMatchException $e) { - $this->assertStringContainsString("Cannot evaluate flag dependency on 'parent-flag' without flags_by_key and evaluation_cache", $e->getMessage()); + $this->assertSame(UnresolvedFlagReason::UNSUPPORTED_DEFINITION, $e->getReason()); $threwException = true; } $this->assertTrue($threwException, "Expected InconclusiveMatchException was not thrown"); @@ -4673,7 +4682,7 @@ public function testFeatureFlagsWithFlagDependencies(): void try { FeatureFlag::matchFeatureFlagProperties($flag, "test-user", $properties); } catch (InconclusiveMatchException $e) { - $this->assertStringContainsString("Cannot evaluate flag dependency", $e->getMessage()); + $this->assertSame(UnresolvedFlagReason::UNSUPPORTED_DEFINITION, $e->getReason()); $threwException = true; } $this->assertTrue($threwException, "Expected InconclusiveMatchException was not thrown"); @@ -4706,7 +4715,7 @@ public function testFallsBackToAPIWhenFlagHasStaticCohort() // Should return 'set-1' from API, not 'set-8' from local evaluation $this->assertEquals('set-1', $result); - $this->checkEmptyErrorLogs(); + $this->checkOnlyUnsupportedDefinitionWarning('multi-condition-flag'); } public function testFallsBackToAPIInGetAllFlagsWhenFlagHasStaticCohort() @@ -4737,7 +4746,7 @@ public function testFallsBackToAPIInGetAllFlagsWhenFlagHasStaticCohort() 'multi-condition-flag' => 'set-1' ], $result); - $this->checkEmptyErrorLogs(); + $this->checkOnlyUnsupportedDefinitionWarning('multi-condition-flag'); } public function testFallsBackToAPIInGetFeatureFlagPayloadWhenFlagHasStaticCohort() @@ -4767,7 +4776,7 @@ public function testFallsBackToAPIInGetFeatureFlagPayloadWhenFlagHasStaticCohort 'message' => 'from-api' ], $result); - $this->checkEmptyErrorLogs(); + $this->checkOnlyUnsupportedDefinitionWarning('flag-with-payload'); } // ==================== Semver Operator Tests ==================== @@ -5462,4 +5471,565 @@ public function testEarlyExitOutOfRolloutBoundAfterInconclusiveConditionThrows() $this->expectException(InconclusiveMatchException::class); FeatureFlag::matchFeatureFlagProperties($flag, "test-user", ["region" => "us"]); } + + // ==================== Unresolved Reason Tests ==================== + + private static function reasonOf(callable $evaluate): ?string + { + try { + $evaluate(); + } catch (InconclusiveMatchException | RequiresServerEvaluationException $e) { + return $e->getReason(); + } + return null; + } + + private static function filter(string $operator, mixed $value, string $key = 'key'): array + { + return ['key' => $key, 'operator' => $operator, 'value' => $value]; + } + + private static function flagWithConditions(array ...$conditions): array + { + return [ + 'key' => 'reason-flag', + 'active' => true, + 'filters' => [ + 'groups' => array_map( + fn($properties) => ['properties' => $properties, 'rollout_percentage' => 100], + $conditions + ), + ], + ]; + } + + private static function staticCohortFilter(): array + { + return ['key' => 'id', 'type' => 'cohort', 'value' => 999]; + } + + public function testInconclusiveExceptionsDefaultToUnsupportedDefinition(): void + { + $this->assertSame( + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + (new InconclusiveMatchException('message'))->getReason() + ); + $this->assertSame( + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + (new RequiresServerEvaluationException('message'))->getReason() + ); + $this->assertSame( + UnresolvedFlagReason::MISSING_CONTEXT, + (new InconclusiveMatchException('message', reason: UnresolvedFlagReason::MISSING_CONTEXT))->getReason() + ); + } + + #[DataProvider('matchPropertyReasonProvider')] + public function testMatchPropertyReportsUnresolvedReason(array $property, array $propertyValues, string $reason): void + { + $this->assertSame($reason, self::reasonOf(fn() => FeatureFlag::matchProperty($property, $propertyValues))); + } + + public static function matchPropertyReasonProvider(): iterable + { + yield 'a missing property' => [self::filter('exact', 'x'), [], UnresolvedFlagReason::MISSING_CONTEXT]; + yield 'an unknown operator and a missing property' => [ + self::filter('unknown', 'x'), + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid semver filter and a missing property' => [ + self::filter('semver_gt', 'x.y'), + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid numeric filter and a missing property' => [ + self::filter('gt', 'ten'), + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid date filter and a missing property' => [ + self::filter('is_date_after', 'not-a-date'), + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an unsupported regex and a missing property' => [ + self::filter('regex', '(?a)'), + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an unknown operator' => [ + self::filter('unknown', 'x'), + ['key' => 'x'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid date filter' => [ + self::filter('is_date_after', 'not-a-date'), + ['key' => '2025-01-01'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'a numeric date filter' => [ + self::filter('is_date_after', 1234), + ['key' => '2025-01-01'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid context date' => [ + self::filter('is_date_after', '2025-01-01'), + ['key' => 'not-a-date'], + UnresolvedFlagReason::MISSING_CONTEXT, + ]; + yield 'a numeric context date' => [ + self::filter('is_date_after', '2025-01-01'), + ['key' => 1], + UnresolvedFlagReason::MISSING_CONTEXT, + ]; + yield 'an invalid date filter and context' => [ + self::filter('is_date_after', 'not-a-date'), + ['key' => 'also-not-a-date'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid semver filter' => [ + self::filter('semver_gt', 'x.y'), + ['key' => '1.2.3'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid tilde filter' => [ + self::filter('semver_tilde', 'x.y'), + ['key' => '1.2.3'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid caret filter' => [ + self::filter('semver_caret', 'x.y'), + ['key' => '1.2.3'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid wildcard filter' => [ + self::filter('semver_wildcard', '1x.*'), + ['key' => '1.8.0'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid context semver' => [ + self::filter('semver_gt', '1.2.3'), + ['key' => 'x.y'], + UnresolvedFlagReason::MISSING_CONTEXT, + ]; + yield 'an invalid context semver for a range' => [ + self::filter('semver_wildcard', '1.*'), + ['key' => 'x.y'], + UnresolvedFlagReason::MISSING_CONTEXT, + ]; + yield 'an invalid semver filter and context' => [ + self::filter('semver_gt', 'x.y'), + ['key' => 'a.b'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an invalid wildcard filter and context' => [ + self::filter('semver_wildcard', '1x.*'), + ['key' => 'a.b'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'an unsupported regex' => [ + self::filter('regex', '(?a)'), + ['key' => 'a'], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + } + + public function testConditionWithNonMatchingFilterResolvesDespiteInconclusiveFilter(): void + { + $missing = self::filter('exact', 'pro', 'plan'); + $nonMatching = self::filter('exact', 'other@example.com', 'email'); + $properties = ['email' => 'test@example.com']; + + foreach ([[$missing, $nonMatching], [$nonMatching, $missing]] as $filters) { + $this->assertFalse( + FeatureFlag::matchFeatureFlagProperties(self::flagWithConditions($filters), 'user', $properties) + ); + } + } + + public function testUnexpectedErrorAfterInconclusiveFilterLeavesTheConditionInconclusive(): void + { + $missing = self::filter('exact', 'pro', 'plan'); + $throwing = self::filter('exact', 'a', 'email'); + // A value that cannot be encoded as JSON makes the exact match throw a plain error. + $properties = ['email' => ["\xB1"]]; + + $this->assertTrue(FeatureFlag::matchFeatureFlagProperties( + self::flagWithConditions([$missing, $throwing], []), + 'user', + $properties + )); + $this->assertSame( + UnresolvedFlagReason::MISSING_CONTEXT, + self::reasonOf(fn() => FeatureFlag::matchFeatureFlagProperties( + self::flagWithConditions([$missing, $throwing]), + 'user', + $properties + )) + ); + } + + public function testConditionWithStaticCohortAndNonMatchingFilterResolvesLocally(): void + { + $flag = self::flagWithConditions([self::staticCohortFilter(), self::filter('exact', 'other', 'email')]); + + $this->assertFalse(FeatureFlag::matchFeatureFlagProperties($flag, 'user', ['email' => 'test'])); + } + + public function testConditionWithInconclusiveAndMatchingFiltersIsInconclusive(): void + { + $flag = self::flagWithConditions([self::filter('exact', 'pro', 'plan'), self::filter('exact', 'a', 'email')]); + + $this->assertSame( + UnresolvedFlagReason::MISSING_CONTEXT, + self::reasonOf(fn() => FeatureFlag::matchFeatureFlagProperties($flag, 'user', ['email' => 'a'])) + ); + } + + #[DataProvider('conditionReasonPrecedenceProvider')] + public function testUnresolvedReasonFollowsSpecPrecedence(array $conditions, string $reason): void + { + $flag = self::flagWithConditions(...$conditions); + $flagsByKey = [ + 'continuity-flag' => [ + 'key' => 'continuity-flag', + 'active' => true, + 'ensure_experience_continuity' => true, + ], + ]; + + $this->assertSame( + $reason, + self::reasonOf(fn() => FeatureFlag::matchFeatureFlagProperties( + $flag, + 'user', + ['email' => 'a'], + [], + $flagsByKey, + [] + )) + ); + } + + public static function conditionReasonPrecedenceProvider(): iterable + { + $missing = self::filter('exact', 'pro', 'plan'); + $unknownOperator = self::filter('unknown', 'a', 'email'); + $dependsOn = fn(array $chain) => [ + 'key' => end($chain), 'type' => 'flag', 'operator' => 'flag_evaluates_to', 'value' => true, + 'dependency_chain' => $chain, + ]; + $unresolvedDependency = $dependsOn(['continuity-flag']); + $missingDependency = $dependsOn(['gone']); + + yield 'missing property then unknown operator' => [ + [[$missing, $unknownOperator]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'unknown operator then missing property' => [ + [[$unknownOperator, $missing]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'missing property condition then unknown operator condition' => [ + [[$missing], [$unknownOperator]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'unknown operator condition then missing property condition' => [ + [[$unknownOperator], [$missing]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'missing property then unresolved dependency' => [ + [[$missing, $unresolvedDependency]], + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, + ]; + yield 'unresolved dependency condition then missing property condition' => [ + [[$unresolvedDependency], [$missing]], + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, + ]; + yield 'unresolved dependency then unknown operator' => [ + [[$unresolvedDependency, $unknownOperator]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'unresolved dependency then missing dependency' => [ + [[$unresolvedDependency, $missingDependency]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'missing property then static cohort' => [ + [[$missing, self::staticCohortFilter()]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'static cohort then missing property' => [ + [[self::staticCohortFilter(), $missing]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + } + + public function testCohortGroupKeepsHighestPrecedenceReason(): void + { + $flag = self::flagWithConditions([['key' => 'id', 'type' => 'cohort', 'value' => 1]]); + $cohorts = [ + '1' => [ + 'type' => 'OR', + 'values' => [ + self::filter('unknown', 'a', 'email'), + self::filter('exact', 'pro', 'plan'), + ], + ], + ]; + + $this->assertSame( + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + self::reasonOf(fn() => FeatureFlag::matchFeatureFlagProperties($flag, 'user', ['email' => 'a'], $cohorts)) + ); + } + + public function testStaticCohortRequiresServerOnlyWhenFirstUncheckableFilter(): void + { + $missing = self::filter('exact', 'pro', 'plan'); + $matching = [self::filter('exact', 'a', 'email')]; + + // The static cohort is the first filter local evaluation cannot check: fall back to the server. + $flag = self::flagWithConditions([self::staticCohortFilter(), $missing], $matching); + try { + FeatureFlag::matchFeatureFlagProperties($flag, 'user', ['email' => 'a']); + $this->fail('Expected RequiresServerEvaluationException'); + } catch (RequiresServerEvaluationException $e) { + $this->assertSame(UnresolvedFlagReason::UNSUPPORTED_DEFINITION, $e->getReason()); + } + + // After another uncheckable filter, a later condition can still resolve the flag locally. + $flag = self::flagWithConditions([$missing, self::staticCohortFilter()], $matching); + $this->assertTrue(FeatureFlag::matchFeatureFlagProperties($flag, 'user', ['email' => 'a'])); + } + + public function testMissingGroupPropertiesReportMissingContext(): void + { + $flag = [ + 'key' => 'mixed-flag', + 'active' => true, + 'filters' => [ + 'groups' => [ + ['aggregation_group_type_index' => 0, 'properties' => [], 'rollout_percentage' => 100], + ], + ], + ]; + + $this->assertSame( + UnresolvedFlagReason::MISSING_CONTEXT, + self::reasonOf(fn() => FeatureFlag::matchFeatureFlagProperties( + $flag, + 'user', + [], + [], + [], + [], + ['company' => 'acme'], + [], + ['0' => 'company'] + )) + ); + } + + #[DataProvider('definitionUnresolvedReasonProvider')] + public function testDefinitionUnresolvedReason(array $flag, ?string $reason): void + { + $flagsByKey = [ + 'continuity-flag' => ['key' => 'continuity-flag', 'active' => true, 'ensure_experience_continuity' => true], + 'loaded-flag' => ['key' => 'loaded-flag', 'active' => true], + ]; + $cohorts = ['1' => ['type' => 'AND', 'values' => [self::filter('unknown', 'a', 'email')]]]; + + $this->assertSame( + $reason, + FeatureFlag::getDefinitionUnresolvedReason($flag, $flagsByKey, $cohorts, ['0' => 'company']) + ); + } + + public static function definitionUnresolvedReasonProvider(): iterable + { + $dependency = fn(array $chain) => [ + 'key' => end($chain), 'type' => 'flag', 'operator' => 'flag_evaluates_to', 'value' => true, + 'dependency_chain' => $chain, + ]; + $continuity = self::flagWithConditions([]); + $continuity['ensure_experience_continuity'] = true; + $inactive = $continuity; + $inactive['active'] = false; + $unknownGroupType = self::flagWithConditions([]); + $unknownGroupType['filters']['aggregation_group_type_index'] = 5; + + yield 'experience continuity' => [$continuity, UnresolvedFlagReason::EXPERIENCE_CONTINUITY]; + yield 'inactive with experience continuity' => [$inactive, null]; + yield 'unknown group type' => [$unknownGroupType, UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + yield 'static cohort' => [self::flagWithConditions([self::staticCohortFilter()]), UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + yield 'unknown operator' => [self::flagWithConditions([self::filter('unknown', 'a')]), UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + yield 'malformed semver' => [self::flagWithConditions([self::filter('semver_gt', 'x.y')]), UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + yield 'unknown operator in a cohort' => [ + self::flagWithConditions([['key' => 'id', 'type' => 'cohort', 'value' => 1]]), + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'missing dependency' => [self::flagWithConditions([$dependency(['gone'])]), UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + yield 'missing dependency in the chain' => [ + self::flagWithConditions([$dependency(['gone', 'loaded-flag'])]), + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'circular dependency' => [self::flagWithConditions([$dependency([])]), UnresolvedFlagReason::UNSUPPORTED_DEFINITION]; + yield 'dependency on an experience continuity flag' => [self::flagWithConditions([$dependency(['continuity-flag'])]), null]; + yield 'loaded dependency' => [self::flagWithConditions([$dependency(['loaded-flag'])]), null]; + yield 'person property' => [self::flagWithConditions([self::filter('exact', 'pro', 'plan')]), null]; + } + + // ==================== Unresolvable Flag Warning Tests ==================== + + private static function definitionsResponse(array ...$flags): array + { + return ['flags' => $flags, 'group_type_mapping' => [], 'cohorts' => []]; + } + + private static function continuityFlag(string $key = 'checkout', int $version = 1): array + { + return [ + 'id' => 1, + 'key' => $key, + 'version' => $version, + 'active' => true, + 'ensure_experience_continuity' => true, + 'filters' => ['groups' => [['properties' => [], 'rollout_percentage' => 100]]], + ]; + } + + private function loadDefinitionsInOrder(array $responses): Client + { + $this->http_client = new MockedHttpClient(host: 'app.posthog.com'); + $this->http_client->setFlagEndpointResponseQueue($responses); + $this->client = new Client(self::FAKE_API_KEY, [], $this->http_client, 'test'); + for ($i = 1; $i < count($responses); $i++) { + $this->client->loadFlags(); + } + return $this->client; + } + + /** @return list */ + private static function warningsNaming(string $flagKey, string $reason): array + { + global $errorMessages; + return array_values(array_filter( + $errorMessages, + fn($message) => str_contains($message, "(reason: {$reason}):") + && preg_match('/\): (.*)\. /U', $message, $matches) === 1 + && in_array($flagKey, explode(', ', $matches[1]), true) + )); + } + + #[DataProvider('unchangedRefreshProvider')] + public function testLoadingNeverResolvableDefinitionLogsOneWarning(array $refresh): void + { + $this->loadDefinitionsInOrder([ + ['response' => self::definitionsResponse(self::continuityFlag()), 'etag' => '"v1"'], + $refresh, + ]); + + $this->assertCount(1, self::warningsNaming('checkout', UnresolvedFlagReason::EXPERIENCE_CONTINUITY)); + $this->assertSame( + ['/flags/definitions?send_cohorts&token=random_key', '/flags/definitions?send_cohorts&token=random_key'], + array_column($this->http_client->calls, 'path') + ); + } + + public static function unchangedRefreshProvider(): iterable + { + yield 'unchanged definitions' => [ + ['response' => self::definitionsResponse(self::continuityFlag()), 'etag' => '"v1-again"'], + ]; + yield '304 not modified' => [['responseCode' => 304, 'etag' => '"v1"']]; + } + + public function testChangedNeverResolvableDefinitionLogsWarningAgain(): void + { + $this->loadDefinitionsInOrder([ + ['response' => self::definitionsResponse(self::continuityFlag(version: 1))], + ['response' => self::definitionsResponse(self::continuityFlag(version: 2))], + ]); + + $this->assertCount(2, self::warningsNaming('checkout', UnresolvedFlagReason::EXPERIENCE_CONTINUITY)); + } + + public function testWarningIsLoggedWithoutDebugAndNamesEveryUnsupportedDefinition(): void + { + $flag = fn(string $key, array $properties) => [ + 'id' => 1, + 'key' => $key, + 'active' => true, + 'filters' => ['groups' => [['properties' => $properties, 'rollout_percentage' => 100]]], + ]; + $dependsOn = fn(string $key, array $chain) => [ + 'key' => $key, 'type' => 'flag', 'operator' => 'flag_evaluates_to', 'value' => true, + 'dependency_chain' => $chain, + ]; + $inactiveContinuity = self::continuityFlag('inactive-continuity'); + $inactiveContinuity['active'] = false; + + $this->loadDefinitionsInOrder([[ + 'response' => self::definitionsResponse( + self::continuityFlag(), + $flag('static-cohort', [self::staticCohortFilter()]), + $flag('unknown-operator', [self::filter('unknown', 'a')]), + $flag('malformed-value', [self::filter('semver_gt', 'x.y')]), + $flag('missing-dependency', [$dependsOn('gone', ['gone'])]), + $flag('circular-dependency', [$dependsOn('checkout', [])]), + $flag('missing-context', [self::filter('exact', 'pro', 'plan')]), + $flag('unresolved-dependency', [$dependsOn('checkout', ['checkout'])]), + $inactiveContinuity, + ), + ]]); + + global $errorMessages; + $this->assertCount(2, $errorMessages); + $this->assertStringContainsString( + '1 flag(s) have experience continuity enabled and cannot be resolved locally ' + . '(reason: experience_continuity): checkout.', + $errorMessages[0] + ); + $this->assertStringContainsString( + '(reason: unsupported_definition): static-cohort, unknown-operator, malformed-value, ' + . 'missing-dependency, circular-dependency.', + $errorMessages[1] + ); + } + + public function testFlagThatReturnsAfterRemovalLogsWarningAgain(): void + { + $this->loadDefinitionsInOrder([ + ['response' => self::definitionsResponse(self::continuityFlag())], + ['response' => self::definitionsResponse()], + ['response' => self::definitionsResponse(self::continuityFlag())], + ]); + + $this->assertCount(2, self::warningsNaming('checkout', UnresolvedFlagReason::EXPERIENCE_CONTINUITY)); + } + + public function testDependencyLoadedThenRemovedLogsWarningAgain(): void + { + $dependent = [ + 'id' => 2, + 'key' => 'dependent', + 'active' => true, + 'filters' => ['groups' => [[ + 'properties' => [[ + 'key' => 'checkout', 'type' => 'flag', 'operator' => 'flag_evaluates_to', 'value' => true, + 'dependency_chain' => ['checkout'], + ]], + 'rollout_percentage' => 100, + ]]], + ]; + + $this->loadDefinitionsInOrder([ + ['response' => self::definitionsResponse($dependent)], + ['response' => self::definitionsResponse($dependent, self::continuityFlag())], + ['response' => self::definitionsResponse($dependent)], + ]); + + $this->assertCount(2, self::warningsNaming('dependent', UnresolvedFlagReason::UNSUPPORTED_DEFINITION)); + } } diff --git a/test/FlagDefinitionCacheProviderTest.php b/test/FlagDefinitionCacheProviderTest.php index 2ae8575..a6d5728 100644 --- a/test/FlagDefinitionCacheProviderTest.php +++ b/test/FlagDefinitionCacheProviderTest.php @@ -812,6 +812,40 @@ private function sampleFlagDefinitionData(): array ]; } + public function testUnresolvableFlagWarningIsLoggedForApiLoadsOnly(): void + { + $definitions = $this->sampleFlagDefinitionData(); + $definitions['flags'][0]['ensure_experience_continuity'] = true; + $warning = '(reason: experience_continuity): beta-ui.'; + + // A client that only reads the shared cache does not warn: the request that fetched the + // definitions already did. + $provider = new MockFlagDefinitionCacheProvider(); + $provider->shouldFetch = false; + $provider->cachedData = $definitions; + $this->createClient($provider, new MockedHttpClient(host: 'app.posthog.com')); + $this->assertWarningCount($warning, 0); + + $provider = new MockFlagDefinitionCacheProvider(); + $httpClient = new MockedHttpClient(host: 'app.posthog.com', flagEndpointResponse: $definitions); + $client = $this->createClient($provider, $httpClient); + $this->assertWarningCount($warning, 1); + + $provider->shouldFetch = false; + $provider->cachedData = $provider->storedData; + $client->loadFlags(); + $this->assertSame(1, $provider->getCallCount); + $this->assertCount(1, $httpClient->calls); + $this->assertWarningCount($warning, 1); + } + + private function assertWarningCount(string $expected, int $count): void + { + global $errorMessages; + $matching = array_filter($errorMessages, fn($message) => str_contains($message, $expected)); + $this->assertCount($count, $matching, implode("\n", $errorMessages)); + } + private function assertWarningContains(string $expected): void { global $errorMessages; diff --git a/test/FlagDependencyTest.php b/test/FlagDependencyTest.php index cad9528..8cbad54 100644 --- a/test/FlagDependencyTest.php +++ b/test/FlagDependencyTest.php @@ -2,11 +2,14 @@ namespace PostHog\Test; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\TestCase; use PostHog\Client; use PostHog\FeatureFlag; use PostHog\InconclusiveMatchException; +use PostHog\RequiresServerEvaluationException; +use PostHog\UnresolvedFlagReason; class FlagDependencyTest extends TestCase { @@ -141,9 +144,12 @@ public function testFlagDependencyCircularDependency(): void $evaluationCache = []; // Both flags should raise InconclusiveMatchException due to circular dependency - $this->expectException(InconclusiveMatchException::class); - $this->expectExceptionMessage("Circular dependency detected for flag 'flag-b'"); - FeatureFlag::matchFeatureFlagProperties($flagA, "test-user", [], [], $flagsByKey, $evaluationCache); + try { + FeatureFlag::matchFeatureFlagProperties($flagA, "test-user", [], [], $flagsByKey, $evaluationCache); + $this->fail('Expected InconclusiveMatchException'); + } catch (InconclusiveMatchException $e) { + $this->assertSame(UnresolvedFlagReason::UNSUPPORTED_DEFINITION, $e->getReason()); + } } public function testFlagDependencyMissingFlag(): void @@ -179,11 +185,12 @@ public function testFlagDependencyMissingFlag(): void $evaluationCache = []; // Should raise InconclusiveMatchException because dependency doesn't exist - $this->expectException(InconclusiveMatchException::class); - $this->expectExceptionMessage( - "Cannot evaluate flag dependency 'non-existent-flag' - flag not found in local flags" - ); - FeatureFlag::matchFeatureFlagProperties($flag, "test-user", [], [], $flagsByKey, $evaluationCache); + try { + FeatureFlag::matchFeatureFlagProperties($flag, "test-user", [], [], $flagsByKey, $evaluationCache); + $this->fail('Expected InconclusiveMatchException'); + } catch (InconclusiveMatchException $e) { + $this->assertSame(UnresolvedFlagReason::UNSUPPORTED_DEFINITION, $e->getReason()); + } } public function testFlagDependencyComplexChain(): void @@ -823,4 +830,241 @@ public function testMultiLevelMultivariateDependencyChain(): void ); $this->assertFalse($intermediateResult); } + + private static function dependencyFlag(string $key, array $properties = [], array $overrides = []): array + { + return array_merge([ + "key" => $key, + "active" => true, + "filters" => ["groups" => [["properties" => $properties, "rollout_percentage" => 100]]], + ], $overrides); + } + + private static function dependsOn(string $key, mixed $value = true, ?array $chain = null): array + { + return [ + "key" => $key, + "operator" => "flag_evaluates_to", + "value" => $value, + "type" => "flag", + "dependency_chain" => $chain ?? [$key], + ]; + } + + private static function dependencyReason(array $flag, array $flagsByKey, array $properties = []): ?string + { + try { + FeatureFlag::matchFeatureFlagProperties($flag, "test-user", $properties, [], $flagsByKey, []); + } catch (InconclusiveMatchException $e) { + return $e->getReason(); + } + return null; + } + + public function testDependencyWithExperienceContinuityIsAnUnresolvedDependency(): void + { + $dependency = self::dependencyFlag("dependency", [], ["ensure_experience_continuity" => true]); + $flag = self::dependencyFlag("flag", [self::dependsOn("dependency")]); + + $this->assertSame( + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, + self::dependencyReason($flag, ["dependency" => $dependency, "flag" => $flag]) + ); + } + + public function testInactiveDependencyWithExperienceContinuityIsFalse(): void + { + $dependency = self::dependencyFlag( + "dependency", + [], + ["active" => false, "ensure_experience_continuity" => true] + ); + $flag = self::dependencyFlag("flag", [self::dependsOn("dependency", false)]); + + $this->assertTrue(FeatureFlag::matchFeatureFlagProperties( + $flag, + "test-user", + [], + [], + ["dependency" => $dependency, "flag" => $flag], + [] + )); + } + + public function testDependencyThatNeedsContextIsAnUnresolvedDependency(): void + { + $dependency = self::dependencyFlag("dependency", [["key" => "plan", "operator" => "exact", "value" => "pro"]]); + $flag = self::dependencyFlag("flag", [self::dependsOn("dependency")]); + + $this->assertSame( + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, + self::dependencyReason($flag, ["dependency" => $dependency, "flag" => $flag]) + ); + } + + public function testInvalidDependencyOperatorOutranksADependencyThatNeedsContext(): void + { + $dependency = self::dependencyFlag("dependency", [["key" => "plan", "operator" => "exact", "value" => "pro"]]); + $flag = self::dependencyFlag("flag", [array_merge(self::dependsOn("dependency"), ["operator" => "exact"])]); + $flagsByKey = ["dependency" => $dependency, "flag" => $flag]; + + $this->assertSame( + [UnresolvedFlagReason::UNSUPPORTED_DEFINITION, UnresolvedFlagReason::UNSUPPORTED_DEFINITION], + [ + self::dependencyReason($flag, $flagsByKey), + FeatureFlag::getDefinitionUnresolvedReason($flag, $flagsByKey, [], []), + ] + ); + } + + public function testDependencyThatRequiresServerIsAnUnresolvedDependency(): void + { + $dependency = self::dependencyFlag("dependency", [["key" => "id", "type" => "cohort", "value" => 999]]); + $flag = self::dependencyFlag("flag", [self::dependsOn("dependency")]); + + try { + FeatureFlag::matchFeatureFlagProperties($flag, "test-user", [], [], ["dependency" => $dependency], []); + $this->fail("Expected RequiresServerEvaluationException"); + } catch (RequiresServerEvaluationException $e) { + $this->assertSame(UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, $e->getReason()); + } + } + + #[DataProvider('serverDependencyReasonProvider')] + public function testDependencyThatRequiresServerReportsTheHighestReasonOfTheFlag( + array $conditionFilters, + array $cohorts, + string $reason + ): void { + $dependency = self::dependencyFlag("dependency", [["key" => "id", "type" => "cohort", "value" => 999]]); + $flag = [ + "key" => "flag", + "active" => true, + "filters" => ["groups" => array_map( + fn (array $filters) => ["properties" => $filters, "rollout_percentage" => 100], + $conditionFilters + )], + ]; + + try { + FeatureFlag::matchFeatureFlagProperties( + $flag, + "test-user", + ["plan" => "free"], + $cohorts, + ["dependency" => $dependency], + [] + ); + $this->fail("Expected RequiresServerEvaluationException"); + } catch (RequiresServerEvaluationException $e) { + $this->assertSame($reason, $e->getReason()); + } + } + + public static function serverDependencyReasonProvider(): iterable + { + $dependsOn = self::dependsOn("dependency"); + $unknownOperator = ["key" => "plan", "operator" => "not_a_real_operator", "value" => "pro"]; + $onProPlan = ["key" => "plan", "operator" => "exact", "value" => "pro"]; + $inCohort = ["key" => "id", "type" => "cohort", "value" => 1]; + + yield 'later condition matches everyone' => [ + [[$dependsOn], []], + [], + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, + ]; + yield 'later condition is unsupported' => [ + [[$dependsOn], [$unknownOperator]], + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'later filter in the same condition is unsupported' => [ + [[$dependsOn, $unknownOperator]], + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'earlier condition fails before its unsupported filter' => [ + [[$onProPlan, $unknownOperator], [$dependsOn]], + [], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'cohort checks the dependency before an unsupported filter' => [ + [[$inCohort]], + [1 => ["type" => "AND", "values" => [$dependsOn, $unknownOperator]]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + yield 'cohort checks an unsupported filter before the dependency' => [ + [[$inCohort]], + [1 => ["type" => "AND", "values" => [$unknownOperator, $dependsOn]]], + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + ]; + } + + public function testMissingDependencyAnywhereInTheChainIsAnUnsupportedDefinition(): void + { + $dependency = self::dependencyFlag("dependency", [], ["ensure_experience_continuity" => true]); + $flag = self::dependencyFlag("flag", [self::dependsOn("dependency", true, ["gone", "dependency"])]); + + $this->assertSame( + UnresolvedFlagReason::UNSUPPORTED_DEFINITION, + self::dependencyReason($flag, ["dependency" => $dependency, "flag" => $flag]) + ); + } + + public function testRepeatedInconclusiveDependencyKeepsItsReason(): void + { + $dependency = self::dependencyFlag("dependency", [], ["ensure_experience_continuity" => true]); + $flag = [ + "key" => "flag", + "active" => true, + "filters" => ["groups" => [ + ["properties" => [self::dependsOn("dependency")], "rollout_percentage" => 100], + ["properties" => [self::dependsOn("dependency", false)], "rollout_percentage" => 100], + ]], + ]; + + $this->assertSame( + UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, + self::dependencyReason($flag, ["dependency" => $dependency]) + ); + } + + public function testGroupConditionDependencyResultIsNotReusedForPersonCondition(): void + { + // The dependency matches only with the group's properties, so a result cached for the group + // condition must not resolve the person condition. + $dependency = self::dependencyFlag( + "dependency", + [["key" => "\$group_key", "operator" => "exact", "value" => "acme"]] + ); + $flag = [ + "key" => "flag", + "active" => true, + "filters" => ["groups" => [ + [ + "aggregation_group_type_index" => 0, + "properties" => [self::dependsOn("dependency", false)], + "rollout_percentage" => 100, + ], + ["properties" => [self::dependsOn("dependency")], "rollout_percentage" => 100], + ]], + ]; + + try { + FeatureFlag::matchFeatureFlagProperties( + $flag, + "test-user", + [], + [], + ["dependency" => $dependency], + [], + ["company" => "acme"], + ["company" => ["\$group_key" => "acme"]], + ["0" => "company"] + ); + $this->fail("Expected InconclusiveMatchException"); + } catch (InconclusiveMatchException $e) { + $this->assertSame(UnresolvedFlagReason::UNRESOLVED_DEPENDENCY, $e->getReason()); + } + } }