From 0d5bdfb3de18f847864b6baa2eb4400e23be0976 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 24 Sep 2026 21:11:30 +0200 Subject: [PATCH 1/3] fix(flag-evaluation): return structured value diagnostics --- .changeset/structured-evaluation-failures.md | 5 + packages/flag-evaluation/src/index.ts | 179 ++++++++++++++++--- packages/flag-evaluation/test/index.test.ts | 156 ++++++++++++++++ 3 files changed, 312 insertions(+), 28 deletions(-) create mode 100644 .changeset/structured-evaluation-failures.md diff --git a/.changeset/structured-evaluation-failures.md b/.changeset/structured-evaluation-failures.md new file mode 100644 index 000000000..bd6117126 --- /dev/null +++ b/.changeset/structured-evaluation-failures.md @@ -0,0 +1,5 @@ +--- +"@reflag/flag-evaluation": patch +--- + +Return structured diagnostics for invalid numeric and date operands and unknown targeting operators instead of writing evaluation failures directly to the console. diff --git a/packages/flag-evaluation/src/index.ts b/packages/flag-evaluation/src/index.ts index 33c69e891..09aafc1c9 100644 --- a/packages/flag-evaluation/src/index.ts +++ b/packages/flag-evaluation/src/index.ts @@ -74,23 +74,26 @@ export type FilterTree = * - "IS_TRUE": Checks if a boolean value is true. * - "IS_FALSE": Checks if a boolean value is false. */ -export type ContextFilterOperator = - | "IS" - | "IS_NOT" - | "ANY_OF" - | "NOT_ANY_OF" - | "CONTAINS" - | "NOT_CONTAINS" - | "GT" - | "LT" - | "AFTER" - | "BEFORE" - | "DATE_AFTER" - | "DATE_BEFORE" - | "SET" - | "NOT_SET" - | "IS_TRUE" - | "IS_FALSE"; +const CONTEXT_FILTER_OPERATORS = [ + "IS", + "IS_NOT", + "ANY_OF", + "NOT_ANY_OF", + "CONTAINS", + "NOT_CONTAINS", + "GT", + "LT", + "AFTER", + "BEFORE", + "DATE_AFTER", + "DATE_BEFORE", + "SET", + "NOT_SET", + "IS_TRUE", + "IS_FALSE", +] as const; + +export type ContextFilterOperator = (typeof CONTEXT_FILTER_OPERATORS)[number]; /** * Represents a filter configuration used to filter data based on specific context. @@ -211,6 +214,24 @@ export type EvaluationError = field: string; operator: ContextFilterOperator | "rolloutPercentage"; message: string; + } + | { + code: "INVALID_CONTEXT_VALUE"; + field: string; + operator: ContextFilterOperator; + message: string; + } + | { + code: "INVALID_TARGETING_VALUE"; + field: string; + operator: ContextFilterOperator; + message: string; + } + | { + code: "UNKNOWN_OPERATOR"; + field: string; + operator: string; + message: string; }; function normalizeArrayElement(value: unknown): string | undefined { @@ -373,6 +394,8 @@ export function hashInt(hashInput: string): number { return Math.floor((value / 0xfffff) * 100000); } +const CONTEXT_FILTER_OPERATOR_SET = new Set(CONTEXT_FILTER_OPERATORS); + const ARRAY_OPERATORS = new Set([ "IS", "IS_NOT", @@ -451,18 +474,11 @@ export function evaluate( ); case "GT": if (isNaN(Number(normalizedFieldValue)) || isNaN(Number(value))) { - // TODO: return error instead? used logger previously - console.error( - `GT operator requires numeric values: ${normalizedFieldValue}, ${value}`, - ); return false; } return Number(normalizedFieldValue) > Number(value); case "LT": if (isNaN(Number(normalizedFieldValue)) || isNaN(Number(value))) { - console.error( - `LT operator requires numeric values: ${normalizedFieldValue}, ${value}`, - ); return false; } return Number(normalizedFieldValue) < Number(value); @@ -482,9 +498,6 @@ export function evaluate( const fieldValueDate = new Date(normalizedFieldValue).getTime(); const valueDate = new Date(value).getTime(); if (isNaN(fieldValueDate) || isNaN(valueDate)) { - console.error( - `${operator} operator requires valid date values: ${normalizedFieldValue}, ${value}`, - ); return false; } return operator === "DATE_AFTER" @@ -512,7 +525,6 @@ export function evaluate( case "IS_FALSE": return normalizedFieldValue == "false"; default: - console.error(`unknown operator: ${operator}`); return false; } } @@ -545,6 +557,104 @@ function addMissingContextFieldError( }); } +type ExpectedValue = "numeric" | "a valid date" | "a numeric day offset"; + +function isExpectedValue(value: string | undefined, expected: ExpectedValue) { + return expected === "a valid date" + ? !isNaN(new Date(value ?? "").getTime()) + : Number.isFinite(Number(value)); +} + +function addInvalidContextValueError( + errors: Map, + field: string, + operator: ContextFilterOperator, + expected: ExpectedValue, +): void { + errors.set(`invalid-context:${field}:${operator}`, { + code: "INVALID_CONTEXT_VALUE", + field, + operator, + message: `Context field "${field}" must be ${expected} for operator "${operator}".`, + }); +} + +function addInvalidTargetingValueError( + errors: Map, + field: string, + operator: ContextFilterOperator, + expected: ExpectedValue, +): void { + errors.set(`invalid-targeting:${field}:${operator}`, { + code: "INVALID_TARGETING_VALUE", + field, + operator, + message: `Targeting value for operator "${operator}" and context field "${field}" must be ${expected}.`, + }); +} + +function addUnknownOperatorError( + errors: Map, + field: string, + operator: string, +): void { + errors.set(`unknown-operator:${field}:${operator}`, { + code: "UNKNOWN_OPERATOR", + field, + operator, + message: `Unknown targeting operator "${operator}" for context field "${field}".`, + }); +} + +function hasValidOperatorValues( + filter: ContextFilter, + normalizedFieldValue: string, + errors: Map, +): boolean { + let contextExpected: ExpectedValue | undefined; + let targetingExpected: ExpectedValue | undefined; + + switch (filter.operator) { + case "GT": + case "LT": + contextExpected = targetingExpected = "numeric"; + break; + case "AFTER": + case "BEFORE": + contextExpected = "a valid date"; + targetingExpected = "a numeric day offset"; + break; + case "DATE_AFTER": + case "DATE_BEFORE": + contextExpected = targetingExpected = "a valid date"; + break; + default: + return true; + } + + const contextValid = isExpectedValue(normalizedFieldValue, contextExpected); + const targetingValid = isExpectedValue(filter.values?.[0], targetingExpected); + + if (!contextValid) { + addInvalidContextValueError( + errors, + filter.field, + filter.operator, + contextExpected, + ); + } + if (!targetingValid) { + addInvalidTargetingValueError( + errors, + filter.field, + filter.operator, + targetingExpected, + ); + } + + return contextValid && targetingValid; +} + function evaluateRecursively( filter: RuleFilter, context: FlattenedContext, @@ -554,6 +664,12 @@ function evaluateRecursively( case "constant": return filter.value; case "context": { + const operator = String(filter.operator); + if (!CONTEXT_FILTER_OPERATOR_SET.has(operator)) { + addUnknownOperatorError(errors, filter.field, operator); + return false; + } + if ( !(filter.field in context) && filter.operator !== "SET" && @@ -572,6 +688,13 @@ function evaluateRecursively( return false; } + if ( + !Array.isArray(normalizedFieldValue) && + !hasValidOperatorValues(filter, normalizedFieldValue, errors) + ) { + return false; + } + return evaluate( normalizedFieldValue, filter.operator, diff --git a/packages/flag-evaluation/test/index.test.ts b/packages/flag-evaluation/test/index.test.ts index 56fc48174..354e0f31a 100644 --- a/packages/flag-evaluation/test/index.test.ts +++ b/packages/flag-evaluation/test/index.test.ts @@ -1,6 +1,7 @@ import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { + ContextFilterOperator, evaluate, evaluateFlagRules, EvaluationParams, @@ -887,6 +888,161 @@ describe("evaluate flag targeting integration ", () => { }); }); + describe("invalid scalar operator values", () => { + it("returns diagnostics for invalid numeric context and targeting values", () => { + const rules: Rule[] = [ + { + value: true, + filter: { + type: "context", + field: "user.age", + operator: "GT", + values: ["not-numeric"], + }, + }, + ]; + const context = { user: { age: "also-not-numeric" } }; + + for (const result of [ + evaluateFlagRules({ flagKey: "numeric", rules, context }), + newEvaluator(rules)(context, "numeric"), + ]) { + expect(result.value).toBeUndefined(); + expect(result.ruleEvaluationResults).toEqual([false]); + expect(result.errors).toEqual([ + { + code: "INVALID_CONTEXT_VALUE", + field: "user.age", + operator: "GT", + message: + 'Context field "user.age" must be numeric for operator "GT".', + }, + { + code: "INVALID_TARGETING_VALUE", + field: "user.age", + operator: "GT", + message: + 'Targeting value for operator "GT" and context field "user.age" must be numeric.', + }, + ]); + } + }); + + it("returns diagnostics for invalid date context and targeting values", () => { + const rules: Rule[] = [ + { + value: true, + filter: { + type: "context", + field: "user.createdAt", + operator: "DATE_AFTER", + values: ["not-a-date"], + }, + }, + ]; + const context = { user: { createdAt: "also-not-a-date" } }; + + for (const result of [ + evaluateFlagRules({ flagKey: "date", rules, context }), + newEvaluator(rules)(context, "date"), + ]) { + expect(result.errors).toEqual([ + { + code: "INVALID_CONTEXT_VALUE", + field: "user.createdAt", + operator: "DATE_AFTER", + message: + 'Context field "user.createdAt" must be a valid date for operator "DATE_AFTER".', + }, + { + code: "INVALID_TARGETING_VALUE", + field: "user.createdAt", + operator: "DATE_AFTER", + message: + 'Targeting value for operator "DATE_AFTER" and context field "user.createdAt" must be a valid date.', + }, + ]); + } + }); + + it("returns a diagnostic for an invalid relative-date offset", () => { + const rules: Rule[] = [ + { + value: true, + filter: { + type: "context", + field: "user.createdAt", + operator: "AFTER", + values: ["not-a-day-offset"], + }, + }, + ]; + const result = evaluateFlagRules({ + flagKey: "relative-date", + rules, + context: { user: { createdAt: "2024-01-10" } }, + }); + + expect(result.errors).toEqual([ + { + code: "INVALID_TARGETING_VALUE", + field: "user.createdAt", + operator: "AFTER", + message: + 'Targeting value for operator "AFTER" and context field "user.createdAt" must be a numeric day offset.', + }, + ]); + }); + + it("returns a diagnostic for an unknown operator", () => { + const rules: Rule[] = [ + { + value: true, + filter: { + type: "context", + field: "user.role", + operator: "UNKNOWN" as ContextFilterOperator, + values: ["admin"], + }, + }, + ]; + const result = evaluateFlagRules({ + flagKey: "unknown-operator", + rules, + context: { user: { role: "admin" } }, + }); + + expect(result.errors).toEqual([ + { + code: "UNKNOWN_OPERATOR", + field: "user.role", + operator: "UNKNOWN", + message: + 'Unknown targeting operator "UNKNOWN" for context field "user.role".', + }, + ]); + }); + + it("does not write invalid evaluations directly to the console", () => { + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + expect(evaluate("not-numeric", "GT", ["also-not-numeric"])).toBe(false); + expect(evaluate("not-a-date", "DATE_AFTER", ["also-not-a-date"])).toBe( + false, + ); + expect( + evaluate("value", "UNKNOWN" as ContextFilterOperator, ["target"]), + ).toBe(false); + expect(consoleError).not.toHaveBeenCalled(); + } finally { + consoleError.mockRestore(); + } + }); + }); + describe("DATE_AFTER and DATE_BEFORE in flag rules", () => { it("should evaluate DATE_AFTER operator in flag rules", () => { const res = evaluateFlagRules({ From b76a0b61440d87411f86e0a8f89f1840c6bf7b79 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Fri, 25 Sep 2026 12:11:29 +0200 Subject: [PATCH 2/3] refactor(flag-evaluation): remove redundant value guards --- packages/flag-evaluation/src/index.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/flag-evaluation/src/index.ts b/packages/flag-evaluation/src/index.ts index 09aafc1c9..fa8935578 100644 --- a/packages/flag-evaluation/src/index.ts +++ b/packages/flag-evaluation/src/index.ts @@ -473,14 +473,8 @@ export function evaluate( !normalizedFieldValue.toLowerCase().includes(value.toLowerCase()) ); case "GT": - if (isNaN(Number(normalizedFieldValue)) || isNaN(Number(value))) { - return false; - } return Number(normalizedFieldValue) > Number(value); case "LT": - if (isNaN(Number(normalizedFieldValue)) || isNaN(Number(value))) { - return false; - } return Number(normalizedFieldValue) < Number(value); case "AFTER": case "BEFORE": { @@ -497,9 +491,6 @@ export function evaluate( case "DATE_BEFORE": { const fieldValueDate = new Date(normalizedFieldValue).getTime(); const valueDate = new Date(value).getTime(); - if (isNaN(fieldValueDate) || isNaN(valueDate)) { - return false; - } return operator === "DATE_AFTER" ? fieldValueDate >= valueDate : fieldValueDate <= valueDate; From 9a5c67d9eee8c20b158d03057515d051e99aed46 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Fri, 25 Sep 2026 12:14:17 +0200 Subject: [PATCH 3/3] fix(flag-evaluation): preserve infinity comparisons --- packages/flag-evaluation/src/index.ts | 2 +- packages/flag-evaluation/test/index.test.ts | 30 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/flag-evaluation/src/index.ts b/packages/flag-evaluation/src/index.ts index fa8935578..e0c7d4745 100644 --- a/packages/flag-evaluation/src/index.ts +++ b/packages/flag-evaluation/src/index.ts @@ -553,7 +553,7 @@ type ExpectedValue = "numeric" | "a valid date" | "a numeric day offset"; function isExpectedValue(value: string | undefined, expected: ExpectedValue) { return expected === "a valid date" ? !isNaN(new Date(value ?? "").getTime()) - : Number.isFinite(Number(value)); + : !isNaN(Number(value)); } function addInvalidContextValueError( diff --git a/packages/flag-evaluation/test/index.test.ts b/packages/flag-evaluation/test/index.test.ts index 354e0f31a..69a4a1aa3 100644 --- a/packages/flag-evaluation/test/index.test.ts +++ b/packages/flag-evaluation/test/index.test.ts @@ -928,6 +928,36 @@ describe("evaluate flag targeting integration ", () => { } }); + it.each([ + ["Infinity", "GT", "1"], + ["1", "LT", "Infinity"], + ] as const)( + "preserves numeric evaluation semantics for %s %s %s", + (contextValue, operator, targetingValue) => { + const rules: Rule[] = [ + { + value: true, + filter: { + type: "context", + field: "value", + operator, + values: [targetingValue], + }, + }, + ]; + const context = { value: contextValue }; + + expect(evaluate(contextValue, operator, [targetingValue])).toBe(true); + for (const result of [ + evaluateFlagRules({ flagKey: "numeric", rules, context }), + newEvaluator(rules)(context, "numeric"), + ]) { + expect(result.value).toBe(true); + expect(result.errors).toBeUndefined(); + } + }, + ); + it("returns diagnostics for invalid date context and targeting values", () => { const rules: Rule[] = [ {