diff --git a/.changeset/unresolved-local-flags.md b/.changeset/unresolved-local-flags.md new file mode 100644 index 00000000..f6ef48ff --- /dev/null +++ b/.changeset/unresolved-local-flags.md @@ -0,0 +1,12 @@ +--- +"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. +- `$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..8b030e26 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,18 @@ public RequiresServerEvaluationException(string message, Exception innerExceptio : base(message, innerException) { } + + internal RequiresServerEvaluationException(string message, UnresolvedFlagReason reason, Exception innerException) + : base(message, innerException) + { + Reason = reason; + } + + /// + /// Why local evaluation could not resolve the flag: + /// for server data the flag uses itself, or for server + /// data that a flag dependency uses. + /// + internal UnresolvedFlagReason Reason { get; } = 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..4907e6f9 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,106 @@ 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; + } + + // Returns null when the definition cannot be read. The load-time warning already logs that, and a failed lookup + // must not stop the server fallback. + UnresolvedFlagReason? TryGetDefinitionUnresolvedReason(LocalFeatureFlag flag) + { + try + { + return GetDefinitionUnresolvedReason(flag); + } +#pragma warning disable CA1031 // The reason is diagnostic only. A malformed definition must not fail a flag that the server can evaluate. + catch (Exception) +#pragma warning restore CA1031 + { + return null; + } + } + + // Returns the server exception with a higher-precedence reason found elsewhere in the flag, so the flag still + // falls back to the server. + static RequiresServerEvaluationException WithReason(RequiresServerEvaluationException e, UnresolvedFlagReason reason) + => reason == e.Reason ? e : new RequiresServerEvaluationException(e.Message, reason, e); + + bool IsUnsupportedFilter(Filter filter, HashSet visitedCohortIds, bool inCohort) + { + switch (filter) + { + case FilterSet filterSet: + return (filterSet.Values ?? []).Any(child => IsUnsupportedFilter(child, visitedCohortIds, inCohort)); + case PropertyFilter property when inCohort && IsCohortRegexUnsupported(property): + return true; + 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 +320,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 +327,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 +353,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 +398,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 +436,22 @@ 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 - throw; + // Static cohort or other missing server-side data - must fallback to API. Evaluation stops here, so + // the definition of the whole flag also counts, including filters and cohorts it did not reach. + var reason = UnresolvedFlagReasonExtensions.Pick(inconclusiveReason, e.Reason); + if (TryGetDefinitionUnresolvedReason(flag) is { } definitionReason) + { + reason = UnresolvedFlagReasonExtensions.Pick(reason, definitionReason); + } + throw WithReason(e, reason); } - 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 +469,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 +492,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 +539,70 @@ 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; + } +#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) + { + 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) + { + throw WithReason(serverEvaluationException, reason); + } + throw new InconclusiveMatchException($"Can't determine if a condition of flag {flag.Key} matches", reason); } } @@ -484,7 +664,8 @@ 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"); } return MatchPropertyGroup(conditions, distinctId, propertyValues); @@ -504,7 +685,7 @@ bool MatchPropertyGroup(FilterSet? filterSet, string distinctId, Dictionary value.IsExactMatch(overrideValue, LocalEvaluationApiResult.PropertyMatchingVersion), @@ -705,10 +910,83 @@ or ComparisonOperator.Regex ComparisonOperator.SemverCaret => 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) }; } + // 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. + static bool IsCohortRegexUnsupported(PropertyFilter propertyFilter) + => 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) + && !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. + /// + // 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) + { + 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 +1014,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 +1028,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 +1066,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 +1085,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 +1117,20 @@ 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) + { + // Keep the server exception, so the dependent flag also falls back to the server, which has the data. + throw new RequiresServerEvaluationException( + $"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..030d6bd2 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,67 @@ void StartPollingIfNotStarted() var localEvaluator = new LocalEvaluator(response.Result, timeProvider, _localEvaluatorLogger); Interlocked.Exchange(ref _localEvaluator, localEvaluator); Interlocked.Exchange(ref _flagDefinitionsLoadedAtMs, timeProvider.GetUtcNow().ToUnixTimeMilliseconds()); + 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; } + // 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 +275,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..a469e6d2 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,21 @@ 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; + + // 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/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..14fea8fd 100644 --- a/tests/UnitTests/Features/FeatureFlagEvaluationsTests.cs +++ b/tests/UnitTests/Features/FeatureFlagEvaluationsTests.cs @@ -982,3 +982,445 @@ 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")); + } + + static readonly string TargetDependsOnDep = TargetDependsOnDepThen(); + + static string TargetDependsOnDepThen(params string[] secondConditionFilters) => $$$""" + {"id": 3, "key": "target", "active": true, + "filters": { + "multivariate": {"variants": [{"key": "a", "rollout_percentage": 50}, {"key": "b", "rollout_percentage": 50}]}, + "groups": [ + {"properties": [{"key": "dep", "type": "flag", "value": true, "operator": "flag_evaluates_to", + "dependency_chain": ["dep"]}], + "rollout_percentage": 100, "variant": "a"}, + {"properties": [{{{string.Join(", ", secondConditionFilters)}}}], "rollout_percentage": 100, "variant": "b"}]}} + """; + + static string DepWithFilter(string filter) => $$$""" + {"id": 4, "key": "dep", "active": true, + "filters": {"groups": [{"properties": [{{{filter}}}], "rollout_percentage": 100}]}} + """; + + static async Task ReadTargetVariantAsync(PostHogClient client, string read) + { + var personProperties = new Dictionary { ["plan"] = "pro" }; + if (read == "get-feature-flag") + { +#pragma warning disable CS0618 + var flag = await client.GetFeatureFlagAsync( + "target", + "user-123", + new FeatureFlagOptions { PersonProperties = personProperties }, + CancellationToken.None); +#pragma warning restore CS0618 + return flag?.VariantKey; + } + + var snapshot = await client.EvaluateFlagsAsync( + "user-123", + new AllFeatureFlagsOptions + { + PersonProperties = personProperties, + FlagKeysToEvaluate = read == "scoped-snapshot" ? ["target"] : null + }, + CancellationToken.None); + return snapshot.GetFlag("target")?.VariantKey; + } + + [Theory] + [InlineData("get-feature-flag")] + [InlineData("scoped-snapshot")] + [InlineData("all-flags-snapshot")] + public async Task FlagWhoseDependencyNeedsServerDataFallsBackToTheServer(string read) + { + var container = CreateContainer(DepWithFilter(StaticCohort), TargetDependsOnDep); + var flagsHandler = container.FakeHttpMessageHandler.AddFlagsResponse( + """{"featureFlags": {"dep": true, "target": "a"}}"""); + var client = container.Activate(); + + Assert.Equal("a", await ReadTargetVariantAsync(client, read)); + Assert.Single(flagsHandler.ReceivedRequests); + } + + [Theory] + [InlineData("get-feature-flag")] + [InlineData("scoped-snapshot")] + [InlineData("all-flags-snapshot")] + public async Task FlagWhoseDependencyNeedsServerDataFallsBackToTheServerDespiteMalformedLaterCondition(string read) + { + var malformedDependency = """ + {"key": null, "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["dep"]} + """; + var container = CreateContainer(DepWithFilter(StaticCohort), TargetDependsOnDepThen(malformedDependency)); + var flagsHandler = container.FakeHttpMessageHandler.AddFlagsResponse( + """{"featureFlags": {"dep": true, "target": "a"}}"""); + var client = container.Activate(); + + Assert.Equal("a", await ReadTargetVariantAsync(client, read)); + Assert.Single(flagsHandler.ReceivedRequests); + // The load-time warning logs the definition it cannot read. Evaluation must not log an error. + Assert.DoesNotContain( + container.FakeLoggerProvider.GetAllEvents(minimumLevel: LogLevel.Error), + e => e.CategoryName != FakeLoggerProvider.GetCategoryName()); + } + + [Theory] + [InlineData("get-feature-flag")] + [InlineData("scoped-snapshot")] + [InlineData("all-flags-snapshot")] + public async Task FlagWhoseDependencyResolvesLocallyDoesNotCallTheServer(string read) + { + var container = CreateContainer(DepWithFilter(PlanIsPro), TargetDependsOnDep); + var flagsHandler = container.FakeHttpMessageHandler.AddFlagsResponse( + """{"featureFlags": {"dep": true, "target": "b"}}"""); + var client = container.Activate(); + + Assert.Equal("a", await ReadTargetVariantAsync(client, read)); + Assert.Empty(flagsHandler.ReceivedRequests); + } + + [Fact] + public async Task LocalOnlyEvaluationReportsFlagWhoseDependencyNeedsServerDataAsUnresolvedDependency() + { + var container = CreateContainer(DepWithFilter(StaticCohort), TargetDependsOnDep); + var flagsHandler = container.FakeHttpMessageHandler.AddFlagsResponse( + """{"featureFlags": {"dep": true, "target": "a"}}"""); + var client = container.Activate(); + + var snapshot = await client.EvaluateFlagsAsync("user-123", LocalOnly, CancellationToken.None); + + Assert.DoesNotContain("target", snapshot.Keys); + Assert.Equal(UnresolvedFlagReason.UnresolvedDependency, snapshot.UnresolvedFlags["target"]); + Assert.Empty(flagsHandler.ReceivedRequests); + } + + [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..892ef113 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,403 @@ 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 DependsOnMembers = """ + {"key": "members", "type": "flag", "value": true, "operator": "flag_evaluates_to", "dependency_chain": ["members"]} + """; + 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"]} + """; + const string DistinctIdIsUser = """{"key": "distinct_id", "type": "person", "value": "user-123", "operator": "exact"}"""; + const string DistinctIdIsSomeoneElse = """{"key": "distinct_id", "type": "person", "value": "user-456", "operator": "exact"}"""; + const string InCohortThatMatchesBeforeUnknownOperator = """{"key": "id", "type": "cohort", "value": 1}"""; + + 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]; + yield return [new[] { Condition(DependsOnMembers), Condition(PlanIsPro) }, UnresolvedFlagReason.UnresolvedDependency]; + yield return [new[] { Condition(PlanIsPro), Condition(DependsOnMembers) }, UnresolvedFlagReason.UnresolvedDependency]; + yield return [new[] { Condition(DependsOnMembers), Condition(UnknownOperator) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(UnknownOperator), Condition(DependsOnMembers) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(DistinctIdIsSomeoneElse, UnknownOperator), Condition(DependsOnMembers) }, 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]; + yield return [new[] { Condition(DependsOnMembers, UnknownOperator) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(UnknownOperator, DependsOnMembers) }, UnresolvedFlagReason.UnsupportedDefinition]; + // Within cohorts. + yield return [new[] { Condition(DependsOnMembers, InCohortThatMatchesBeforeUnknownOperator) }, UnresolvedFlagReason.UnsupportedDefinition]; + yield return [new[] { Condition(InCohortThatMatchesBeforeUnknownOperator, DependsOnMembers) }, UnresolvedFlagReason.UnsupportedDefinition]; + } + + [Theory] + [MemberData(nameof(PrecedenceCases))] + public void ReportsTheHighestPrecedenceReasonWhateverTheOrder(string[] conditions, UnresolvedFlagReason expected) + { + var cohorts = $$$""" + {"1": {"type": "OR", "values": [{"type": "OR", "values": [{{{DistinctIdIsUser}}}, {{{UnknownOperator}}}]}]}} + """; + var evaluator = CreateEvaluatorWithCohorts( + cohorts, + CheckoutWithExperienceContinuity, + Flag("members", Condition(StaticCohort)), + 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); + } + + [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")] + [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)] + [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)}}}]}]}}"""; + 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 DependencyThatRequiresServerForcesServerFallbackAsUnresolvedDependency() + { + var evaluator = CreateEvaluator( + Flag("checkout", Condition(StaticCohort)), + Flag("beta-ui-with-fallback", Condition(DependsOnCheckout), Condition())); + + var exception = Assert.Throws( + () => evaluator.EvaluateFeatureFlag("beta-ui-with-fallback", "user-123")); + Assert.Equal(UnresolvedFlagReason.UnresolvedDependency, exception.Reason); + } + + [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]; + // 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]; + 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 [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, + "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"}]}]}, + "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); + 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..f2504f76 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,156 @@ 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(); + 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") + { + var notModified = container.FakeHttpMessageHandler.AddLocalEvaluationNotModifiedResponse(); + await client.LoadFeatureFlagsAsync(CancellationToken.None); + Assert.Single(notModified.ReceivedRequests); + } + else + { + 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); + } + + [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));