From 3c0989a5f17a728c0668a1f79cb88225e942d49e Mon Sep 17 00:00:00 2001 From: "Gustavo H. Strassburger" Date: Tue, 27 Oct 2026 14:37:35 -0300 Subject: [PATCH 1/3] feat(flags): report unresolved local flags on EvaluateFlagsAsync snapshots Implements the "Snapshot unresolved flag reporting" and "Warning for flags local evaluation never resolves" requirements of the evaluate-flags spec (PostHog/sdk-specs#85). - Add `FeatureFlagEvaluations.UnresolvedFlags`, with an `UnresolvedFlagReason` for each flag and the spec's fixed precedence. - Report `local_evaluation_inconclusive` instead of `flag_missing` when a snapshot read hits an unresolved flag. - Do not stop a condition at the first inconclusive filter, so a filter that does not match resolves the condition locally. - Check `active` before experience continuity. - Log one warning per flag that local evaluation can never resolve, repeated only when its definition changes. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 39793b31-809e-4689-a5d8-b4359cbb22c1 --- .changeset/unresolved-local-flags.md | 13 + .../RequiresServerEvaluationException.cs | 12 + src/PostHog/Features/FeatureFlagError.cs | 5 + .../Features/FeatureFlagEvaluations.cs | 26 +- .../Features/IFeatureFlagEvaluationsHost.cs | 7 +- src/PostHog/Features/LocalEvaluator.cs | 353 ++++++++++++++--- .../Features/LocalFeatureFlagsLoader.cs | 78 ++++ src/PostHog/Features/UnresolvedFlagReason.cs | 70 ++++ src/PostHog/IPostHogClient.cs | 4 +- .../Json/InconclusiveMatchException.cs | 18 + src/PostHog/Json/PropertyFilterValue.cs | 99 ++--- src/PostHog/Library/RegexHelpers.cs | 16 +- src/PostHog/NoOpPostHogClient.cs | 3 +- src/PostHog/PostHogClient.cs | 19 +- src/PostHog/PublicAPI.Unshipped.txt | 8 + .../Features/FeatureFlagEvaluationsTests.cs | 331 ++++++++++++++++ .../Features/LocalEvaluatorRegexTests.cs | 12 +- .../UnitTests/Features/LocalEvaluatorTests.cs | 356 ++++++++++++++++++ .../Features/LocalFeatureFlagsLoaderTests.cs | 130 +++++++ .../VersionedPropertyMatchingTests.cs | 2 +- 20 files changed, 1443 insertions(+), 119 deletions(-) create mode 100644 .changeset/unresolved-local-flags.md create mode 100644 src/PostHog/Features/UnresolvedFlagReason.cs diff --git a/.changeset/unresolved-local-flags.md b/.changeset/unresolved-local-flags.md new file mode 100644 index 00000000..521b2423 --- /dev/null +++ b/.changeset/unresolved-local-flags.md @@ -0,0 +1,13 @@ +--- +"PostHog": minor +--- + +Add `FeatureFlagEvaluations.UnresolvedFlags`, which lists the flags in an `EvaluateFlagsAsync` snapshot that have a loaded local definition but that neither local evaluation nor the remote fallback resolved, each with an `UnresolvedFlagReason`. Log a warning once for each flag whose definition local evaluation can never resolve. + +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, instead of falling back to a server request or staying unresolved. A static cohort forces the server fallback only when it is the first filter in its condition that local evaluation cannot check. +- An inactive flag with experience continuity enabled now resolves to `false` locally instead of making a server request. +- A condition that depends on a flag that needs the server, such as a flag with a static cohort, no longer stops local evaluation at that condition, so a later condition can resolve the flag locally. +- `$feature_flag_called` events from snapshot reads report `local_evaluation_inconclusive` instead of `flag_missing` for unresolved flags. +- When definitions load, the SDK logs a warning that names each flag with experience continuity (`experience_continuity`) or with something local evaluation does not support, such as a static cohort, an unknown operator, or a missing or circular dependency (`unsupported_definition`). It logs the warning again only when the definition of that flag changes. A static cohort no longer logs an error on each evaluation. diff --git a/src/PostHog/Exceptions/RequiresServerEvaluationException.cs b/src/PostHog/Exceptions/RequiresServerEvaluationException.cs index e558870c..22e031c0 100644 --- a/src/PostHog/Exceptions/RequiresServerEvaluationException.cs +++ b/src/PostHog/Exceptions/RequiresServerEvaluationException.cs @@ -1,4 +1,5 @@ using System; +using PostHog.Features; namespace PostHog.Exceptions { @@ -35,5 +36,16 @@ public RequiresServerEvaluationException(string message, Exception innerExceptio : base(message, innerException) { } + + internal RequiresServerEvaluationException(string message, UnresolvedFlagReason reason) : base(message) + { + Reason = reason; + } + + /// + /// Why local evaluation could not resolve the flag. A condition that reaches this exception can raise it + /// to a higher-precedence reason found earlier in the same flag. + /// + internal UnresolvedFlagReason Reason { get; set; } = UnresolvedFlagReason.UnsupportedDefinition; } } diff --git a/src/PostHog/Features/FeatureFlagError.cs b/src/PostHog/Features/FeatureFlagError.cs index 0d841958..c670c43a 100644 --- a/src/PostHog/Features/FeatureFlagError.cs +++ b/src/PostHog/Features/FeatureFlagError.cs @@ -18,6 +18,11 @@ internal static class FeatureFlagError /// public const string FlagMissing = "flag_missing"; + /// + /// Flag has a loaded local definition but local evaluation could not resolve it + /// + public const string LocalEvaluationInconclusive = "local_evaluation_inconclusive"; + /// /// Rate/quota limit exceeded (when quotaLimited contains "feature_flags") /// diff --git a/src/PostHog/Features/FeatureFlagEvaluations.cs b/src/PostHog/Features/FeatureFlagEvaluations.cs index e09a8770..8aac4062 100644 --- a/src/PostHog/Features/FeatureFlagEvaluations.cs +++ b/src/PostHog/Features/FeatureFlagEvaluations.cs @@ -1,4 +1,5 @@ using System.Collections.Concurrent; +using System.Collections.ObjectModel; using System.Text.Json; using static PostHog.Library.Ensure; @@ -23,6 +24,9 @@ public sealed class FeatureFlagEvaluations readonly GroupCollection? _groups; readonly IReadOnlyCollection _errors; + static readonly IReadOnlyDictionary NoUnresolvedFlags = + new ReadOnlyDictionary(new Dictionary()); + internal FeatureFlagEvaluations( IFeatureFlagEvaluationsHost host, string distinctId, @@ -32,7 +36,8 @@ internal FeatureFlagEvaluations( long? flagDefinitionsLoadedAt, GroupCollection? groups, IReadOnlyCollection? errors, - ConcurrentDictionary? accessed = null) + ConcurrentDictionary? accessed = null, + Dictionary? unresolvedFlags = null) { _host = NotNull(host); DistinctId = distinctId ?? string.Empty; @@ -43,6 +48,9 @@ internal FeatureFlagEvaluations( _groups = groups; _errors = errors ?? Array.Empty(); _accessed = accessed ?? new ConcurrentDictionary(StringComparer.Ordinal); + UnresolvedFlags = unresolvedFlags is { Count: > 0 } + ? new ReadOnlyDictionary(unresolvedFlags) + : NoUnresolvedFlags; } /// @@ -74,6 +82,18 @@ internal FeatureFlagEvaluations( /// public IReadOnlyCollection Keys => _records.Keys; + /// + /// The flags that have a loaded local definition but that neither local evaluation nor a remote fallback + /// resolved, mapped to the reason local evaluation could not resolve them. These flags are absent from + /// , and their accessors return the missing-flag values. Reading this property does not + /// record access and does not fire $feature_flag_called. + /// + /// + /// Always empty on snapshots returned by + /// and . + /// + public IReadOnlyDictionary UnresolvedFlags { get; } + /// /// Returns true when the named flag is present in the snapshot and enabled. Records /// access on the snapshot and fires $feature_flag_called on first access for a given @@ -238,11 +258,13 @@ internal static FeatureFlagEvaluations Empty( requestId: RequestId, evaluatedAt: EvaluatedAt, flagDefinitionsLoadedAt: FlagDefinitionsLoadedAt, - errors: _errors); + errors: _errors, + unresolved: UnresolvedFlags.ContainsKey(keyChecked)); return record; } + // Filtered snapshots carry no unresolved flags. FeatureFlagEvaluations CloneWith(Dictionary records) => new( _host, diff --git a/src/PostHog/Features/IFeatureFlagEvaluationsHost.cs b/src/PostHog/Features/IFeatureFlagEvaluationsHost.cs index 2581fba5..8b371feb 100644 --- a/src/PostHog/Features/IFeatureFlagEvaluationsHost.cs +++ b/src/PostHog/Features/IFeatureFlagEvaluationsHost.cs @@ -9,7 +9,9 @@ internal interface IFeatureFlagEvaluationsHost { /// /// Fires a $feature_flag_called event for the given access, deduplicated against the - /// per-distinct-id cache that the legacy single-flag path also writes to. + /// per-distinct-id cache that the legacy single-flag path also writes to. + /// is true when the key is in , so a missing + /// record reports local_evaluation_inconclusive instead of flag_missing. /// void CaptureFeatureFlagCalled( string distinctId, @@ -19,7 +21,8 @@ void CaptureFeatureFlagCalled( string? requestId, long? evaluatedAt, long? flagDefinitionsLoadedAt, - IReadOnlyCollection errors); + IReadOnlyCollection errors, + bool unresolved); /// /// Logs a warning from or diff --git a/src/PostHog/Features/LocalEvaluator.cs b/src/PostHog/Features/LocalEvaluator.cs index 5deb0f6a..d9115490 100644 --- a/src/PostHog/Features/LocalEvaluator.cs +++ b/src/PostHog/Features/LocalEvaluator.cs @@ -15,6 +15,7 @@ using PostHog.Api; using PostHog.Exceptions; using PostHog.Json; +using PostHog.Library; using static PostHog.Library.Ensure; namespace PostHog.Features; @@ -124,7 +125,14 @@ public StringOrValue EvaluateFeatureFlag( warnOnUnknownGroups); } - public (IReadOnlyDictionary, bool) EvaluateAllFlags( + /// + /// Evaluates every flag in scope. + /// + /// + /// The resolved flags, whether a remote fallback is needed, and the reason for each flag in scope that local + /// evaluation left unresolved. + /// + public (IReadOnlyDictionary, bool, IReadOnlyDictionary) EvaluateAllFlags( string distinctId, GroupCollection? groups = null, Dictionary? personProperties = null, @@ -132,10 +140,11 @@ public StringOrValue EvaluateFeatureFlag( IReadOnlyCollection? flagKeysToEvaluate = null) { Dictionary results = new(); + Dictionary unresolved = new(StringComparer.Ordinal); if (LocalEvaluationApiResult.Flags is []) { - return (results, true); + return (results, true, unresolved); } var fallbackToRemote = false; @@ -160,10 +169,17 @@ public StringOrValue EvaluateFeatureFlag( results[flag.Key] = FeatureFlag.CreateFromLocalEvaluation(flag.Key, flagValue, flag, _logger); } - catch (InconclusiveMatchException) + catch (InconclusiveMatchException e) { // No need to log this, since it's just telling us to fall back to the `/flags` endpoint. fallbackToRemote = true; + unresolved[flag.Key] = e.Reason; + } + catch (RequiresServerEvaluationException e) + { + // Same as above. The load-time warning already names the static cohort that causes this. + fallbackToRemote = true; + unresolved[flag.Key] = e.Reason; } catch (Exception e) when (e is not ArgumentException and not NullReferenceException) { @@ -172,7 +188,83 @@ public StringOrValue EvaluateFeatureFlag( } } - return (results, fallbackToRemote); + return (results, fallbackToRemote, unresolved); + } + + /// + /// Returns 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. + /// + /// The flag definition. + /// + /// , , + /// or null when the definition alone does not stop local evaluation. + /// + public UnresolvedFlagReason? GetDefinitionUnresolvedReason(LocalFeatureFlag flag) + { + if (!flag.Active) + { + return null; + } + + if (flag.EnsureExperienceContinuity) + { + return UnresolvedFlagReason.ExperienceContinuity; + } + + if (flag.Filters?.AggregationGroupTypeIndex is { } aggregationGroupIndex + && !_groupTypeMapping.ContainsKey(aggregationGroupIndex)) + { + return UnresolvedFlagReason.UnsupportedDefinition; + } + + var visitedCohortIds = new HashSet(); + var usesUnsupportedFilter = (flag.Filters?.Groups ?? []) + .SelectMany(condition => condition.Properties ?? []) + .Any(filter => IsUnsupportedFilter(filter, visitedCohortIds, inCohort: false)); + return usesUnsupportedFilter ? UnresolvedFlagReason.UnsupportedDefinition : null; + } + + bool IsUnsupportedFilter(Filter filter, HashSet visitedCohortIds, bool inCohort) + { + switch (filter) + { + case FilterSet filterSet: + return filterSet.Values.Any(child => IsUnsupportedFilter(child, visitedCohortIds, inCohort)); + case PropertyFilter { Type: FilterType.Flag } dependency: + // MatchProperty cannot evaluate a flag dependency inside a cohort. + if (inCohort) + { + return true; + } + try + { + ValidateFlagDependencyFilter(dependency); + } + catch (InconclusiveMatchException) + { + return true; + } + return dependency.DependencyChain!.Any(key => !_localFeatureFlags.ContainsKey(key)); + case PropertyFilter { Type: FilterType.Cohort } cohort: + if (cohort.Value?.CohortId is not { } cohortId || !_cohortFilters.TryGetValue(cohortId, out var cohortFilters)) + { + return true; + } + return visitedCohortIds.Add(cohortId) && IsUnsupportedFilter(cohortFilters, visitedCohortIds, inCohort: true); + case PropertyFilter property: + try + { + AssertFilterSupported(property); + return false; + } + catch (InconclusiveMatchException) + { + return true; + } + default: + return false; + } } public StringOrValue ComputeFlagLocally( @@ -205,11 +297,6 @@ StringOrValue ComputeFlagLocallyWithCache( return cachedResult; } - if (flag.EnsureExperienceContinuity) - { - throw new InconclusiveMatchException($"Flag \"{flag.Key}\" has experience continuity enabled"); - } - if (!flag.Active) { var result = false; @@ -217,6 +304,12 @@ StringOrValue ComputeFlagLocallyWithCache( return result; } + if (flag.EnsureExperienceContinuity) + { + throw new InconclusiveMatchException( + $"Flag \"{flag.Key}\" has experience continuity enabled", + UnresolvedFlagReason.ExperienceContinuity); + } var filters = flag.Filters; var aggregationGroupIndex = filters?.AggregationGroupTypeIndex; @@ -237,7 +330,9 @@ StringOrValue ComputeFlagLocallyWithCache( { // Weird: We have a group type index that doesn't point to an actual group. _logger.LogWarnUnknownGroupType(aggregationGroupIndex.Value, flag.Key); - throw new InconclusiveMatchException($"Flag has unknown group type index: {aggregationGroupIndex}"); + throw new InconclusiveMatchException( + $"Flag has unknown group type index: {aggregationGroupIndex}", + UnresolvedFlagReason.UnsupportedDefinition); } if (groups.TryGetGroup(groupType, out var group)) @@ -280,7 +375,7 @@ StringOrValue MatchFeatureFlagProperties( var flagConditions = filters?.Groups ?? []; var flagAggregation = filters?.AggregationGroupTypeIndex; var earlyExit = filters?.EarlyExit ?? false; - var isInconclusive = false; + UnresolvedFlagReason? inconclusiveReason = null; var flagVariants = filters?.Multivariate?.Variants ?? []; foreach (var condition in flagConditions) @@ -318,16 +413,18 @@ StringOrValue MatchFeatureFlagProperties( { conditionResult = EvaluateConditionMatch(flag, effectiveBucketingId, condition, effectiveProperties, evaluationCache, groups); } - catch (RequiresServerEvaluationException) + catch (RequiresServerEvaluationException e) { - // Static cohort or other missing server-side data - must fallback to API + // Static cohort or other missing server-side data - must fallback to API. + // A reason an earlier condition found can still outrank this one. + e.Reason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, e.Reason); throw; } - catch (InconclusiveMatchException) + catch (InconclusiveMatchException e) { // Evaluation error (bad regex, invalid date, missing property, etc.) // Track that we had an inconclusive match, but try other conditions - isInconclusive = true; + inconclusiveReason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, e.Reason); } if (conditionResult is null) @@ -345,9 +442,9 @@ StringOrValue MatchFeatureFlagProperties( // to the caller and isn't re-caught as a regular inconclusive match. if (earlyExit && conditionResult == ConditionMatchResult.OutOfRolloutBound) { - if (isInconclusive) + if (inconclusiveReason is { } reason) { - throw new InconclusiveMatchException("Can't determine if feature flag is enabled or not with given properties"); + throw new InconclusiveMatchException("Can't determine if feature flag is enabled or not with given properties", reason); } return false; } @@ -368,9 +465,9 @@ StringOrValue MatchFeatureFlagProperties( : true; } - if (isInconclusive) + if (inconclusiveReason is { } unresolvedReason) { - throw new InconclusiveMatchException("Can't determine if feature flag is enabled or not with given properties"); + throw new InconclusiveMatchException("Can't determine if feature flag is enabled or not with given properties", unresolvedReason); } // We can only return False when either all conditions are False, or @@ -415,14 +512,65 @@ ConditionMatchResult EvaluateConditionMatch( var rolloutPercentage = condition.RolloutPercentage; if (condition.Properties is not null) { - if (condition.Properties.Select(property => property.Type switch + // 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. + UnresolvedFlagReason? inconclusiveReason = null; + RequiresServerEvaluationException? serverEvaluationException = null; + foreach (var property in condition.Properties) + { + bool isMatch; + try { - FilterType.Cohort => MatchCohort(property, distinctId, properties), - FilterType.Flag => MatchFlagDependencyFilter(property, distinctId, properties, evaluationCache, groups), - _ => MatchProperty(property, distinctId, properties) ?? false - }).Any(isMatch => !isMatch)) + if (property.Type is not (FilterType.Cohort or FilterType.Flag) && IsPropertyMissing(property, properties)) + { + // Same result as MatchProperty for an absent key, without the cost of throwing for every + // missing filter. + AssertFilterSupported(property); + inconclusiveReason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, UnresolvedFlagReason.MissingContext); + continue; + } + + isMatch = property.Type switch + { + FilterType.Cohort => MatchCohort(property, distinctId, properties), + FilterType.Flag => MatchFlagDependencyFilter(property, distinctId, properties, evaluationCache, groups), + _ => MatchProperty(property, distinctId, properties) ?? false + }; + } + 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 is null) + { + serverEvaluationException ??= e; + } + inconclusiveReason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, e.Reason); + continue; + } + catch (InconclusiveMatchException e) + { + inconclusiveReason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, e.Reason); + continue; + } + + if (!isMatch) + { + return ConditionMatchResult.NoMatch; + } + } + + if (inconclusiveReason is { } reason) { - return ConditionMatchResult.NoMatch; + // Throw the server exception again, so MatchFeatureFlagProperties stops at it and falls back to the + // server instead of trying later conditions. + if (serverEvaluationException is not null) + { + serverEvaluationException.Reason = reason; + throw serverEvaluationException; + } + throw new InconclusiveMatchException($"Can't determine if a condition of flag {flag.Key} matches", reason); } } @@ -484,7 +632,9 @@ bool MatchCohort( var cohortId = filter.Value?.CohortId; if (cohortId is null || !_cohortFilters.TryGetValue(cohortId.Value, out var conditions)) { - throw new RequiresServerEvaluationException($"cohort {cohortId} not found in local cohorts - likely a static cohort that requires server evaluation"); + throw new RequiresServerEvaluationException( + $"cohort {cohortId} not found in local cohorts - likely a static cohort that requires server evaluation", + UnresolvedFlagReason.UnsupportedDefinition); } return MatchPropertyGroup(conditions, distinctId, propertyValues); @@ -504,7 +654,7 @@ bool MatchPropertyGroup(FilterSet? filterSet, string distinctId, Dictionary value.IsSemverCaretMatch(overrideValue), ComparisonOperator.SemverWildcard => value.IsSemverWildcardMatch(overrideValue), null => true, // If no operator is specified, just return true. - _ => throw new InconclusiveMatchException($"Unknown operator: {propertyFilter.Operator}") + _ => throw new InconclusiveMatchException($"Unknown operator: {propertyFilter.Operator}", UnresolvedFlagReason.UnsupportedDefinition) }; } + static bool IsPropertyMissing(PropertyFilter propertyFilter, Dictionary? properties) + => properties is not null + && !string.Equals(propertyFilter.Key, PostHogProperties.DistinctId, StringComparison.Ordinal) + && !properties.ContainsKey(propertyFilter.Key); + + /// + /// Throws an with the + /// reason when local evaluation cannot use the filter, whatever the properties hold: an unknown operator, or a + /// filter value that the operator cannot parse. + /// + static void AssertFilterSupported(PropertyFilter propertyFilter) + { + if (propertyFilter.Operator is ComparisonOperator.IsSet or ComparisonOperator.IsNotSet) + { + return; + } + + if (propertyFilter.Value is not { } value) + { + throw new InconclusiveMatchException("The filter property value is null", UnresolvedFlagReason.UnsupportedDefinition); + } + + switch (propertyFilter.Operator) + { + case null + or ComparisonOperator.Exact + or ComparisonOperator.IsNot + or ComparisonOperator.GreaterThan + or ComparisonOperator.GreaterThanOrEquals + or ComparisonOperator.LessThan + or ComparisonOperator.LessThanOrEquals + or ComparisonOperator.ContainsIgnoreCase + or ComparisonOperator.DoesNotContainIgnoreCase + or ComparisonOperator.StartsWith + or ComparisonOperator.NotStartsWith + or ComparisonOperator.EndsWith + or ComparisonOperator.NotEndsWith: + return; + case ComparisonOperator.Regex or ComparisonOperator.NotRegex: + if (value.StringValue is { } pattern && !RegexHelpers.IsCompatiblePattern(pattern)) + { + throw new InconclusiveMatchException("Regex syntax requires server evaluation", UnresolvedFlagReason.UnsupportedDefinition); + } + return; + case ComparisonOperator.IsDateBefore or ComparisonOperator.IsDateAfter: + value.ParseFilterDate(); + return; + case ComparisonOperator.SemverEquals + or ComparisonOperator.SemverNotEquals + or ComparisonOperator.SemverGreaterThan + or ComparisonOperator.SemverGreaterThanOrEquals + or ComparisonOperator.SemverLessThan + or ComparisonOperator.SemverLessThanOrEquals + or ComparisonOperator.SemverTilde + or ComparisonOperator.SemverCaret: + value.ParseFilterSemver(); + return; + case ComparisonOperator.SemverWildcard: + value.ParseFilterWildcard(); + return; + default: + throw new InconclusiveMatchException($"Unknown operator: {propertyFilter.Operator}", UnresolvedFlagReason.UnsupportedDefinition); + } + } + /// /// Evaluates a flag dependency property filter. /// @@ -736,7 +959,9 @@ bool MatchFlagDependencyFilter( // Get the evaluated value for this flag var immediateDependencyValue = evaluationCache.TryGetValue(flagKey, out var keyForFlagDependencyFilter) ? keyForFlagDependencyFilter - : throw new InconclusiveMatchException($"Flag dependency '{flagKey}' is missing in evaluation cache"); + : throw new InconclusiveMatchException( + $"Flag dependency '{flagKey}' is missing in evaluation cache", + UnresolvedFlagReason.UnsupportedDefinition); // Match the value against expectations return MatchesDependencyValue(propertyValue, immediateDependencyValue); @@ -748,26 +973,32 @@ bool MatchFlagDependencyFilter( if (propertyFilter.Value is not { } propertyValue) { - throw new InconclusiveMatchException("The filter property value is null"); + throw new InconclusiveMatchException("The filter property value is null", UnresolvedFlagReason.UnsupportedDefinition); } // Check if dependency_chain is present - it should always be provided for flag dependencies if (propertyFilter.DependencyChain is null) { - throw new InconclusiveMatchException($"Flag dependency property for '{flagKey}' is missing required 'dependency_chain' field"); + throw new InconclusiveMatchException( + $"Flag dependency property for '{flagKey}' is missing required 'dependency_chain' field", + UnresolvedFlagReason.UnsupportedDefinition); } // Handle circular dependency (empty chain means circular) if (propertyFilter.DependencyChain.Count == 0) { // Circular dependencies should evaluate to inconclusive as per PostHog API design - throw new InconclusiveMatchException($"Flag dependency property for '{flagKey}' has an empty 'dependency_chain' field indicating a circular dependency"); + throw new InconclusiveMatchException( + $"Flag dependency property for '{flagKey}' has an empty 'dependency_chain' field indicating a circular dependency", + UnresolvedFlagReason.UnsupportedDefinition); } // The last item in the dependency chain should be the value in the current property filter. if (propertyFilter.DependencyChain[^1] != flagKey) { - throw new InconclusiveMatchException($"Flag dependency property for '{flagKey}' has an invalid 'dependency_chain' field - last item should be the flag key itself"); + throw new InconclusiveMatchException( + $"Flag dependency property for '{flagKey}' has an invalid 'dependency_chain' field - last item should be the flag key itself", + UnresolvedFlagReason.UnsupportedDefinition); } return (flagKey, propertyValue); @@ -780,6 +1011,17 @@ void EvaluateDependencyChain(IReadOnlyList dependencyChain, Dictionary> evaluationCache, GroupCollection? groups) { + // A dependency with no definition outranks one that is only inconclusive, so check the whole chain before + // evaluating any of it. + var missingDependency = dependencyChain.FirstOrDefault( + depFlagKey => !evaluationCache.ContainsKey(depFlagKey) && !_localFeatureFlags.ContainsKey(depFlagKey)); + if (missingDependency is not null) + { + throw new InconclusiveMatchException( + $"Cannot evaluate flag dependency '{missingDependency}' - flag not found in local flags", + UnresolvedFlagReason.UnsupportedDefinition); + } + // Evaluate all dependencies in the chain order and cache the results. foreach (var depFlagKey in dependencyChain) { @@ -788,24 +1030,18 @@ void EvaluateDependencyChain(IReadOnlyList dependencyChain, continue; } - evaluationCache[depFlagKey] = EvaluateSingleDependency(depFlagKey, distinctId, properties, evaluationCache, groups); + evaluationCache[depFlagKey] = EvaluateSingleDependency(_localFeatureFlags[depFlagKey], distinctId, properties, evaluationCache, groups); } } // Evaluates a single flag dependency and returns the result. StringOrValue EvaluateSingleDependency( - string depFlagKey, + LocalFeatureFlag depFlag, string distinctId, Dictionary? properties, Dictionary> evaluationCache, GroupCollection? groups) { - // Need to evaluate this dependency first - if (!_localFeatureFlags.TryGetValue(depFlagKey, out var depFlag)) - { - throw new InconclusiveMatchException($"Cannot evaluate flag dependency '{depFlagKey}' - flag not found in local flags"); - } - // Check if the dependency flag is active (same check as in ComputeFlagLocallyWithCache) if (!depFlag.Active) { @@ -826,8 +1062,19 @@ StringOrValue EvaluateSingleDependency( } catch (InconclusiveMatchException e) { - // If we can't evaluate a dependency, propagate the error - throw new InconclusiveMatchException($"Cannot evaluate flag dependency '{depFlagKey}': {e.Message}", e); + // If we can't evaluate a dependency, propagate the error. The dependency's own reason belongs to the + // dependency, not to the flag that depends on it. + throw new InconclusiveMatchException( + $"Cannot evaluate flag dependency '{depFlag.Key}': {e.Message}", + UnresolvedFlagReason.UnresolvedDependency, + e); + } + catch (RequiresServerEvaluationException e) + { + throw new InconclusiveMatchException( + $"Cannot evaluate flag dependency '{depFlag.Key}': {e.Message}", + UnresolvedFlagReason.UnresolvedDependency, + e); } } diff --git a/src/PostHog/Features/LocalFeatureFlagsLoader.cs b/src/PostHog/Features/LocalFeatureFlagsLoader.cs index 8415586f..9d393ae1 100644 --- a/src/PostHog/Features/LocalFeatureFlagsLoader.cs +++ b/src/PostHog/Features/LocalFeatureFlagsLoader.cs @@ -33,6 +33,9 @@ internal sealed class LocalFeatureFlagsLoader( readonly PeriodicTimer _timer = new(options.Value.FeatureFlagPollInterval, timeProvider); readonly ILogger _logger = loggerFactory.CreateLogger(); readonly ILogger _localEvaluatorLogger = loggerFactory.CreateLogger(); + readonly object _warnedUnresolvableFlagsLock = new(); + // The definition and reason of each flag already warned about as never locally resolvable. + Dictionary _warnedUnresolvableFlags = new(StringComparer.Ordinal); void StartPollingIfNotStarted() { @@ -115,9 +118,58 @@ void StartPollingIfNotStarted() var localEvaluator = new LocalEvaluator(response.Result, timeProvider, _localEvaluatorLogger); Interlocked.Exchange(ref _localEvaluator, localEvaluator); Interlocked.Exchange(ref _flagDefinitionsLoadedAtMs, timeProvider.GetUtcNow().ToUnixTimeMilliseconds()); + WarnAboutUnresolvableFlags(localEvaluator); return localEvaluator; } + // Warns once about each flag whose definition local evaluation can never resolve, until its definition changes. + void WarnAboutUnresolvableFlags(LocalEvaluator localEvaluator) + { + List? experienceContinuityFlags = null; + List? unsupportedDefinitionFlags = null; + lock (_warnedUnresolvableFlagsLock) + { + // Rebuilt from the current definitions, so a flag that is deleted or becomes resolvable is forgotten + // and warns again if it later returns with an unresolvable definition. + var warnedFlags = new Dictionary(StringComparer.Ordinal); + foreach (var flag in localEvaluator.LocalEvaluationApiResult.Flags) + { + if (localEvaluator.GetDefinitionUnresolvedReason(flag) is not { } reason) + { + continue; + } + + warnedFlags[flag.Key] = (flag, reason); + if (_warnedUnresolvableFlags.TryGetValue(flag.Key, out var warned) + && warned.Reason == reason + && warned.Definition.Equals(flag)) + { + continue; + } + + if (reason is UnresolvedFlagReason.ExperienceContinuity) + { + (experienceContinuityFlags ??= []).Add(flag.Key); + } + else + { + (unsupportedDefinitionFlags ??= []).Add(flag.Key); + } + } + _warnedUnresolvableFlags = warnedFlags; + } + + if (experienceContinuityFlags is not null) + { + _logger.LogWarningExperienceContinuityFlags(experienceContinuityFlags.Count, string.Join(", ", experienceContinuityFlags)); + } + + if (unsupportedDefinitionFlags is not null) + { + _logger.LogWarningUnsupportedDefinitionFlags(unsupportedDefinitionFlags.Count, string.Join(", ", unsupportedDefinitionFlags)); + } + } + async Task PollForFeatureFlagsAsync(CancellationToken cancellationToken) { try @@ -214,4 +266,30 @@ public static partial void LogTraceOperationCancelled( Level = LogLevel.Warning, Message = "[FEATURE FLAGS] Quota exceeded, resetting feature flag data. Learn more about billing limits at https://posthog.com/docs/billing/limits-alerts")] public static partial void LogWarningQuotaExceeded(this ILogger logger, Exception e); + + [LoggerMessage( + EventId = 502, + Level = LogLevel.Warning, + Message = "[FEATURE FLAGS] You are using local evaluation but {Count} flag(s) have experience continuity enabled " + + "and cannot be resolved locally (reason: experience_continuity): {FlagKeys}. Evaluating these flags " + + "makes a server request, or leaves them unresolved when OnlyEvaluateLocally is true. To evaluate them " + + "locally, disable experience continuity on these flags in PostHog.")] + public static partial void LogWarningExperienceContinuityFlags( + this ILogger logger, + int count, + string flagKeys); + + [LoggerMessage( + EventId = 503, + Level = LogLevel.Warning, + Message = "[FEATURE FLAGS] You are using local evaluation but {Count} flag(s) use something local evaluation " + + "does not support, such as a static cohort, an unknown operator, or a missing or circular flag " + + "dependency (reason: unsupported_definition): {FlagKeys}. Evaluating these flags falls back to a " + + "server request when evaluation reaches that part, or leaves them unresolved when " + + "OnlyEvaluateLocally is true. To evaluate them locally, change these flags in PostHog or upgrade " + + "the PostHog .NET SDK.")] + public static partial void LogWarningUnsupportedDefinitionFlags( + this ILogger logger, + int count, + string flagKeys); } \ No newline at end of file diff --git a/src/PostHog/Features/UnresolvedFlagReason.cs b/src/PostHog/Features/UnresolvedFlagReason.cs new file mode 100644 index 00000000..eeba8569 --- /dev/null +++ b/src/PostHog/Features/UnresolvedFlagReason.cs @@ -0,0 +1,70 @@ +using System.Text.Json.Serialization; +using PostHog.Json; +// System.Text.Json 9+ ships its own JsonStringEnumMemberNameAttribute. Bind to PostHog's, +// which JsonStringEnumMemberNameJsonConverter reads and which is shipped public API. +using JsonStringEnumMemberName = PostHog.Json.JsonStringEnumMemberNameAttribute; + +namespace PostHog.Features; + +/// +/// Why local evaluation could not resolve a flag that has a loaded definition. See +/// . +/// +/// +/// When more than one cause applies to a flag, the first reason in declaration order is reported. +/// +[JsonConverter(typeof(JsonStringEnumMemberNameJsonConverter))] +public enum UnresolvedFlagReason +{ + /// + /// The flag has experience continuity enabled. Local evaluation never resolves it; to fix it, change the flag. + /// + [JsonStringEnumMemberName("experience_continuity")] + ExperienceContinuity, + + /// + /// The flag uses something the local evaluator does not support, such as a static cohort, an unknown + /// operator, a malformed value, a dependency with no loaded definition, or a circular dependency. To fix it, + /// change the flag or upgrade the SDK. + /// + [JsonStringEnumMemberName("unsupported_definition")] + UnsupportedDefinition, + + /// + /// A flag this flag depends on has a loaded definition but was unresolved. + /// + [JsonStringEnumMemberName("unresolved_dependency")] + UnresolvedDependency, + + /// + /// The call did not pass a property the flag needs, or passed it in a form the evaluator cannot use. To fix + /// it, pass the property. + /// + [JsonStringEnumMemberName("missing_context")] + MissingContext +} + +/// +/// Extensions for . +/// +public static class UnresolvedFlagReasonExtensions +{ + /// + /// Returns the stable string for the reason that every PostHog SDK uses, for example + /// experience_continuity. + /// + /// The reason. + /// The stable string for . + public static string ToValueString(this UnresolvedFlagReason reason) => reason switch + { + UnresolvedFlagReason.ExperienceContinuity => "experience_continuity", + UnresolvedFlagReason.UnsupportedDefinition => "unsupported_definition", + UnresolvedFlagReason.UnresolvedDependency => "unresolved_dependency", + UnresolvedFlagReason.MissingContext => "missing_context", + _ => throw new ArgumentOutOfRangeException(nameof(reason), reason, null) + }; + + // The enum is declared in precedence order, so the lower value wins. + internal static UnresolvedFlagReason Pick(UnresolvedFlagReason? current, UnresolvedFlagReason next) + => current is { } existing && existing < next ? existing : next; +} diff --git a/src/PostHog/IPostHogClient.cs b/src/PostHog/IPostHogClient.cs index 5598a291..42e24e08 100644 --- a/src/PostHog/IPostHogClient.cs +++ b/src/PostHog/IPostHogClient.cs @@ -253,7 +253,9 @@ Task> GetAllFeatureFlagsAsync( /// /flags request, and the returned snapshot. Requested keys missing from local definitions /// trigger remote fallback unless is /// true. - /// filters an already-evaluated snapshot in memory. + /// filters an already-evaluated snapshot in memory. Flags with a loaded local definition that neither local + /// evaluation nor the fallback resolved are absent from the snapshot and listed with a reason in + /// . /// Task EvaluateFlagsAsync( string distinctId, diff --git a/src/PostHog/Json/InconclusiveMatchException.cs b/src/PostHog/Json/InconclusiveMatchException.cs index d9e722d9..a0bec54e 100644 --- a/src/PostHog/Json/InconclusiveMatchException.cs +++ b/src/PostHog/Json/InconclusiveMatchException.cs @@ -1,3 +1,5 @@ +using PostHog.Features; + namespace PostHog.Json; /// @@ -29,4 +31,20 @@ public InconclusiveMatchException() public InconclusiveMatchException(string message, Exception innerException) : base(message, innerException) { } + + internal InconclusiveMatchException(string message, UnresolvedFlagReason reason) : base(message) + { + Reason = reason; + } + + internal InconclusiveMatchException(string message, UnresolvedFlagReason reason, Exception innerException) + : base(message, innerException) + { + Reason = reason; + } + + /// + /// Why local evaluation could not resolve the flag. + /// + internal UnresolvedFlagReason Reason { get; } = UnresolvedFlagReason.UnsupportedDefinition; } \ No newline at end of file diff --git a/src/PostHog/Json/PropertyFilterValue.cs b/src/PostHog/Json/PropertyFilterValue.cs index 33f52f97..3abea05a 100644 --- a/src/PostHog/Json/PropertyFilterValue.cs +++ b/src/PostHog/Json/PropertyFilterValue.cs @@ -7,6 +7,7 @@ using System.Text.Json.Serialization; using System.Text.RegularExpressions; using PostHog.Api; +using PostHog.Features; using PostHog.Library; using static PostHog.Library.Ensure; using static PostHog.Library.SemanticVersion; @@ -918,40 +919,44 @@ string stringOverride when bool.TryParse(stringOverride, out var boolValue) => B public bool IsDateBefore(object? overrideValue, DateTimeOffset now) { // Question: Should we support DateOnly and TimeOnly? - var overrideDate = ParseDate(overrideValue); + // Parse the filter first, so a bad filter is reported even when the override value is bad too. + var comparand = ParseFilterDate(); + var overrideDate = ParseDate(overrideValue, UnresolvedFlagReason.MissingContext); - if (RelativeDate.TryParseRelativeDate(StringValue, out var relativeDate)) - { - return overrideDate is DateTimeOffset overrideDateTimeOffset && relativeDate.IsDateBefore(overrideDateTimeOffset, now) - || (overrideDate is DateTime overrideDateTime && relativeDate.IsDateBefore(overrideDateTime, now)); - } - - return ParseDate(StringValue) switch + return comparand switch { + RelativeDate relativeDate => overrideDate is DateTimeOffset overrideDateTimeOffset && relativeDate.IsDateBefore(overrideDateTimeOffset, now) + || (overrideDate is DateTime overrideDateTime && relativeDate.IsDateBefore(overrideDateTime, now)), DateTimeOffset comparandDate => overrideDate is DateTimeOffset overrideDateTimeOffset && overrideDateTimeOffset < comparandDate, DateTime comparandDate => overrideDate is DateTimeOffset overrideDateTimeOffset && overrideDateTimeOffset < comparandDate, - _ => throw new InconclusiveMatchException("The date provided is not a valid format") + _ => throw new InconclusiveMatchException("The date provided is not a valid format", UnresolvedFlagReason.UnsupportedDefinition) }; + } - object? ParseDate(object? value) - { - if (value is DateTime or DateTimeOffset + // Returns a RelativeDate, or the absolute date the filter value names. + internal object? ParseFilterDate() + => RelativeDate.TryParseRelativeDate(StringValue, out var relativeDate) + ? relativeDate + : ParseDate(StringValue, UnresolvedFlagReason.UnsupportedDefinition); + + static object? ParseDate(object? value, UnresolvedFlagReason reason) + { + if (value is DateTime or DateTimeOffset #if NET5_0_OR_GREATER - or DateOnly + or DateOnly #endif ) - { - return value; - } - - return value is string dateString - ? DateTimeOffset.TryParse(dateString, out var dto) - ? dto - : DateTime.TryParse(dateString, out var dt) - ? dt - : throw new InconclusiveMatchException("The date provided is not a valid format") - : throw new InconclusiveMatchException("The date provided must be a string, DateTime, or DateTimeOffset, object"); + { + return value; } + + return value is string dateString + ? DateTimeOffset.TryParse(dateString, out var dto) + ? dto + : DateTime.TryParse(dateString, out var dt) + ? dt + : throw new InconclusiveMatchException("The date provided is not a valid format", reason) + : throw new InconclusiveMatchException("The date provided must be a string, DateTime, or DateTimeOffset, object", reason); } /// @@ -991,20 +996,35 @@ static SemanticVersion ParseOverrideSemver(object? overrideValue) var overrideVersionString = ToInvariantString(overrideValue); if (!SemanticVersion.TryParse(overrideVersionString, out var version)) { - throw new InconclusiveMatchException($"Cannot parse override value '{overrideVersionString}' as a semantic version"); + throw new InconclusiveMatchException( + $"Cannot parse override value '{overrideVersionString}' as a semantic version", + UnresolvedFlagReason.MissingContext); } return version.Value; } - SemanticVersion ParseFilterSemver() + internal SemanticVersion ParseFilterSemver() { if (!SemanticVersion.TryParse(StringValue, out var version)) { - throw new InconclusiveMatchException($"Cannot parse filter value '{StringValue}' as a semantic version"); + throw new InconclusiveMatchException( + $"Cannot parse filter value '{StringValue}' as a semantic version", + UnresolvedFlagReason.UnsupportedDefinition); } return version.Value; } + internal (SemanticVersion Lower, SemanticVersion Upper) ParseFilterWildcard() + { + if (!TryParseWildcard(StringValue, out var lower, out var upper)) + { + throw new InconclusiveMatchException( + $"Cannot parse filter value '{StringValue}' as a wildcard pattern", + UnresolvedFlagReason.UnsupportedDefinition); + } + return (lower.Value, upper.Value); + } + /// /// Compares the override value as a semantic version against this filter value. /// @@ -1013,9 +1033,8 @@ SemanticVersion ParseFilterSemver() /// Thrown if either value cannot be parsed as a valid semver. public int CompareSemver(object? overrideValue) { - var overrideVersion = ParseOverrideSemver(overrideValue); var filterVersion = ParseFilterSemver(); - return overrideVersion.CompareTo(filterVersion); + return ParseOverrideSemver(overrideValue).CompareTo(filterVersion); } /// @@ -1027,10 +1046,8 @@ public int CompareSemver(object? overrideValue) /// Thrown if either value cannot be parsed as a valid semver. public bool IsSemverTildeMatch(object? overrideValue) { - var overrideVersion = ParseOverrideSemver(overrideValue); - var filterVersion = ParseFilterSemver(); - var (lower, upper) = filterVersion.GetTildeBounds(); - return overrideVersion.IsInRange(lower, upper); + var (lower, upper) = ParseFilterSemver().GetTildeBounds(); + return ParseOverrideSemver(overrideValue).IsInRange(lower, upper); } /// @@ -1045,10 +1062,8 @@ public bool IsSemverTildeMatch(object? overrideValue) /// Thrown if either value cannot be parsed as a valid semver. public bool IsSemverCaretMatch(object? overrideValue) { - var overrideVersion = ParseOverrideSemver(overrideValue); - var filterVersion = ParseFilterSemver(); - var (lower, upper) = filterVersion.GetCaretBounds(); - return overrideVersion.IsInRange(lower, upper); + var (lower, upper) = ParseFilterSemver().GetCaretBounds(); + return ParseOverrideSemver(overrideValue).IsInRange(lower, upper); } /// @@ -1061,14 +1076,8 @@ public bool IsSemverCaretMatch(object? overrideValue) /// Thrown if either value cannot be parsed. public bool IsSemverWildcardMatch(object? overrideValue) { - var overrideVersion = ParseOverrideSemver(overrideValue); - - if (!TryParseWildcard(StringValue, out var lower, out var upper)) - { - throw new InconclusiveMatchException($"Cannot parse filter value '{StringValue}' as a wildcard pattern"); - } - - return overrideVersion.IsInRange(lower.Value, upper.Value); + var (lower, upper) = ParseFilterWildcard(); + return ParseOverrideSemver(overrideValue).IsInRange(lower, upper); } /// diff --git a/src/PostHog/Library/RegexHelpers.cs b/src/PostHog/Library/RegexHelpers.cs index 88d84237..fbabe452 100644 --- a/src/PostHog/Library/RegexHelpers.cs +++ b/src/PostHog/Library/RegexHelpers.cs @@ -1,6 +1,7 @@ using System.Diagnostics.CodeAnalysis; using System.Text; using System.Text.RegularExpressions; +using PostHog.Features; using PostHog.Json; namespace PostHog.Library; @@ -44,7 +45,9 @@ public static bool TryValidateRegex( } var compatiblePattern = GetCompatiblePattern(pattern) - ?? throw new InconclusiveMatchException("Regex syntax requires server evaluation"); + ?? throw new InconclusiveMatchException( + "Regex syntax requires server evaluation", + UnresolvedFlagReason.UnsupportedDefinition); Regex regex; try { @@ -58,7 +61,9 @@ public static bool TryValidateRegex( // .NET matches UTF-16 code units and its Unicode classes differ from the service's. if (input.Any(character => character > 0x7f)) { - throw new InconclusiveMatchException("Unicode regex matching requires server evaluation"); + throw new InconclusiveMatchException( + "Unicode regex matching requires server evaluation", + UnresolvedFlagReason.UnsupportedDefinition); } try @@ -67,10 +72,15 @@ public static bool TryValidateRegex( } catch (RegexMatchTimeoutException) { - throw new InconclusiveMatchException("Regex execution requires server evaluation"); + throw new InconclusiveMatchException( + "Regex execution requires server evaluation", + UnresolvedFlagReason.UnsupportedDefinition); } } + public static bool IsCompatiblePattern(string pattern) + => pattern.Length == 0 || GetCompatiblePattern(pattern) is not null; + // Successful .NET compilation alone does not establish compatibility with the service's regex syntax. static string? GetCompatiblePattern(string pattern) { diff --git a/src/PostHog/NoOpPostHogClient.cs b/src/PostHog/NoOpPostHogClient.cs index f939ca0a..f479c5dd 100644 --- a/src/PostHog/NoOpPostHogClient.cs +++ b/src/PostHog/NoOpPostHogClient.cs @@ -130,7 +130,8 @@ void IFeatureFlagEvaluationsHost.CaptureFeatureFlagCalled( string? requestId, long? evaluatedAt, long? flagDefinitionsLoadedAt, - IReadOnlyCollection errors) + IReadOnlyCollection errors, + bool unresolved) { } diff --git a/src/PostHog/PostHogClient.cs b/src/PostHog/PostHogClient.cs index b234d413..719c90f6 100644 --- a/src/PostHog/PostHogClient.cs +++ b/src/PostHog/PostHogClient.cs @@ -1007,14 +1007,15 @@ public void CaptureFeatureFlagCalled( string? requestId, long? evaluatedAt, long? flagDefinitionsLoadedAt, - IReadOnlyCollection errors) + IReadOnlyCollection errors, + bool unresolved) { // Mirror the legacy path's "missing flag" handling: append the FlagMissing error // and use a synthetic disabled FeatureFlag so the response shape is consistent. var snapshotErrors = new List(errors); if (record is null) { - snapshotErrors.Add(FeatureFlagError.FlagMissing); + snapshotErrors.Add(unresolved ? FeatureFlagError.LocalEvaluationInconclusive : FeatureFlagError.FlagMissing); } var flag = record?.Flag ?? new FeatureFlag { Key = featureKey, IsEnabled = false }; @@ -1095,6 +1096,7 @@ public async Task EvaluateFlagsAsync( string? requestId = null; long? evaluatedAt = null; long? flagDefinitionsLoadedAt = null; + IReadOnlyDictionary? locallyUnresolved = null; // 1. Local pass. var fallbackToRemote = true; @@ -1106,12 +1108,13 @@ public async Task EvaluateFlagsAsync( await _featureFlagsLoader.GetFeatureFlagsForLocalEvaluationAsync(cancellationToken); if (localEvaluator is not null) { - var (locallyEvaluated, needsRemote) = localEvaluator.EvaluateAllFlags( + var (locallyEvaluated, needsRemote, unresolved) = localEvaluator.EvaluateAllFlags( resolvedDistinctId, options?.Groups, options?.PersonProperties, warnOnUnknownGroups: false, flagKeysToEvaluate: requestedFlagKeys); + locallyUnresolved = unresolved; foreach (var (key, flag) in locallyEvaluated) { @@ -1202,6 +1205,11 @@ and not NullReferenceException } } + // A flag the remote fallback resolved has a value, so it is not unresolved. + var unresolvedFlags = locallyUnresolved? + .Where(pair => !records.ContainsKey(pair.Key)) + .ToDictionary(pair => pair.Key, pair => pair.Value, StringComparer.Ordinal); + return new FeatureFlagEvaluations( _evaluationsHost, resolvedDistinctId, @@ -1210,7 +1218,8 @@ and not NullReferenceException evaluatedAt, flagDefinitionsLoadedAt, options?.Groups, - errors); + errors, + unresolvedFlags: unresolvedFlags); static EvaluatedFlagRecord ToRecord(string key, FeatureFlag flag, bool locallyEvaluated, bool minimalFlagCalledEvents) => new() @@ -1256,7 +1265,7 @@ public async Task> GetAllFeatureFlagsAs await _featureFlagsLoader.GetFeatureFlagsForLocalEvaluationAsync(cancellationToken); if (localEvaluator is not null) { - var (localEvaluationResults, fallbackToRemote) = localEvaluator.EvaluateAllFlags( + var (localEvaluationResults, fallbackToRemote, _) = localEvaluator.EvaluateAllFlags( resolvedDistinctId, options?.Groups, options?.PersonProperties, diff --git a/src/PostHog/PublicAPI.Unshipped.txt b/src/PostHog/PublicAPI.Unshipped.txt index b1c7c623..ba62c673 100644 --- a/src/PostHog/PublicAPI.Unshipped.txt +++ b/src/PostHog/PublicAPI.Unshipped.txt @@ -11,6 +11,13 @@ PostHog.Api.FlagsResult.MinimalFlagCalledEvents.init -> void PostHog.Features.FeatureFlag.HasExperiment.get -> bool? PostHog.Features.FeatureFlag.HasExperiment.init -> void PostHog.Features.FeatureFlagEvaluations.IsEnabled(string! key, bool defaultValue) -> bool +PostHog.Features.FeatureFlagEvaluations.UnresolvedFlags.get -> System.Collections.Generic.IReadOnlyDictionary! +PostHog.Features.UnresolvedFlagReason +PostHog.Features.UnresolvedFlagReason.ExperienceContinuity = 0 -> PostHog.Features.UnresolvedFlagReason +PostHog.Features.UnresolvedFlagReason.MissingContext = 3 -> PostHog.Features.UnresolvedFlagReason +PostHog.Features.UnresolvedFlagReason.UnresolvedDependency = 2 -> PostHog.Features.UnresolvedFlagReason +PostHog.Features.UnresolvedFlagReason.UnsupportedDefinition = 1 -> PostHog.Features.UnresolvedFlagReason +PostHog.Features.UnresolvedFlagReasonExtensions PostHog.Json.PropertyFilterValue.IsPrefixOf(object? other, System.StringComparison stringComparison) -> bool PostHog.Json.PropertyFilterValue.IsSuffixOf(object? other, System.StringComparison stringComparison) -> bool PostHog.PostHogOptions.BeforeSend.get -> System.Func? @@ -19,3 +26,4 @@ PostHog.PostHogOptions.FeatureFlagRequestMaxRetries.get -> int PostHog.PostHogOptions.FeatureFlagRequestMaxRetries.set -> void PostHog.PostHogOptions.SecretKey.get -> string? PostHog.PostHogOptions.SecretKey.set -> void +static PostHog.Features.UnresolvedFlagReasonExtensions.ToValueString(this PostHog.Features.UnresolvedFlagReason reason) -> string! diff --git a/tests/UnitTests/Features/FeatureFlagEvaluationsTests.cs b/tests/UnitTests/Features/FeatureFlagEvaluationsTests.cs index a6ff6490..52ce41d9 100644 --- a/tests/UnitTests/Features/FeatureFlagEvaluationsTests.cs +++ b/tests/UnitTests/Features/FeatureFlagEvaluationsTests.cs @@ -982,3 +982,334 @@ public async Task CaptureExceptionAttachesFeatureFlagsFromSnapshot() Assert.Equal("variant-x", props.GetProperty("$feature/flag-b").GetString()); } } + +public class TheUnresolvedFlagsProperty +{ + const string BetaUiResolvesTrue = """ + {"id": 1, "key": "beta-ui", "active": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """; + + const string CheckoutWithExperienceContinuity = """ + {"id": 2, "key": "checkout", "active": true, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """; + + const string BetaUiDependsOnCheckout = """ + {"id": 1, "key": "beta-ui", "active": true, + "filters": {"groups": [{"properties": [ + {"key": "checkout", "type": "flag", "value": true, "operator": "flag_evaluates_to", + "dependency_chain": ["checkout"]}], + "rollout_percentage": 100}]}} + """; + + const string PlanIsPro = """{"key": "plan", "type": "person", "value": "pro", "operator": "exact"}"""; + + const string StaticCohort = """{"key": "id", "type": "cohort", "value": 999}"""; + + static readonly AllFeatureFlagsOptions LocalOnly = new() { OnlyEvaluateLocally = true }; + + static string CheckoutWithFilters(params string[] filters) => $$$""" + {"id": 2, "key": "checkout", "active": true, + "filters": {"groups": [{"properties": [{{{string.Join(", ", filters)}}}], "rollout_percentage": 100}]}} + """; + + static TestContainer CreateContainer(params string[] flags) + { + var container = new TestContainer(personalApiKey: "fake-personal-api-key"); + // Served verbatim: the typed overload cannot serialize a boolean flag dependency value. + container.FakeHttpMessageHandler.AddLocalEvaluationResponseWithETag( + $$$"""{"flags": [{{{string.Join(", ", flags)}}}], "cohorts": {}}""", + "\"definitions\""); + return container; + } + + static async Task GetFeatureFlagCalledEventsAsync( + PostHogClient client, + FakeHttpMessageHandler.RequestHandler batchHandler) + { + await client.FlushAsync(); + if (batchHandler.ReceivedRequests.Count == 0) + { + return []; + } + using var doc = JsonDocument.Parse(batchHandler.GetReceivedRequestBody(indented: false)); + return doc.RootElement.GetProperty("batch").EnumerateArray() + .Where(e => e.GetProperty("event").GetString() == "$feature_flag_called") + .Select(e => e.GetProperty("properties").Clone()) + .ToArray(); + } + + static async Task GetFeatureFlagErrorAsync( + PostHogClient client, + FakeHttpMessageHandler.RequestHandler batchHandler, + string key) + { + var properties = (await GetFeatureFlagCalledEventsAsync(client, batchHandler)) + .Single(p => p.GetProperty("$feature_flag").GetString() == key); + return properties.TryGetProperty("$feature_flag_error", out var error) ? error.GetString() : null; + } + + [Fact] + public async Task LocalOnlyEvaluationReportsExperienceContinuityFlagAsUnresolved() + { + var container = CreateContainer(BetaUiResolvesTrue, CheckoutWithExperienceContinuity); + var flagsHandler = container.FakeHttpMessageHandler.AddFlagsResponse("""{"featureFlags": {}}"""); + var batchHandler = container.FakeHttpMessageHandler.AddBatchResponse(); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync("user-123", LocalOnly, CancellationToken.None); + + Assert.True(snapshot.Records["beta-ui"].Enabled); + Assert.DoesNotContain("checkout", snapshot.Keys); + var unresolved = Assert.Single(snapshot.UnresolvedFlags); + Assert.Equal("checkout", unresolved.Key); + Assert.Equal(UnresolvedFlagReason.ExperienceContinuity, unresolved.Value); + Assert.Equal("experience_continuity", unresolved.Value.ToValueString()); + Assert.Empty(flagsHandler.ReceivedRequests); + Assert.Empty(await GetFeatureFlagCalledEventsAsync(client, batchHandler)); + Assert.Empty(snapshot.OnlyAccessed().UnresolvedFlags); + Assert.Empty(snapshot.Only("beta-ui", "checkout").UnresolvedFlags); + } + + public static IEnumerable InconclusiveCauses() + { + yield return [CheckoutWithFilters(PlanIsPro), null!, UnresolvedFlagReason.MissingContext]; + yield return [ + CheckoutWithFilters("""{"key": "plan", "type": "person", "value": "pro", "operator": "operator_from_the_future"}"""), + "enterprise", + UnresolvedFlagReason.UnsupportedDefinition]; + yield return [CheckoutWithFilters(StaticCohort), null!, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [CheckoutWithFilters(StaticCohort, PlanIsPro), null!, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [CheckoutWithFilters(PlanIsPro, StaticCohort), null!, UnresolvedFlagReason.UnsupportedDefinition]; + } + + [Theory] + [MemberData(nameof(InconclusiveCauses))] + public async Task EachInconclusiveCauseReportsItsUnresolvedReason( + string checkoutDefinition, + string? plan, + UnresolvedFlagReason expectedReason) + { + var container = CreateContainer(checkoutDefinition); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync( + "user-123", + new AllFeatureFlagsOptions + { + OnlyEvaluateLocally = true, + PersonProperties = plan is null ? [] : new() { ["plan"] = plan } + }, + CancellationToken.None); + + Assert.DoesNotContain("checkout", snapshot.Keys); + Assert.Equal(expectedReason, snapshot.UnresolvedFlags["checkout"]); + } + + [Fact] + public async Task FlagBlockedByUnresolvedDependencyReportsTheDependency() + { + var container = CreateContainer(CheckoutWithExperienceContinuity, BetaUiDependsOnCheckout); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync( + "user-123", + new AllFeatureFlagsOptions { OnlyEvaluateLocally = true, FlagKeysToEvaluate = ["beta-ui"] }, + CancellationToken.None); + + Assert.DoesNotContain("beta-ui", snapshot.Keys); + Assert.Equal(UnresolvedFlagReason.UnresolvedDependency, snapshot.UnresolvedFlags["beta-ui"]); + Assert.False(snapshot.UnresolvedFlags.ContainsKey("checkout")); + } + + [Fact] + public async Task FlagThatDependsOnFlagWithNoLoadedDefinitionReportsUnsupportedDefinition() + { + var container = CreateContainer(BetaUiDependsOnCheckout); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync( + "user-123", + new AllFeatureFlagsOptions { OnlyEvaluateLocally = true, FlagKeysToEvaluate = ["beta-ui"] }, + CancellationToken.None); + + Assert.DoesNotContain("beta-ui", snapshot.Keys); + Assert.Equal(UnresolvedFlagReason.UnsupportedDefinition, snapshot.UnresolvedFlags["beta-ui"]); + Assert.False(snapshot.UnresolvedFlags.ContainsKey("checkout")); + } + + [Fact] + public async Task FlagResolvedByRemoteFallbackIsNotUnresolvedAndReportsNoError() + { + var container = CreateContainer(CheckoutWithExperienceContinuity); + container.FakeHttpMessageHandler.AddFlagsResponse("""{"featureFlags": {"checkout": true}}"""); + var batchHandler = container.FakeHttpMessageHandler.AddBatchResponse(); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync("user-123", options: null, CancellationToken.None); + + Assert.Empty(snapshot.UnresolvedFlags); + Assert.True(snapshot.IsEnabled("checkout")); + var properties = Assert.Single(await GetFeatureFlagCalledEventsAsync(client, batchHandler)); + Assert.Equal("checkout", properties.GetProperty("$feature_flag").GetString()); + Assert.True(properties.GetProperty("$feature_flag_response").GetBoolean()); + Assert.False(properties.TryGetProperty("$feature_flag_error", out _)); + } + + [Theory] + [InlineData(null, "local_evaluation_inconclusive")] + [InlineData("""{"featureFlags": {}}""", "local_evaluation_inconclusive")] + [InlineData("""{"featureFlags": {}, "errorsWhileComputingFlags": true}""", "errors_while_computing_flags,local_evaluation_inconclusive")] + public async Task RemoteFallbackThatDoesNotResolveTheFlagLeavesItUnresolved(string? flagsResponse, string expectedError) + { + var container = new TestContainer(services => services.Configure(options => + { + options.SecretKey = "fake-personal-api-key"; + options.FeatureFlagRequestMaxRetries = 0; + })); + container.FakeHttpMessageHandler.AddLocalEvaluationResponse( + $$"""{"flags": [{{CheckoutWithExperienceContinuity}}]}"""); + var flagsHandler = flagsResponse is null + ? container.FakeHttpMessageHandler.AddFlagsResponseException(new HttpRequestException("Network error")) + : container.FakeHttpMessageHandler.AddFlagsResponse(flagsResponse); + var batchHandler = container.FakeHttpMessageHandler.AddBatchResponse(); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync("user-123", options: null, CancellationToken.None); + + Assert.Single(flagsHandler.ReceivedRequests); + Assert.DoesNotContain("checkout", snapshot.Keys); + Assert.Equal(UnresolvedFlagReason.ExperienceContinuity, snapshot.UnresolvedFlags["checkout"]); + Assert.False(snapshot.IsEnabled("checkout")); + Assert.Equal( + flagsResponse is null ? "unknown_error,local_evaluation_inconclusive" : expectedError, + await GetFeatureFlagErrorAsync(client, batchHandler, "checkout")); + } + + [Fact] + public async Task ReadingUnresolvedFlagReportsInconclusiveLocalEvaluation() + { + var container = CreateContainer(CheckoutWithExperienceContinuity); + var batchHandler = container.FakeHttpMessageHandler.AddBatchResponse(); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync("user-123", LocalOnly, CancellationToken.None); + + Assert.False(snapshot.IsEnabled("checkout")); + Assert.Equal("local_evaluation_inconclusive", await GetFeatureFlagErrorAsync(client, batchHandler, "checkout")); + } + + [Fact] + public async Task FilteredSnapshotReportsUnresolvedKeyAsMissing() + { + var container = CreateContainer(BetaUiResolvesTrue, CheckoutWithExperienceContinuity); + var batchHandler = container.FakeHttpMessageHandler.AddBatchResponse(); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync("user-123", LocalOnly, CancellationToken.None); + var filtered = snapshot.Only("beta-ui", "checkout"); + + Assert.Empty(filtered.UnresolvedFlags); + Assert.False(filtered.IsEnabled("checkout")); + Assert.Equal("flag_missing", await GetFeatureFlagErrorAsync(client, batchHandler, "checkout")); + } + + [Fact] + public async Task KeyWithoutLocalDefinitionIsMissingNotUnresolved() + { + var container = CreateContainer(BetaUiResolvesTrue); + container.FakeHttpMessageHandler.AddFlagsResponse("""{"featureFlags": {}}"""); + var batchHandler = container.FakeHttpMessageHandler.AddBatchResponse(); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync( + "user-123", + new AllFeatureFlagsOptions { FlagKeysToEvaluate = ["beta-ui", "typo-flag"] }, + CancellationToken.None); + snapshot.IsEnabled("typo-flag"); + + Assert.Empty(snapshot.UnresolvedFlags); + Assert.Equal("flag_missing", await GetFeatureFlagErrorAsync(client, batchHandler, "typo-flag")); + } + + [Fact] + public async Task FlagOutsideTheRequestedKeysIsNotUnresolved() + { + var container = CreateContainer(BetaUiResolvesTrue, CheckoutWithExperienceContinuity); + var flagsHandler = container.FakeHttpMessageHandler.AddFlagsResponse("""{"featureFlags": {}}"""); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync( + "user-123", + new AllFeatureFlagsOptions { FlagKeysToEvaluate = ["beta-ui"] }, + CancellationToken.None); + + Assert.True(snapshot.Records["beta-ui"].Enabled); + Assert.Empty(snapshot.UnresolvedFlags); + Assert.Empty(flagsHandler.ReceivedRequests); + } + + [Fact] + public async Task InactiveFlagIsResolvedAsFalseNotUnresolved() + { + var container = CreateContainer(""" + {"id": 2, "key": "checkout", "active": false, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync("user-123", LocalOnly, CancellationToken.None); + + Assert.False(snapshot.Records["checkout"].Enabled); + Assert.Empty(snapshot.UnresolvedFlags); + } + + [Fact] + public async Task UnexpectedEvaluationErrorIsMissingNotUnresolved() + { + var container = CreateContainer(CheckoutWithFilters(PlanIsPro)); + var batchHandler = container.FakeHttpMessageHandler.AddBatchResponse(); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync( + "user-123", + new AllFeatureFlagsOptions + { + OnlyEvaluateLocally = true, + PersonProperties = new() { ["plan"] = new ThrowingValue() } + }, + CancellationToken.None); + snapshot.IsEnabled("checkout"); + + Assert.Empty(snapshot.UnresolvedFlags); + Assert.Equal("flag_missing", await GetFeatureFlagErrorAsync(client, batchHandler, "checkout")); + } + + [Fact] + public async Task EmptySnapshotHasNoUnresolvedFlags() + { + var container = CreateContainer(CheckoutWithExperienceContinuity); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync(string.Empty, LocalOnly, CancellationToken.None); + + Assert.Empty(snapshot.UnresolvedFlags); + } + + [Theory] + [InlineData(UnresolvedFlagReason.ExperienceContinuity, "experience_continuity")] + [InlineData(UnresolvedFlagReason.UnsupportedDefinition, "unsupported_definition")] + [InlineData(UnresolvedFlagReason.UnresolvedDependency, "unresolved_dependency")] + [InlineData(UnresolvedFlagReason.MissingContext, "missing_context")] + public void ReasonHasTheStableSpecValue(UnresolvedFlagReason reason, string expected) + { + Assert.Equal(expected, reason.ToValueString()); + Assert.Equal($"\"{expected}\"", JsonSerializer.Serialize(reason)); + } + + sealed class ThrowingValue + { + public override string ToString() => throw new InvalidOperationException("Cannot stringify"); + } +} diff --git a/tests/UnitTests/Features/LocalEvaluatorRegexTests.cs b/tests/UnitTests/Features/LocalEvaluatorRegexTests.cs index dc6b8f8f..76c03e8c 100644 --- a/tests/UnitTests/Features/LocalEvaluatorRegexTests.cs +++ b/tests/UnitTests/Features/LocalEvaluatorRegexTests.cs @@ -126,7 +126,7 @@ public void InvalidCohortLeavesRemainInconclusiveBeforeNegation(ComparisonOperat Assert.Throws(() => evaluator.EvaluateFeatureFlag("regex", "user", personProperties: properties)); - var (flags, fallbackToRemote) = evaluator.EvaluateAllFlags("user", personProperties: properties); + var (flags, fallbackToRemote, _) = evaluator.EvaluateAllFlags("user", personProperties: properties); Assert.True(fallbackToRemote); Assert.Empty(flags); } @@ -245,7 +245,7 @@ public void KeepsPortableSearchesLocal(string pattern, string? input, bool match var expected = comparison is ComparisonOperator.Regex ? matched : !matched; Assert.Equal(expected, evaluator.EvaluateFeatureFlag("regex", "user", personProperties: properties)); - var (flags, fallbackToRemote) = evaluator.EvaluateAllFlags("user", personProperties: properties); + var (flags, fallbackToRemote, _) = evaluator.EvaluateAllFlags("user", personProperties: properties); Assert.False(fallbackToRemote); Assert.Equal(expected, flags["regex"].IsEnabled); } @@ -265,7 +265,7 @@ public void InvalidPortablePatternsNeverMatchEitherOperator(string pattern, stri var evaluator = CreateEvaluator(pattern, comparison); var properties = new Dictionary { ["property"] = input }; Assert.False(evaluator.EvaluateFeatureFlag("regex", "user", personProperties: properties).Value); - var (flags, fallbackToRemote) = evaluator.EvaluateAllFlags("user", personProperties: properties); + var (flags, fallbackToRemote, _) = evaluator.EvaluateAllFlags("user", personProperties: properties); Assert.False(fallbackToRemote); Assert.False(flags["regex"].IsEnabled); } @@ -334,7 +334,7 @@ public void UnsupportedSearchesReachFallbackIncludingNegation(string pattern, st var properties = new Dictionary { ["property"] = input }; Assert.Throws(() => evaluator.EvaluateFeatureFlag("regex", "user", personProperties: properties)); - var (flags, fallbackToRemote) = evaluator.EvaluateAllFlags("user", personProperties: properties); + var (flags, fallbackToRemote, _) = evaluator.EvaluateAllFlags("user", personProperties: properties); Assert.True(fallbackToRemote); Assert.Empty(flags); } @@ -349,7 +349,7 @@ public void NativeExecutionTimeoutsReachFallbackWithoutNegation() var properties = new Dictionary { ["property"] = new string('a', 200) + "!" }; Assert.Throws(() => evaluator.EvaluateFeatureFlag("regex", "user", personProperties: properties)); - var (flags, fallbackToRemote) = evaluator.EvaluateAllFlags("user", personProperties: properties); + var (flags, fallbackToRemote, _) = evaluator.EvaluateAllFlags("user", personProperties: properties); Assert.True(fallbackToRemote); Assert.Empty(flags); } @@ -362,7 +362,7 @@ public void MissingPropertiesStillReachFallback(ComparisonOperator comparison) { var evaluator = CreateEvaluator("", comparison); Assert.Throws(() => evaluator.EvaluateFeatureFlag("regex", "user", personProperties: [])); - var (flags, fallbackToRemote) = evaluator.EvaluateAllFlags("user", personProperties: []); + var (flags, fallbackToRemote, _) = evaluator.EvaluateAllFlags("user", personProperties: []); Assert.True(fallbackToRemote); Assert.Empty(flags); } diff --git a/tests/UnitTests/Features/LocalEvaluatorTests.cs b/tests/UnitTests/Features/LocalEvaluatorTests.cs index a3749132..30ed0686 100644 --- a/tests/UnitTests/Features/LocalEvaluatorTests.cs +++ b/tests/UnitTests/Features/LocalEvaluatorTests.cs @@ -1,9 +1,11 @@ using System.Globalization; using System.Text.Json; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Time.Testing; using PostHog; using PostHog.Api; +using PostHog.Exceptions; using PostHog.Features; using PostHog.Json; using UnitTests.Library; @@ -3111,3 +3113,357 @@ public void ReturnsMatchWhenMultipleInconclusiveGroupsPrecedeMatchingGroup() Assert.True(result.Value); } } + +public class TheUnresolvedFlagReasons +{ + const string PlanIsPro = """{"key": "plan", "type": "person", "value": "pro", "operator": "exact"}"""; + const string CountryIsUs = """{"key": "country", "type": "person", "value": "US", "operator": "exact"}"""; + const string UnknownOperator = """{"key": "plan", "type": "person", "value": "pro", "operator": "operator_from_the_future"}"""; + const string StaticCohort = """{"key": "id", "type": "cohort", "value": 999}"""; + const string DependsOnCheckout = """ + {"key": "checkout", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["checkout"]} + """; + + const string CheckoutWithExperienceContinuity = """ + {"id": 2, "key": "checkout", "active": true, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """; + + static LocalEvaluator CreateEvaluator(params string[] flags) => CreateEvaluatorWithCohorts("{}", flags); + + static LocalEvaluator CreateEvaluatorWithCohorts(string cohorts, params string[] flags) => + new(JsonSerializer.Deserialize( + $$$""" + {"flags": [{{{string.Join(", ", flags)}}}], "cohorts": {{{cohorts}}}, "group_type_mapping": {"0": "company"}} + """, + JsonSerializerHelper.Options)!); + + static string Flag(string key, params string[] conditions) => $$$""" + {"id": 1, "key": "{{{key}}}", "active": true, "filters": {"groups": [{{{string.Join(", ", conditions)}}}]}} + """; + + static string Condition(params string[] filters) => + $$$"""{"properties": [{{{string.Join(", ", filters)}}}], "rollout_percentage": 100}"""; + + static string Filter(string comparison, string value) => + $$$"""{"key": "plan", "type": "person", "value": {{{JsonSerializer.Serialize(value)}}}, "operator": "{{{comparison}}}"}"""; + + static UnresolvedFlagReason? EvaluateReason( + LocalEvaluator evaluator, + string key, + Dictionary? personProperties = null) + { + var (flags, fallbackToRemote, unresolved) = evaluator.EvaluateAllFlags( + "user-123", + personProperties: personProperties ?? [], + flagKeysToEvaluate: [key]); + if (!unresolved.TryGetValue(key, out var reason)) + { + return null; + } + Assert.True(fallbackToRemote); + Assert.Empty(flags); + return reason; + } + + public static IEnumerable PrecedenceCases() + { + // Across conditions. + yield return [new[] { Condition(PlanIsPro), Condition(UnknownOperator) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(UnknownOperator), Condition(PlanIsPro) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(PlanIsPro), Condition(DependsOnCheckout) }, UnresolvedFlagReason.UnresolvedDependency]; + yield return [new[] { Condition(DependsOnCheckout), Condition(PlanIsPro) }, UnresolvedFlagReason.UnresolvedDependency]; + yield return [new[] { Condition(DependsOnCheckout), Condition(StaticCohort) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(PlanIsPro), Condition(StaticCohort) }, UnresolvedFlagReason.UnsupportedDefinition]; + // Within one condition. + yield return [new[] { Condition(PlanIsPro, DependsOnCheckout) }, UnresolvedFlagReason.UnresolvedDependency]; + yield return [new[] { Condition(DependsOnCheckout, PlanIsPro) }, UnresolvedFlagReason.UnresolvedDependency]; + yield return [new[] { Condition(DependsOnCheckout, UnknownOperator) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(UnknownOperator, DependsOnCheckout) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(StaticCohort, PlanIsPro) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(PlanIsPro, StaticCohort) }, UnresolvedFlagReason.UnsupportedDefinition]; + } + + [Theory] + [MemberData(nameof(PrecedenceCases))] + public void ReportsTheHighestPrecedenceReasonWhateverTheOrder(string[] conditions, UnresolvedFlagReason expected) + { + var evaluator = CreateEvaluator(CheckoutWithExperienceContinuity, Flag("beta-ui", conditions)); + + Assert.Equal(expected, EvaluateReason(evaluator, "beta-ui")); + } + + [Theory] + [InlineData(PlanIsPro, CountryIsUs)] + [InlineData(CountryIsUs, PlanIsPro)] + [InlineData(StaticCohort, CountryIsUs)] + [InlineData(CountryIsUs, StaticCohort)] + [InlineData(DependsOnCheckout, CountryIsUs)] + public void ConditionWithNonMatchingFilterResolvesLocallyDespiteUncheckableFilter(string first, string second) + { + var evaluator = CreateEvaluator(CheckoutWithExperienceContinuity, Flag("beta-ui", Condition(first, second))); + var properties = new Dictionary { ["country"] = "DE" }; + + Assert.False(evaluator.EvaluateFeatureFlag("beta-ui", "user-123", personProperties: properties).Value); + Assert.Null(EvaluateReason(evaluator, "beta-ui", properties)); + } + + [Fact] + public void StaticCohortFallsBackToServerWhenItIsTheFirstFilterThatCannotBeChecked() + { + var evaluator = CreateEvaluator(Flag("beta-ui", Condition(StaticCohort, PlanIsPro), Condition())); + + var exception = Assert.Throws( + () => evaluator.EvaluateFeatureFlag("beta-ui", "user-123", personProperties: [])); + Assert.Equal(UnresolvedFlagReason.UnsupportedDefinition, exception.Reason); + } + + [Fact] + public void StaticCohortAfterAnotherUncheckableFilterLetsLaterConditionsResolveLocally() + { + var evaluator = CreateEvaluator(Flag("beta-ui", Condition(PlanIsPro, StaticCohort), Condition())); + + Assert.True(evaluator.EvaluateFeatureFlag("beta-ui", "user-123", personProperties: []).Value); + } + + [Theory] + [InlineData("operator_from_the_future", "pro")] + [InlineData("is_date_before", "not-a-date")] + [InlineData("semver_gt", "not-a-version")] + [InlineData("semver_tilde", "not-a-version")] + [InlineData("semver_wildcard", "not-a-pattern")] + [InlineData("regex", "café")] + public void UnusableFilterOutranksMissingProperty(string comparison, string filterValue) + { + var evaluator = CreateEvaluator(Flag("beta-ui", Condition(Filter(comparison, filterValue)))); + + Assert.Equal(UnresolvedFlagReason.UnsupportedDefinition, EvaluateReason(evaluator, "beta-ui")); + } + + [Theory] + [InlineData("exact", "pro")] + [InlineData("is_date_before", "-24d")] + [InlineData("semver_gt", "1.2.3")] + [InlineData("regex", "^pro$")] + public void MissingPropertyForUsableFilterIsMissingContext(string comparison, string filterValue) + { + var evaluator = CreateEvaluator(Flag("beta-ui", Condition(Filter(comparison, filterValue)))); + + Assert.Equal(UnresolvedFlagReason.MissingContext, EvaluateReason(evaluator, "beta-ui")); + } + + [Theory] + [InlineData("is_date_before", "2024-01-01", "not-a-date", UnresolvedFlagReason.MissingContext)] + [InlineData("is_date_before", "-24d", "not-a-date", UnresolvedFlagReason.MissingContext)] + [InlineData("is_date_after", "2024-01-01", 42, UnresolvedFlagReason.MissingContext)] + [InlineData("is_date_before", "not-a-date", "2024-01-01", UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("is_date_before", "not-a-date", "not-a-date", UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("semver_gt", "1.2.3", "not-a-version", UnresolvedFlagReason.MissingContext)] + [InlineData("semver_gt", "not-a-version", "1.2.3", UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("semver_eq", "not-a-version", "not-a-version", UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("semver_caret", "not-a-version", "not-a-version", UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("semver_wildcard", "1.*", "not-a-version", UnresolvedFlagReason.MissingContext)] + [InlineData("semver_wildcard", "not-a-pattern", "not-a-version", UnresolvedFlagReason.UnsupportedDefinition)] + public void BadPersonValueIsMissingContextAndBadFilterValueIsUnsupportedDefinition( + string comparison, + string filterValue, + object personValue, + UnresolvedFlagReason expected) + { + var evaluator = CreateEvaluator(Flag("beta-ui", Condition(Filter(comparison, filterValue)))); + + Assert.Equal(expected, EvaluateReason(evaluator, "beta-ui", new() { ["plan"] = personValue })); + } + + [Theory] + [InlineData("AND", new[] { PlanIsPro }, UnresolvedFlagReason.MissingContext)] + [InlineData("OR", new[] { PlanIsPro, UnknownOperator }, UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("OR", new[] { UnknownOperator, PlanIsPro }, UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("AND", new[] { PlanIsPro, StaticCohort }, UnresolvedFlagReason.UnsupportedDefinition)] + public void CohortReportsTheHighestPrecedenceReasonOfItsFilters(string groupType, string[] filters, UnresolvedFlagReason expected) + { + var cohorts = $$$"""{"1": {"type": "{{{groupType}}}", "values": [{"type": "{{{groupType}}}", "values": [{{{string.Join(", ", filters)}}}]}]}}"""; + var evaluator = CreateEvaluatorWithCohorts( + cohorts, + Flag("beta-ui", Condition("""{"key": "id", "type": "cohort", "value": 1}"""))); + + Assert.Equal(expected, EvaluateReason(evaluator, "beta-ui")); + } + + [Fact] + public void UnknownGroupTypeIndexIsUnsupportedDefinition() + { + var evaluator = CreateEvaluator(""" + {"id": 1, "key": "beta-ui", "active": true, + "filters": {"aggregation_group_type_index": 5, "groups": [{"properties": [], "rollout_percentage": 100}]}} + """); + + Assert.Equal(UnresolvedFlagReason.UnsupportedDefinition, EvaluateReason(evaluator, "beta-ui")); + } + + [Fact] + public void InactiveFlagResolvesToFalseBeforeExperienceContinuityIsChecked() + { + var evaluator = CreateEvaluator(""" + {"id": 2, "key": "checkout", "active": false, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """); + + Assert.False(evaluator.EvaluateFeatureFlag("checkout", "user-123").Value); + var (flags, fallbackToRemote, unresolved) = evaluator.EvaluateAllFlags("user-123"); + Assert.False(flags["checkout"].IsEnabled); + Assert.False(fallbackToRemote); + Assert.Empty(unresolved); + } + + [Fact] + public void ExperienceContinuityFlagIsExperienceContinuity() + { + var evaluator = CreateEvaluator(CheckoutWithExperienceContinuity); + + Assert.Equal(UnresolvedFlagReason.ExperienceContinuity, EvaluateReason(evaluator, "checkout")); + } + + [Fact] + public void DependencyWithExperienceContinuityIsUnresolvedDependency() + { + var evaluator = CreateEvaluator(CheckoutWithExperienceContinuity, Flag("beta-ui", Condition(DependsOnCheckout))); + + Assert.Equal(UnresolvedFlagReason.UnresolvedDependency, EvaluateReason(evaluator, "beta-ui")); + } + + [Fact] + public void InactiveDependencyWithExperienceContinuityResolvesToFalse() + { + var evaluator = CreateEvaluator( + """ + {"id": 2, "key": "checkout", "active": false, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """, + Flag("beta-ui", Condition(DependsOnCheckout))); + + Assert.False(evaluator.EvaluateFeatureFlag("beta-ui", "user-123").Value); + Assert.Null(EvaluateReason(evaluator, "beta-ui")); + } + + [Fact] + public void DependencyThatRequiresServerIsUnresolvedDependencyWithoutForcingServerFallback() + { + var evaluator = CreateEvaluator( + Flag("checkout", Condition(StaticCohort)), + Flag("beta-ui", Condition(DependsOnCheckout)), + Flag("beta-ui-with-fallback", Condition(DependsOnCheckout), Condition())); + + var exception = Assert.Throws( + () => evaluator.EvaluateFeatureFlag("beta-ui", "user-123")); + Assert.Equal(UnresolvedFlagReason.UnresolvedDependency, exception.Reason); + Assert.True(evaluator.EvaluateFeatureFlag("beta-ui-with-fallback", "user-123").Value); + } + + [Theory] + [InlineData("""{"key": "checkout", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": []}""")] + [InlineData("""{"key": "checkout", "type": "flag", "value": true, "operator": "flag_evaluates_to"}""")] + [InlineData("""{"key": "missing", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["missing"]}""")] + [InlineData("""{"key": "missing", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["checkout", "missing"]}""")] + public void CircularMalformedOrMissingDependencyIsUnsupportedDefinition(string dependency) + { + var evaluator = CreateEvaluator(CheckoutWithExperienceContinuity, Flag("beta-ui", Condition(dependency))); + + Assert.Equal(UnresolvedFlagReason.UnsupportedDefinition, EvaluateReason(evaluator, "beta-ui")); + } + + [Fact] + public void UnexpectedExceptionIsNotUnresolved() + { + var evaluator = CreateEvaluator(Flag("beta-ui", Condition(PlanIsPro))); + + var (flags, fallbackToRemote, unresolved) = evaluator.EvaluateAllFlags( + "user-123", + personProperties: new() { ["plan"] = new ThrowingValue() }); + + Assert.Empty(flags); + Assert.True(fallbackToRemote); + Assert.Empty(unresolved); + } + + [Fact] + public void StaticCohortDoesNotLogAnErrorOnEachEvaluation() + { + var loggerProvider = new FakeLoggerProvider(); + var evaluator = new LocalEvaluator( + JsonSerializer.Deserialize( + $$$"""{"flags": [{{{Flag("beta-ui", Condition(StaticCohort))}}}]}""", + JsonSerializerHelper.Options)!, + TimeProvider.System, + ((ILoggerFactory)loggerProvider).CreateLogger()); + + var (_, fallbackToRemote, unresolved) = evaluator.EvaluateAllFlags("user-123"); + + Assert.True(fallbackToRemote); + Assert.Equal(UnresolvedFlagReason.UnsupportedDefinition, unresolved["beta-ui"]); + Assert.Empty(loggerProvider.GetAllEvents(minimumLevel: LogLevel.Warning)); + } + + public static IEnumerable DefinitionCases() + { + yield return [CheckoutWithExperienceContinuity, UnresolvedFlagReason.ExperienceContinuity]; + yield return [ + """ + {"id": 2, "key": "checkout", "active": false, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [{"key": "id", "type": "cohort", "value": 999}]}]}} + """, + null]; + yield return [Flag("checkout", Condition(PlanIsPro)), null]; + yield return [Flag("checkout", Condition(CountryIsUs), Condition(StaticCohort)), UnresolvedFlagReason.UnsupportedDefinition]; + yield return [Flag("checkout", Condition(UnknownOperator)), UnresolvedFlagReason.UnsupportedDefinition]; + yield return [Flag("checkout", Condition(Filter("is_date_after", "not-a-date"))), UnresolvedFlagReason.UnsupportedDefinition]; + yield return [Flag("checkout", Condition(Filter("semver_gt", "not-a-version"))), UnresolvedFlagReason.UnsupportedDefinition]; + yield return [Flag("checkout", Condition(Filter("regex", "café"))), UnresolvedFlagReason.UnsupportedDefinition]; + yield return [Flag("checkout", Condition(Filter("regex", "^pro$"))), null]; + yield return [ + Flag("checkout", Condition("""{"key": "missing", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["missing"]}""")), + UnresolvedFlagReason.UnsupportedDefinition]; + yield return [ + Flag("checkout", Condition("""{"key": "beta-ui", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": []}""")), + UnresolvedFlagReason.UnsupportedDefinition]; + // A dependency that is only unresolved depends on the other flag, not on this definition. + yield return [ + Flag("checkout", Condition("""{"key": "beta-ui", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["beta-ui"]}""")), + null]; + yield return [Flag("checkout", Condition("""{"key": "id", "type": "cohort", "value": 1}""")), UnresolvedFlagReason.UnsupportedDefinition]; + yield return [Flag("checkout", Condition("""{"key": "id", "type": "cohort", "value": 2}""")), null]; + yield return [ + """ + {"id": 2, "key": "checkout", "active": true, + "filters": {"aggregation_group_type_index": 5, "groups": [{"properties": [], "rollout_percentage": 100}]}} + """, + UnresolvedFlagReason.UnsupportedDefinition]; + } + + [Theory] + [MemberData(nameof(DefinitionCases))] + public void GetDefinitionUnresolvedReasonFindsCausesThatComeFromTheDefinition(string checkout, UnresolvedFlagReason? expected) + { + var betaUi = """ + {"id": 1, "key": "beta-ui", "active": true, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """; + var cohorts = """ + { + "1": {"type": "AND", "values": [{"type": "AND", "values": [ + {"key": "plan", "type": "person", "value": "pro", "operator": "operator_from_the_future"}]}]}, + "2": {"type": "AND", "values": [{"type": "AND", "values": [ + {"key": "plan", "type": "person", "value": "pro", "operator": "exact"}]}]} + } + """; + var evaluator = CreateEvaluatorWithCohorts(cohorts, checkout, betaUi); + Assert.True(evaluator.TryGetLocalFeatureFlag("checkout", out var flag)); + + Assert.Equal(expected, evaluator.GetDefinitionUnresolvedReason(flag)); + } + + sealed class ThrowingValue + { + public override string ToString() => throw new InvalidOperationException("Cannot stringify"); + } +} diff --git a/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs index b66deef6..2187a9ad 100644 --- a/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs +++ b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs @@ -1,4 +1,5 @@ using System.Net; +using Microsoft.Extensions.Logging; using PostHog; using PostHog.Api; using PostHog.Features; @@ -196,3 +197,132 @@ public async Task CompletesGracefullyWhenPollingNeverStarted() await client.DisposeAsync(); } } + +public class TheUnresolvableFlagWarning +{ + const string CheckoutWithExperienceContinuity = """ + {"id": 2, "key": "checkout", "active": true, "ensure_experience_continuity": true, + "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + """; + + static string Flag(string key, string filter = "", bool experienceContinuity = false, int rolloutPercentage = 100) => $$$""" + {"id": 1, "key": "{{{key}}}", "active": true, "ensure_experience_continuity": {{{(experienceContinuity ? "true" : "false")}}}, + "filters": {"groups": [{"properties": [{{{filter}}}], "rollout_percentage": {{{rolloutPercentage}}}}]}} + """; + + static string Definitions(params string[] flags) => $$$"""{"flags": [{{{string.Join(", ", flags)}}}], "cohorts": {}}"""; + + static (TestContainer Container, PostHogClient Client, FakeHttpMessageHandler.RequestHandler FlagsHandler) CreateClient() + { + var container = new TestContainer("fake-personal-api-key"); + var flagsHandler = container.FakeHttpMessageHandler.AddFlagsResponse("""{"featureFlags": {}}"""); + return (container, container.Activate(), flagsHandler); + } + + static async Task LoadAsync(TestContainer container, PostHogClient client, string definitions) + { + var handler = container.FakeHttpMessageHandler.AddLocalEvaluationResponseWithETag(definitions, "\"definitions\""); + await client.LoadFeatureFlagsAsync(CancellationToken.None); + Assert.Single(handler.ReceivedRequests); + } + + static List GetWarnings(TestContainer container, string reason) => + container.FakeLoggerProvider.GetAllEvents(minimumLevel: LogLevel.Warning) + .Select(e => e.Message ?? string.Empty) + .Where(message => message.Contains($"(reason: {reason})", StringComparison.Ordinal)) + .ToList(); + + [Theory] + [InlineData("unchanged definitions")] + [InlineData("304 not modified")] + public async Task LoadingDefinitionThatLocalEvaluationNeverResolvesLogsOneWarning(string refreshResult) + { + var (container, client, flagsHandler) = CreateClient(); + await LoadAsync(container, client, Definitions(CheckoutWithExperienceContinuity)); + + if (refreshResult == "304 not modified") + { + var notModified = container.FakeHttpMessageHandler.AddLocalEvaluationNotModifiedResponse(); + await client.LoadFeatureFlagsAsync(CancellationToken.None); + Assert.Single(notModified.ReceivedRequests); + } + else + { + await LoadAsync(container, client, Definitions(CheckoutWithExperienceContinuity)); + } + + var warning = Assert.Single(GetWarnings(container, "experience_continuity")); + Assert.Contains("checkout", warning, StringComparison.Ordinal); + Assert.Empty(flagsHandler.ReceivedRequests); + } + + [Fact] + public async Task ChangedDefinitionThatLocalEvaluationNeverResolvesLogsTheWarningAgain() + { + var (container, client, _) = CreateClient(); + await LoadAsync(container, client, Definitions(Flag("checkout", experienceContinuity: true))); + await LoadAsync(container, client, Definitions(Flag("checkout", experienceContinuity: true, rolloutPercentage: 50))); + + var warnings = GetWarnings(container, "experience_continuity"); + Assert.Equal(2, warnings.Count); + Assert.All(warnings, warning => Assert.Contains("checkout", warning, StringComparison.Ordinal)); + } + + [Fact] + public async Task WarnsOncePerReasonAboutDefinitionCausesOnly() + { + var (container, client, _) = CreateClient(); + + await LoadAsync(container, client, Definitions( + Flag("experience-continuity", experienceContinuity: true), + Flag("static-cohort", """{"key": "id", "type": "cohort", "value": 999}"""), + Flag("unknown-operator", """{"key": "plan", "type": "person", "value": "pro", "operator": "operator_from_the_future"}"""), + Flag("missing-dependency", """{"key": "missing", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["missing"]}"""), + Flag("circular-dependency", """{"key": "resolvable", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": []}"""), + Flag("unresolved-dependency", """{"key": "experience-continuity", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["experience-continuity"]}"""), + Flag("missing-context", """{"key": "plan", "type": "person", "value": "pro", "operator": "exact"}"""), + Flag("resolvable"))); + + var experienceContinuity = Assert.Single(GetWarnings(container, "experience_continuity")); + Assert.Contains(": experience-continuity.", experienceContinuity, StringComparison.Ordinal); + var unsupportedDefinition = Assert.Single(GetWarnings(container, "unsupported_definition")); + Assert.Contains( + ": static-cohort, unknown-operator, missing-dependency, circular-dependency.", + unsupportedDefinition, + StringComparison.Ordinal); + Assert.Equal(2, container.FakeLoggerProvider.GetAllEvents(minimumLevel: LogLevel.Warning).Count); + } + + [Fact] + public async Task FlagThatLeavesAndReturnsLogsTheWarningAgain() + { + var (container, client, _) = CreateClient(); + await LoadAsync(container, client, Definitions(CheckoutWithExperienceContinuity, Flag("resolvable"))); + await LoadAsync(container, client, Definitions(Flag("resolvable"))); + await LoadAsync(container, client, Definitions(CheckoutWithExperienceContinuity, Flag("resolvable"))); + + Assert.Equal(2, GetWarnings(container, "experience_continuity").Count); + } + + [Fact] + public async Task FlagThatBecomesResolvableAndThenUnresolvableLogsTheWarningAgain() + { + var (container, client, _) = CreateClient(); + await LoadAsync(container, client, Definitions(Flag("checkout", experienceContinuity: true))); + await LoadAsync(container, client, Definitions(Flag("checkout"))); + await LoadAsync(container, client, Definitions(Flag("checkout", experienceContinuity: true))); + + Assert.Equal(2, GetWarnings(container, "experience_continuity").Count); + } + + [Fact] + public async Task ChangedReasonLogsTheNewReason() + { + var (container, client, _) = CreateClient(); + await LoadAsync(container, client, Definitions(Flag("checkout", experienceContinuity: true))); + await LoadAsync(container, client, Definitions(Flag("checkout", """{"key": "id", "type": "cohort", "value": 999}"""))); + + Assert.Single(GetWarnings(container, "experience_continuity")); + Assert.Contains("checkout", Assert.Single(GetWarnings(container, "unsupported_definition")), StringComparison.Ordinal); + } +} diff --git a/tests/UnitTests/Features/VersionedPropertyMatchingTests.cs b/tests/UnitTests/Features/VersionedPropertyMatchingTests.cs index 26bb16dc..a4399597 100644 --- a/tests/UnitTests/Features/VersionedPropertyMatchingTests.cs +++ b/tests/UnitTests/Features/VersionedPropertyMatchingTests.cs @@ -149,7 +149,7 @@ public void PersonGroupRecursiveCohortAndDependencyShareSnapshot(int? version, b { Assert.Equal(expected, evaluator.EvaluateFeatureFlag(flag.Key, "person", groups, properties)); } - var (all, fallback) = evaluator.EvaluateAllFlags("person", groups, properties); + var (all, fallback, _) = evaluator.EvaluateAllFlags("person", groups, properties); Assert.False(fallback); Assert.Equal(4, all.Count); Assert.All(all.Values, flag => Assert.Equal(expected, flag.IsEnabled)); From a2c64d217410c385033254157217a236e21d44b1 Mon Sep 17 00:00:00 2001 From: "Gustavo H. Strassburger" Date: Wed, 28 Oct 2026 00:55:32 -0300 Subject: [PATCH 2/3] fix(flags): address review findings for unresolved local flags --- .../RequiresServerEvaluationException.cs | 11 ++----- src/PostHog/Features/LocalEvaluator.cs | 33 +++++++++++++++---- .../Features/LocalFeatureFlagsLoader.cs | 11 ++++++- src/PostHog/Library/RegexHelpers.cs | 6 ++++ .../UnitTests/Features/LocalEvaluatorTests.cs | 23 ++++++++++++- .../Features/LocalFeatureFlagsLoaderTests.cs | 28 ++++++++++++++-- 6 files changed, 93 insertions(+), 19 deletions(-) diff --git a/src/PostHog/Exceptions/RequiresServerEvaluationException.cs b/src/PostHog/Exceptions/RequiresServerEvaluationException.cs index 22e031c0..df2982e5 100644 --- a/src/PostHog/Exceptions/RequiresServerEvaluationException.cs +++ b/src/PostHog/Exceptions/RequiresServerEvaluationException.cs @@ -37,15 +37,10 @@ public RequiresServerEvaluationException(string message, Exception innerExceptio { } - internal RequiresServerEvaluationException(string message, UnresolvedFlagReason reason) : base(message) - { - Reason = reason; - } - /// - /// Why local evaluation could not resolve the flag. A condition that reaches this exception can raise it - /// to a higher-precedence reason found earlier in the same flag. + /// Why local evaluation could not resolve the flag. Only + /// outranks it, and that reason is found before any condition can raise this exception. /// - internal UnresolvedFlagReason Reason { get; set; } = UnresolvedFlagReason.UnsupportedDefinition; + internal UnresolvedFlagReason Reason { get; } = UnresolvedFlagReason.UnsupportedDefinition; } } diff --git a/src/PostHog/Features/LocalEvaluator.cs b/src/PostHog/Features/LocalEvaluator.cs index d9115490..f7a82f33 100644 --- a/src/PostHog/Features/LocalEvaluator.cs +++ b/src/PostHog/Features/LocalEvaluator.cs @@ -230,7 +230,12 @@ bool IsUnsupportedFilter(Filter filter, HashSet visitedCohortIds, bool inC switch (filter) { case FilterSet filterSet: - return filterSet.Values.Any(child => IsUnsupportedFilter(child, visitedCohortIds, inCohort)); + return (filterSet.Values ?? []).Any(child => IsUnsupportedFilter(child, visitedCohortIds, inCohort)); + case PropertyFilter { Operator: ComparisonOperator.Regex or ComparisonOperator.NotRegex, Value: { } value } when inCohort + && (value.StringValue is not { } pattern || !RegexHelpers.CanMatchLocally(pattern)): + // A cohort reports a regex it cannot run as UnsupportedDefinition. At flag level the same regex + // resolves locally as no match, so this check stays out of AssertFilterSupported. + return true; case PropertyFilter { Type: FilterType.Flag } dependency: // MatchProperty cannot evaluate a flag dependency inside a cohort. if (inCohort) @@ -413,11 +418,9 @@ StringOrValue MatchFeatureFlagProperties( { conditionResult = EvaluateConditionMatch(flag, effectiveBucketingId, condition, effectiveProperties, evaluationCache, groups); } - catch (RequiresServerEvaluationException e) + catch (RequiresServerEvaluationException) { // Static cohort or other missing server-side data - must fallback to API. - // A reason an earlier condition found can still outrank this one. - e.Reason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, e.Reason); throw; } catch (InconclusiveMatchException e) @@ -554,6 +557,12 @@ ConditionMatchResult EvaluateConditionMatch( inconclusiveReason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, e.Reason); continue; } +#pragma warning disable CA1031 // An earlier filter already left the condition inconclusive. Stop at that reason, so an unexpected error here cannot fail a flag that a later condition can still resolve. + catch (Exception) when (inconclusiveReason is not null) +#pragma warning restore CA1031 + { + break; + } if (!isMatch) { @@ -567,7 +576,6 @@ ConditionMatchResult EvaluateConditionMatch( // server instead of trying later conditions. if (serverEvaluationException is not null) { - serverEvaluationException.Reason = reason; throw serverEvaluationException; } throw new InconclusiveMatchException($"Can't determine if a condition of flag {flag.Key} matches", reason); @@ -633,8 +641,7 @@ bool MatchCohort( if (cohortId is null || !_cohortFilters.TryGetValue(cohortId.Value, out var conditions)) { throw new RequiresServerEvaluationException( - $"cohort {cohortId} not found in local cohorts - likely a static cohort that requires server evaluation", - UnresolvedFlagReason.UnsupportedDefinition); + $"cohort {cohortId} not found in local cohorts - likely a static cohort that requires server evaluation"); } return MatchPropertyGroup(conditions, distinctId, propertyValues); @@ -713,6 +720,15 @@ bool MatchPropertyGroup(FilterSet? filterSet, string distinctId, Dictionary value.IsExactMatch(overrideValue, LocalEvaluationApiResult.PropertyMatchingVersion), @@ -877,6 +894,8 @@ static bool IsPropertyMissing(PropertyFilter propertyFilter, Dictionary + // Keep the operator list in sync with the switch in MatchProperty. An operator missing here is reported as + // unsupported when the property is absent, and in the load-time warning. static void AssertFilterSupported(PropertyFilter propertyFilter) { if (propertyFilter.Operator is ComparisonOperator.IsSet or ComparisonOperator.IsNotSet) diff --git a/src/PostHog/Features/LocalFeatureFlagsLoader.cs b/src/PostHog/Features/LocalFeatureFlagsLoader.cs index 9d393ae1..030d6bd2 100644 --- a/src/PostHog/Features/LocalFeatureFlagsLoader.cs +++ b/src/PostHog/Features/LocalFeatureFlagsLoader.cs @@ -118,7 +118,16 @@ void StartPollingIfNotStarted() var localEvaluator = new LocalEvaluator(response.Result, timeProvider, _localEvaluatorLogger); Interlocked.Exchange(ref _localEvaluator, localEvaluator); Interlocked.Exchange(ref _flagDefinitionsLoadedAtMs, timeProvider.GetUtcNow().ToUnixTimeMilliseconds()); - WarnAboutUnresolvableFlags(localEvaluator); + try + { + WarnAboutUnresolvableFlags(localEvaluator); + } +#pragma warning disable CA1031 // The warning is diagnostic only. A definition the scan cannot read must not stop polling or fail a flag call. + catch (Exception e) +#pragma warning restore CA1031 + { + _logger.LogErrorUnexpectedException(e); + } return localEvaluator; } diff --git a/src/PostHog/Library/RegexHelpers.cs b/src/PostHog/Library/RegexHelpers.cs index fbabe452..a469e6d2 100644 --- a/src/PostHog/Library/RegexHelpers.cs +++ b/src/PostHog/Library/RegexHelpers.cs @@ -81,6 +81,12 @@ public static bool TryValidateRegex( public static bool IsCompatiblePattern(string pattern) => pattern.Length == 0 || GetCompatiblePattern(pattern) is not null; + // True when MatchFeatureFlagRegex accepts and compiles the pattern instead of rejecting it or returning null. + public static bool CanMatchLocally(string pattern) + => pattern.Length == 0 + || (GetCompatiblePattern(pattern) is { } compatiblePattern + && TryValidateRegex(compatiblePattern, out _, RegexOptions.None)); + // Successful .NET compilation alone does not establish compatibility with the service's regex syntax. static string? GetCompatiblePattern(string pattern) { diff --git a/tests/UnitTests/Features/LocalEvaluatorTests.cs b/tests/UnitTests/Features/LocalEvaluatorTests.cs index 30ed0686..2d9782b2 100644 --- a/tests/UnitTests/Features/LocalEvaluatorTests.cs +++ b/tests/UnitTests/Features/LocalEvaluatorTests.cs @@ -3226,6 +3226,17 @@ public void StaticCohortAfterAnotherUncheckableFilterLetsLaterConditionsResolveL Assert.True(evaluator.EvaluateFeatureFlag("beta-ui", "user-123", personProperties: []).Value); } + [Fact] + public void UnexpectedExceptionAfterAnotherUncheckableFilterLetsLaterConditionsResolveLocally() + { + var evaluator = CreateEvaluator(Flag("beta-ui", Condition(CountryIsUs, PlanIsPro), Condition())); + + Assert.True(evaluator.EvaluateFeatureFlag( + "beta-ui", + "user-123", + personProperties: new() { ["plan"] = new ThrowingValue() }).Value); + } + [Theory] [InlineData("operator_from_the_future", "pro")] [InlineData("is_date_before", "not-a-date")] @@ -3420,6 +3431,8 @@ public void StaticCohortDoesNotLogAnErrorOnEachEvaluation() yield return [Flag("checkout", Condition(Filter("semver_gt", "not-a-version"))), UnresolvedFlagReason.UnsupportedDefinition]; yield return [Flag("checkout", Condition(Filter("regex", "café"))), UnresolvedFlagReason.UnsupportedDefinition]; yield return [Flag("checkout", Condition(Filter("regex", "^pro$"))), null]; + // A flag-level regex that does not compile resolves locally as no match. + yield return [Flag("checkout", Condition(Filter("regex", "["))), null]; yield return [ Flag("checkout", Condition("""{"key": "missing", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["missing"]}""")), UnresolvedFlagReason.UnsupportedDefinition]; @@ -3432,6 +3445,9 @@ public void StaticCohortDoesNotLogAnErrorOnEachEvaluation() null]; yield return [Flag("checkout", Condition("""{"key": "id", "type": "cohort", "value": 1}""")), UnresolvedFlagReason.UnsupportedDefinition]; yield return [Flag("checkout", Condition("""{"key": "id", "type": "cohort", "value": 2}""")), null]; + yield return [Flag("checkout", Condition("""{"key": "id", "type": "cohort", "value": 3}""")), null]; + yield return [Flag("checkout", Condition("""{"key": "id", "type": "cohort", "value": 4}""")), UnresolvedFlagReason.UnsupportedDefinition]; + yield return [Flag("checkout", Condition("""{"key": "id", "type": "cohort", "value": 5}""")), UnresolvedFlagReason.UnsupportedDefinition]; yield return [ """ {"id": 2, "key": "checkout", "active": true, @@ -3453,7 +3469,12 @@ public void GetDefinitionUnresolvedReasonFindsCausesThatComeFromTheDefinition(st "1": {"type": "AND", "values": [{"type": "AND", "values": [ {"key": "plan", "type": "person", "value": "pro", "operator": "operator_from_the_future"}]}]}, "2": {"type": "AND", "values": [{"type": "AND", "values": [ - {"key": "plan", "type": "person", "value": "pro", "operator": "exact"}]}]} + {"key": "plan", "type": "person", "value": "pro", "operator": "exact"}]}]}, + "3": {"type": "AND", "values": null}, + "4": {"type": "AND", "values": [{"type": "AND", "values": [ + {"key": "plan", "type": "person", "value": "[", "operator": "regex"}]}]}, + "5": {"type": "AND", "values": [{"type": "AND", "values": [ + {"key": "plan", "type": "person", "value": 5, "operator": "not_regex"}]}]} } """; var evaluator = CreateEvaluatorWithCohorts(cohorts, checkout, betaUi); diff --git a/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs index 2187a9ad..f2504f76 100644 --- a/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs +++ b/tests/UnitTests/Features/LocalFeatureFlagsLoaderTests.cs @@ -238,7 +238,11 @@ static List GetWarnings(TestContainer container, string reason) => public async Task LoadingDefinitionThatLocalEvaluationNeverResolvesLogsOneWarning(string refreshResult) { var (container, client, flagsHandler) = CreateClient(); - await LoadAsync(container, client, Definitions(CheckoutWithExperienceContinuity)); + var definitions = Definitions( + CheckoutWithExperienceContinuity, + Flag("static-cohort", """{"key": "id", "type": "cohort", "value": 999}"""), + Flag("missing-dependency", """{"key": "missing", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["missing"]}""")); + await LoadAsync(container, client, definitions); if (refreshResult == "304 not modified") { @@ -248,11 +252,31 @@ public async Task LoadingDefinitionThatLocalEvaluationNeverResolvesLogsOneWarnin } else { - await LoadAsync(container, client, Definitions(CheckoutWithExperienceContinuity)); + await LoadAsync(container, client, definitions); } var warning = Assert.Single(GetWarnings(container, "experience_continuity")); Assert.Contains("checkout", warning, StringComparison.Ordinal); + Assert.Single(GetWarnings(container, "unsupported_definition")); + Assert.Empty(flagsHandler.ReceivedRequests); + } + + [Fact] + public async Task DefinitionTheWarningCannotReadDoesNotBreakLoadingOrFlagCalls() + { + var (container, client, flagsHandler) = CreateClient(); + container.FakeHttpMessageHandler.AddLocalEvaluationResponseWithETag( + Definitions( + Flag("malformed", """{"key": null, "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["resolvable"]}"""), + Flag("resolvable")), + "\"definitions\""); + + var flags = await client.EvaluateFlagsAsync( + "user-123", + new FeatureFlagOptions { FlagKeysToEvaluate = ["resolvable"] }, + CancellationToken.None); + + Assert.True(flags.IsEnabled("resolvable")); Assert.Empty(flagsHandler.ReceivedRequests); } From bd134795cd450d910b3b7e27565e964fcfe1a12f Mon Sep 17 00:00:00 2001 From: "Gustavo H. Strassburger" Date: Wed, 28 Oct 2026 14:00:14 -0300 Subject: [PATCH 3/3] fix(flags): report unrunnable cohort regex before missing context --- src/PostHog/Features/LocalEvaluator.cs | 17 +++++++++++++---- tests/UnitTests/Features/LocalEvaluatorTests.cs | 4 ++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/PostHog/Features/LocalEvaluator.cs b/src/PostHog/Features/LocalEvaluator.cs index f7a82f33..dd1bac07 100644 --- a/src/PostHog/Features/LocalEvaluator.cs +++ b/src/PostHog/Features/LocalEvaluator.cs @@ -231,10 +231,7 @@ bool IsUnsupportedFilter(Filter filter, HashSet visitedCohortIds, bool inC { case FilterSet filterSet: return (filterSet.Values ?? []).Any(child => IsUnsupportedFilter(child, visitedCohortIds, inCohort)); - case PropertyFilter { Operator: ComparisonOperator.Regex or ComparisonOperator.NotRegex, Value: { } value } when inCohort - && (value.StringValue is not { } pattern || !RegexHelpers.CanMatchLocally(pattern)): - // A cohort reports a regex it cannot run as UnsupportedDefinition. At flag level the same regex - // resolves locally as no match, so this check stays out of AssertFilterSupported. + case PropertyFilter property when inCohort && IsCohortRegexUnsupported(property): return true; case PropertyFilter { Type: FilterType.Flag } dependency: // MatchProperty cannot evaluate a flag dependency inside a cohort. @@ -725,6 +722,12 @@ bool MatchPropertyGroup(FilterSet? filterSet, string distinctId, Dictionary propertyFilter is { Operator: ComparisonOperator.Regex or ComparisonOperator.NotRegex, Value: { } value } + && (value.StringValue is not { } pattern || !RegexHelpers.CanMatchLocally(pattern)); + static bool IsPropertyMissing(PropertyFilter propertyFilter, Dictionary? properties) => properties is not null && !string.Equals(propertyFilter.Key, PostHogProperties.DistinctId, StringComparison.Ordinal) diff --git a/tests/UnitTests/Features/LocalEvaluatorTests.cs b/tests/UnitTests/Features/LocalEvaluatorTests.cs index 2d9782b2..931b1086 100644 --- a/tests/UnitTests/Features/LocalEvaluatorTests.cs +++ b/tests/UnitTests/Features/LocalEvaluatorTests.cs @@ -3120,6 +3120,8 @@ public class TheUnresolvedFlagReasons const string CountryIsUs = """{"key": "country", "type": "person", "value": "US", "operator": "exact"}"""; const string UnknownOperator = """{"key": "plan", "type": "person", "value": "pro", "operator": "operator_from_the_future"}"""; const string StaticCohort = """{"key": "id", "type": "cohort", "value": 999}"""; + const string InvalidRegex = """{"key": "plan", "type": "person", "value": "[", "operator": "regex"}"""; + const string NumericRegex = """{"key": "plan", "type": "person", "value": 5, "operator": "not_regex"}"""; const string DependsOnCheckout = """ {"key": "checkout", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["checkout"]} """; @@ -3291,6 +3293,8 @@ public void BadPersonValueIsMissingContextAndBadFilterValueIsUnsupportedDefiniti [InlineData("OR", new[] { PlanIsPro, UnknownOperator }, UnresolvedFlagReason.UnsupportedDefinition)] [InlineData("OR", new[] { UnknownOperator, PlanIsPro }, UnresolvedFlagReason.UnsupportedDefinition)] [InlineData("AND", new[] { PlanIsPro, StaticCohort }, UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("AND", new[] { InvalidRegex }, UnresolvedFlagReason.UnsupportedDefinition)] + [InlineData("AND", new[] { NumericRegex }, UnresolvedFlagReason.UnsupportedDefinition)] public void CohortReportsTheHighestPrecedenceReasonOfItsFilters(string groupType, string[] filters, UnresolvedFlagReason expected) { var cohorts = $$$"""{"1": {"type": "{{{groupType}}}", "values": [{"type": "{{{groupType}}}", "values": [{{{string.Join(", ", filters)}}}]}]}}""";