diff --git a/.changeset/array-context-evaluation.md b/.changeset/array-context-evaluation.md new file mode 100644 index 00000000..49e4094f --- /dev/null +++ b/.changeset/array-context-evaluation.md @@ -0,0 +1,6 @@ +--- +"@reflag/flag-evaluation": minor +"@reflag/node-sdk": patch +--- + +Add type-preserving native-array context evaluation with `ANY_OF`, `NOT_ANY_OF`, `SET`, and `NOT_SET` semantics. Unsupported array operators evaluate to false and produce non-fatal diagnostics that the Node SDK surfaces as rate-limited warnings. diff --git a/packages/flag-evaluation/src/index.ts b/packages/flag-evaluation/src/index.ts index 2064963f..2bc050bf 100644 --- a/packages/flag-evaluation/src/index.ts +++ b/packages/flag-evaluation/src/index.ts @@ -74,7 +74,7 @@ export type FilterTree = * - "IS_TRUE": Checks if a boolean value is true. * - "IS_FALSE": Checks if a boolean value is false. */ -type ContextFilterOperator = +export type ContextFilterOperator = | "IS" | "IS_NOT" | "ANY_OF" @@ -197,6 +197,77 @@ export interface Rule { value: T; } +export type NormalizedContextValue = string | string[]; +export type FlattenedContext = Record; + +export type EvaluationError = + | { + code: "MISSING_CONTEXT_FIELD"; + field: string; + message: string; + } + | { + code: "UNSUPPORTED_ARRAY_OPERATOR"; + field: string; + operator: ContextFilterOperator | "rolloutPercentage"; + message: string; + }; + +function normalizeArrayElement(value: unknown): string | undefined { + if (value === undefined) return undefined; + if (value === null) return ""; + if (typeof value !== "object") return String(value); + + // Composite array elements are outside the targeting model. Keep their + // behavior explicit by comparing their JSON encoding. + try { + return JSON.stringify(value); + } catch { + return undefined; + } +} + +function normalizeArray(value: unknown[]): string[] { + return value.flatMap((entry) => { + const normalized = normalizeArrayElement(entry); + return normalized === undefined ? [] : [normalized]; + }); +} + +/** + * Flattens context for evaluation while preserving arrays as leaf values. + * Primitive array elements use the same string coercion as scalar values; + * composite elements are JSON encoded. + */ +export function flattenContext(data: object): FlattenedContext { + const result = Object.create(null) as FlattenedContext; + + function recurse(value: unknown, prop: string): void { + if (value === undefined) return; + + if (value === null) { + result[prop] = ""; + } else if (Array.isArray(value)) { + result[prop] = normalizeArray(value); + } else if (typeof value !== "object") { + result[prop] = String(value); + } else { + const entries = Object.entries(value); + if (entries.length === 0) { + result[prop] = ""; + return; + } + + for (const [key, entry] of entries) { + recurse(entry, prop ? `${prop}.${key}` : key); + } + } + } + + if (Object.keys(data).length > 0) recurse(data, ""); + return result; +} + /** * Flattens a nested JSON object into a single-level object, with keys indicating the nesting levels. * Keys in the resulting object are represented in a dot notation to reflect the nesting structure of the original data. @@ -302,50 +373,79 @@ export function hashInt(hashInput: string): number { return Math.floor((value / 0xfffff) * 100000); } +const ARRAY_OPERATORS = new Set([ + "ANY_OF", + "NOT_ANY_OF", + "SET", + "NOT_SET", +]); + /** - * Evaluates a field value against a specified operator and comparison values. - * - * @param {string} fieldValue - The value to be evaluated. - * @param {ContextFilterOperator} operator - The operator used for the evaluation (e.g., "CONTAINS", "GT"). - * @param {string[]} values - An array of comparison values for evaluation. - * @return {boolean} The result of the evaluation based on the operator and comparison values. + * Evaluates a scalar or array field value against an operator and comparison values. */ export function evaluate( - fieldValue: string, + normalizedFieldValue: NormalizedContextValue, operator: ContextFilterOperator, values: string[], valueSet?: Set, ): boolean { const value = values[0]; + if (Array.isArray(normalizedFieldValue)) { + switch (operator) { + case "ANY_OF": { + const candidates = valueSet ?? new Set(values); + return normalizedFieldValue.some((entry) => candidates.has(entry)); + } + case "NOT_ANY_OF": { + const candidates = valueSet ?? new Set(values); + return !normalizedFieldValue.some((entry) => candidates.has(entry)); + } + case "SET": + return normalizedFieldValue.length > 0; + case "NOT_SET": + return normalizedFieldValue.length === 0; + default: + // Exact, textual, numeric, date, and boolean operators are scalar-only. + // Do not accidentally stringify arrays for comparison. + return false; + } + } + switch (operator) { case "CONTAINS": - return fieldValue.toLowerCase().includes(value.toLowerCase()); + return ( + typeof value === "string" && + normalizedFieldValue.toLowerCase().includes(value.toLowerCase()) + ); case "NOT_CONTAINS": - return !fieldValue.toLowerCase().includes(value.toLowerCase()); + return ( + typeof value === "string" && + !normalizedFieldValue.toLowerCase().includes(value.toLowerCase()) + ); case "GT": - if (isNaN(Number(fieldValue)) || isNaN(Number(value))) { + if (isNaN(Number(normalizedFieldValue)) || isNaN(Number(value))) { // TODO: return error instead? used logger previously console.error( - `GT operator requires numeric values: ${fieldValue}, ${value}`, + `GT operator requires numeric values: ${normalizedFieldValue}, ${value}`, ); return false; } - return Number(fieldValue) > Number(value); + return Number(normalizedFieldValue) > Number(value); case "LT": - if (isNaN(Number(fieldValue)) || isNaN(Number(value))) { + if (isNaN(Number(normalizedFieldValue)) || isNaN(Number(value))) { console.error( - `LT operator requires numeric values: ${fieldValue}, ${value}`, + `LT operator requires numeric values: ${normalizedFieldValue}, ${value}`, ); return false; } - return Number(fieldValue) < Number(value); + return Number(normalizedFieldValue) < Number(value); case "AFTER": case "BEFORE": { // more/less than `value` days ago const daysAgo = new Date(); daysAgo.setDate(daysAgo.getDate() - Number(value)); - const fieldValueDate = new Date(fieldValue).getTime(); + const fieldValueDate = new Date(normalizedFieldValue).getTime(); return operator === "AFTER" ? fieldValueDate > daysAgo.getTime() @@ -353,11 +453,11 @@ export function evaluate( } case "DATE_AFTER": case "DATE_BEFORE": { - const fieldValueDate = new Date(fieldValue).getTime(); + 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: ${fieldValue}, ${value}`, + `${operator} operator requires valid date values: ${normalizedFieldValue}, ${value}`, ); return false; } @@ -366,83 +466,121 @@ export function evaluate( : fieldValueDate <= valueDate; } case "SET": - return fieldValue !== ""; + return normalizedFieldValue !== ""; case "NOT_SET": - return fieldValue === ""; + return normalizedFieldValue === ""; case "IS": - return fieldValue === value; + return normalizedFieldValue === value; case "IS_NOT": - return fieldValue !== value; + return normalizedFieldValue !== value; case "ANY_OF": - return valueSet ? valueSet.has(fieldValue) : values.includes(fieldValue); + return valueSet + ? valueSet.has(normalizedFieldValue) + : values.includes(normalizedFieldValue); case "NOT_ANY_OF": return valueSet - ? !valueSet.has(fieldValue) - : !values.includes(fieldValue); + ? !valueSet.has(normalizedFieldValue) + : !values.includes(normalizedFieldValue); case "IS_TRUE": - return fieldValue == "true"; + return normalizedFieldValue == "true"; case "IS_FALSE": - return fieldValue == "false"; + return normalizedFieldValue == "false"; default: console.error(`unknown operator: ${operator}`); return false; } } +function addUnsupportedArrayOperatorError( + errors: Map, + field: string, + operator: ContextFilterOperator | "rolloutPercentage", +): void { + const message = + operator === "rolloutPercentage" + ? `Percentage rollout does not support array-valued context field "${field}".` + : `Operator ${operator} does not support array-valued context field "${field}".`; + errors.set(`${field}:${operator}`, { + code: "UNSUPPORTED_ARRAY_OPERATOR", + field, + operator, + message, + }); +} + +function addMissingContextFieldError( + errors: Map, + field: string, +): void { + errors.set(`missing:${field}`, { + code: "MISSING_CONTEXT_FIELD", + field, + message: `Context field "${field}" is required to evaluate targeting rules.`, + }); +} + function evaluateRecursively( filter: RuleFilter, - context: Record, - missingContextFieldsSet: Set, + context: FlattenedContext, + errors: Map, ): boolean { switch (filter.type) { case "constant": return filter.value; - case "context": + case "context": { if ( !(filter.field in context) && filter.operator !== "SET" && filter.operator !== "NOT_SET" ) { - missingContextFieldsSet.add(filter.field); + addMissingContextFieldError(errors, filter.field); + return false; + } + + const normalizedFieldValue = context[filter.field] ?? ""; + if ( + Array.isArray(normalizedFieldValue) && + !ARRAY_OPERATORS.has(filter.operator) + ) { + addUnsupportedArrayOperatorError(errors, filter.field, filter.operator); return false; } return evaluate( - context[filter.field] ?? "", + normalizedFieldValue, filter.operator, filter.values || [], filter.valueSet, ); + } case "rolloutPercentage": { if (!(filter.partialRolloutAttribute in context)) { - missingContextFieldsSet.add(filter.partialRolloutAttribute); + addMissingContextFieldError(errors, filter.partialRolloutAttribute); return false; } - const hashVal = hashInt( - `${filter.key}.${context[filter.partialRolloutAttribute]}`, - ); + const normalizedRolloutValue = context[filter.partialRolloutAttribute]; + if (Array.isArray(normalizedRolloutValue)) { + addUnsupportedArrayOperatorError( + errors, + filter.partialRolloutAttribute, + "rolloutPercentage", + ); + return false; + } + const hashVal = hashInt(`${filter.key}.${normalizedRolloutValue}`); return hashVal < filter.partialRolloutThreshold; } - case "group": - return filter.filters.reduce((acc, current) => { - if (filter.operator === "and") { - return ( - acc && - evaluateRecursively(current, context, missingContextFieldsSet) - ); - } - return ( - acc || evaluateRecursively(current, context, missingContextFieldsSet) - ); - }, filter.operator === "and"); + case "group": { + const evaluateChild = (child: RuleFilter) => + evaluateRecursively(child, context, errors); + return filter.operator === "and" + ? filter.filters.every(evaluateChild) + : filter.filters.some(evaluateChild); + } case "negation": - return !evaluateRecursively( - filter.filter, - context, - missingContextFieldsSet, - ); + return !evaluateRecursively(filter.filter, context, errors); default: return false; } @@ -470,10 +608,11 @@ export interface EvaluationParams { * * @property {string} flagKey - The unique key identifying the flag being evaluated. * @property {T | undefined} value - The resolved value of the flag, if the evaluation is successful. - * @property {Record} context - The contextual information used during the evaluation process. + * @property {Record} context - The normalized contextual information used during evaluation. * @property {boolean[]} ruleEvaluationResults - Array indicating the success or failure of each rule evaluated. * @property {string} [reason] - Optional field providing additional explanation regarding the evaluation result. - * @property {string[]} [missingContextFields] - Optional array of context fields that were required but not provided during the evaluation. + * @property {string[]} [missingContextFields] - Legacy array of context fields that were required but not provided during evaluation. + * @property {EvaluationError[]} [errors] - Non-fatal diagnostics for rules that could not be evaluated. */ export interface EvaluationResult { flagKey: string; @@ -481,7 +620,9 @@ export interface EvaluationResult { context: Record; ruleEvaluationResults: boolean[]; reason?: string; + /** @deprecated Use `errors` and check for `MISSING_CONTEXT_FIELD`. */ missingContextFields?: string[]; + errors?: EvaluationError[]; } export function evaluateFlagRules({ @@ -489,14 +630,23 @@ export function evaluateFlagRules({ flagKey, rules, }: EvaluationParams): EvaluationResult { - const flatContext = flattenJSON(context); - const missingContextFieldsSet = new Set(); + const flatContext = flattenContext(context); + const evaluationErrors = new Map(); - const ruleEvaluationResults = rules.map((rule) => - evaluateRecursively(rule.filter, flatContext, missingContextFieldsSet), - ); + const ruleEvaluationResults = rules.map((rule) => { + const ruleErrors = new Map(); + const matched = evaluateRecursively(rule.filter, flatContext, ruleErrors); + for (const [key, error] of ruleErrors) evaluationErrors.set(key, error); + + // An invalid condition must fail the entire rule, even when wrapped in a + // negation or combined with another condition that would otherwise match. + return ruleErrors.size === 0 && matched; + }); - const missingContextFields = Array.from(missingContextFieldsSet); + const errors = Array.from(evaluationErrors.values()); + const missingContextFields = errors.flatMap((error) => + error.code === "MISSING_CONTEXT_FIELD" ? [error.field] : [], + ); const firstMatchedRuleIndex = ruleEvaluationResults.findIndex(Boolean); const firstMatchedRule = @@ -511,6 +661,7 @@ export function evaluateFlagRules({ ? `rule #${firstMatchedRuleIndex} matched` : "no matched rules", missingContextFields, + ...(errors.length > 0 ? { errors } : {}), }; } diff --git a/packages/flag-evaluation/test/index.test.ts b/packages/flag-evaluation/test/index.test.ts index 87fa138e..307143ef 100644 --- a/packages/flag-evaluation/test/index.test.ts +++ b/packages/flag-evaluation/test/index.test.ts @@ -4,6 +4,7 @@ import { evaluate, evaluateFlagRules, EvaluationParams, + flattenContext, flattenJSON, hashInt, newEvaluator, @@ -240,6 +241,14 @@ describe("evaluate flag targeting integration ", () => { reason: "no matched rules", flagKey: "flag", missingContextFields: ["company.id"], + errors: [ + { + code: "MISSING_CONTEXT_FIELD", + field: "company.id", + message: + 'Context field "company.id" is required to evaluate targeting rules.', + }, + ], ruleEvaluationResults: [false], }); }); @@ -267,6 +276,14 @@ describe("evaluate flag targeting integration ", () => { value: undefined, reason: "no matched rules", missingContextFields: ["happening.id"], + errors: [ + { + code: "MISSING_CONTEXT_FIELD", + field: "happening.id", + message: + 'Context field "happening.id" is required to evaluate targeting rules.', + }, + ], ruleEvaluationResults: [false], }); }); @@ -502,6 +519,246 @@ describe("evaluate flag targeting integration ", () => { }, ); + describe("array-valued context", () => { + it("evaluates ANY_OF using array intersection", () => { + const res = evaluateFlagRules({ + flagKey: "role-based-flag", + rules: [ + { + value: true, + filter: { + type: "context", + field: "user.roles", + operator: "ANY_OF", + values: ["admin", "owner"], + }, + }, + ], + context: { + user: { roles: ["viewer", "admin"] }, + }, + }); + + expect(res).toEqual({ + flagKey: "role-based-flag", + value: true, + context: { "user.roles": ["viewer", "admin"] }, + ruleEvaluationResults: [true], + reason: "rule #0 matched", + missingContextFields: [], + }); + }); + + it("keeps JSON-looking strings scalar", () => { + const evaluator = newEvaluator([ + { + value: "matched", + filter: { + type: "context", + field: "user.roles", + operator: "ANY_OF", + values: ['["viewer","admin"]'], + }, + }, + ]); + + expect( + evaluator({ user: { roles: '["viewer","admin"]' } }, "scalar").value, + ).toBe("matched"); + }); + + it("does not apply percentage rollout to arrays", () => { + const res = evaluateFlagRules({ + flagKey: "rollout", + rules: [ + { + value: true, + filter: { + type: "rolloutPercentage", + key: "rollout", + partialRolloutAttribute: "user.ids", + partialRolloutThreshold: 100000, + }, + }, + ], + context: { user: { ids: ["u1"] } }, + }); + + expect(res.value).toBeUndefined(); + expect(res.missingContextFields).toEqual([]); + expect(res.errors).toEqual([ + { + code: "UNSUPPORTED_ARRAY_OPERATOR", + field: "user.ids", + operator: "rolloutPercentage", + message: + 'Percentage rollout does not support array-valued context field "user.ids".', + }, + ]); + }); + + it("returns a non-fatal diagnostic for scalar-only operators", () => { + const res = evaluateFlagRules({ + flagKey: "role-based-flag", + rules: [ + { + value: true, + filter: { + type: "context", + field: "user.roles", + operator: "CONTAINS", + values: ["admin"], + }, + }, + ], + context: { user: { roles: ["admin"] } }, + }); + + expect(res.value).toBeUndefined(); + expect(res.ruleEvaluationResults).toEqual([false]); + expect(res.errors).toEqual([ + { + code: "UNSUPPORTED_ARRAY_OPERATOR", + field: "user.roles", + operator: "CONTAINS", + message: + 'Operator CONTAINS does not support array-valued context field "user.roles".', + }, + ]); + }); + + it("fails the top-level rule when a nested condition produces an error", () => { + const res = evaluateFlagRules({ + flagKey: "invalid-rules", + rules: [ + { + value: "negated-array", + filter: { + type: "negation", + filter: { + type: "context", + field: "user.roles", + operator: "CONTAINS", + values: ["admin"], + }, + }, + }, + { + value: "negated-missing", + filter: { + type: "negation", + filter: { + type: "context", + field: "user.plan", + operator: "IS", + values: ["pro"], + }, + }, + }, + { + value: "matching-or", + filter: { + type: "group", + operator: "or", + filters: [ + { + type: "context", + field: "user.teams", + operator: "IS", + values: ["platform"], + }, + { type: "constant", value: true }, + ], + }, + }, + ], + context: { + user: { roles: ["admin"], teams: ["platform"] }, + }, + }); + + expect(res.value).toBeUndefined(); + expect(res.ruleEvaluationResults).toEqual([false, false, false]); + expect(res.errors?.map(({ code, field }) => ({ code, field }))).toEqual([ + { code: "UNSUPPORTED_ARRAY_OPERATOR", field: "user.roles" }, + { code: "MISSING_CONTEXT_FIELD", field: "user.plan" }, + { code: "UNSUPPORTED_ARRAY_OPERATOR", field: "user.teams" }, + ]); + }); + + it("does not report errors for short-circuited conditions", () => { + const res = evaluateFlagRules({ + flagKey: "conditional-context", + rules: [ + { + value: "admin", + filter: { + type: "group", + operator: "and", + filters: [ + { + type: "context", + field: "user.role", + operator: "SET", + }, + { + type: "context", + field: "user.role", + operator: "IS", + values: ["admin"], + }, + ], + }, + }, + { + value: "fallback", + filter: { + type: "group", + operator: "or", + filters: [ + { type: "constant", value: true }, + { + type: "context", + field: "user.department", + operator: "IS", + values: ["engineering"], + }, + ], + }, + }, + ], + context: { user: {} }, + }); + + expect(res.value).toBe("fallback"); + expect(res.ruleEvaluationResults).toEqual([false, true]); + expect(res.errors).toBeUndefined(); + expect(res.missingContextFields).toEqual([]); + }); + + it("does not report an array leaf as missing", () => { + const res = evaluateFlagRules({ + flagKey: "role-based-flag", + rules: [ + { + value: true, + filter: { + type: "context", + field: "user.roles", + operator: "ANY_OF", + values: ["owner"], + }, + }, + ], + context: { user: { roles: [] } }, + }); + + expect(res.context).toEqual({ "user.roles": [] }); + expect(res.missingContextFields).toEqual([]); + expect(res.value).toBeUndefined(); + }); + }); + describe("DATE_AFTER and DATE_BEFORE in flag rules", () => { it("should evaluate DATE_AFTER operator in flag rules", () => { const res = evaluateFlagRules({ @@ -793,6 +1050,53 @@ describe("operator evaluation", () => { }); } + it.each(["CONTAINS", "NOT_CONTAINS"] as const)( + "returns false for %s without comparison values", + (operator) => { + expect(evaluate("value", operator, [])).toBe(false); + }, + ); + + it.each([ + [["a", "b"], "ANY_OF", ["a"], true], + [["a", "b"], "ANY_OF", ["c"], false], + [["a", "b"], "ANY_OF", ["b", "c"], true], + [["a", "b"], "NOT_ANY_OF", ["c"], true], + [["a", "b"], "NOT_ANY_OF", ["a", "c"], false], + [[], "SET", [], false], + [[], "NOT_SET", [], true], + [[""], "SET", [], true], + [["A"], "ANY_OF", ["a"], false], + [["a", "a"], "ANY_OF", ["a"], true], + ["[1,true]", "ANY_OF", ["1"], false], + ["[1,true]", "ANY_OF", ["true"], false], + ["[1,true]", "ANY_OF", ["[1,true]"], true], + ] as const)( + "evaluates array semantics for %j %s %j", + (fieldValue, operator, values, expected) => { + expect( + evaluate(fieldValue as string | string[], operator, [...values]), + ).toBe(expected); + }, + ); + + it.each([ + "IS", + "IS_NOT", + "CONTAINS", + "NOT_CONTAINS", + "GT", + "LT", + "AFTER", + "BEFORE", + "DATE_AFTER", + "DATE_BEFORE", + "IS_TRUE", + "IS_FALSE", + ] as const)("does not apply scalar operator %s to arrays", (operator) => { + expect(evaluate(["a"], operator, ["a"])).toBe(false); + }); + describe("DATE_AFTER and DATE_BEFORE operators", () => { const dateTests = [ // DATE_AFTER tests @@ -915,6 +1219,68 @@ describe("rollout hash", () => { } }); +describe("flattenContext", () => { + it("preserves arrays as normalized leaf values", () => { + expect( + flattenContext({ + user: { + id: "u1", + roles: ["admin", "editor"], + levels: [1, 2], + states: [true, false], + nullable: [null], + }, + }), + ).toEqual({ + "user.id": "u1", + "user.roles": ["admin", "editor"], + "user.levels": ["1", "2"], + "user.states": ["true", "false"], + "user.nullable": [""], + }); + }); + + it("stores dangerous root keys without mutating the accumulator prototype", () => { + const flattened = flattenContext( + JSON.parse('{"__proto__":["safe"],"constructor":"value"}'), + ); + + expect(Object.getPrototypeOf(flattened)).toBeNull(); + expect(flattened["__proto__"]).toEqual(["safe"]); + expect(flattened.constructor).toBe("value"); + }); + + it("JSON-encodes composite array elements without traversing them", () => { + expect( + flattenContext({ + other: { + values: [{ role: "admin" }, [1, true]], + }, + }), + ).toEqual({ + "other.values": ['{"role":"admin"}', "[1,true]"], + }); + }); + + it("preserves nested scalar and empty-value behavior", () => { + expect( + flattenContext({ + user: { + profile: { region: "eu" }, + emptyObject: {}, + emptyArray: [], + nil: null, + }, + }), + ).toEqual({ + "user.profile.region": "eu", + "user.emptyObject": "", + "user.emptyArray": [], + "user.nil": "", + }); + }); +}); + describe("flattenJSON", () => { it("should handle an empty object correctly", () => { const input = {}; diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index a7923742..2f5eb205 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -1,6 +1,7 @@ import fs from "fs"; import { + EvaluationError, EvaluationResult, flattenJSON, newEvaluator, @@ -71,6 +72,19 @@ import { const reflagConfigDefaultFile = "reflag.config.json"; +function evaluationErrorsRateLimitKey(errors: EvaluationError[]): string { + return errors + .map((error) => + JSON.stringify([ + error.code, + error.field, + "operator" in error ? error.operator : "", + ]), + ) + .sort() + .join("\n"); +} + type PartialBy = Omit & Partial>; type FlagOverrideLayer = { id: number; @@ -1356,56 +1370,87 @@ export class ReflagClient { } /** - * Warns if a flag has targeting rules that require context fields that are missing. + * Warns if a flag or config evaluation produced diagnostics. * - * @param context - The context. * @param flag - The flag to check. */ - private _warnMissingFlagContextFields( - context: Context, - flag: { + private _warnFlagEvaluationDiagnostics(flag: { + key: string; + missingContextFields?: string[]; + evaluationErrors?: EvaluationError[]; + config?: { key: string; missingContextFields?: string[]; - config?: { - key: string; - missingContextFields?: string[]; - }; - }, - ) { - const report: Record = {}; + evaluationErrors?: EvaluationError[]; + }; + }) { + const missingFieldsReport: Record = {}; + const errorReport: Record = {}; const { config, ...flagData } = flag; + const evaluations = [ + { + reportKey: flagData.key, + rateLimitKey: { flagKey: flagData.key }, + missingContextFields: flagData.missingContextFields, + errors: flagData.evaluationErrors, + }, + ...(config + ? [ + { + reportKey: `${flagData.key}.config`, + rateLimitKey: { flagKey: flagData.key, configKey: config.key }, + missingContextFields: config.missingContextFields, + errors: config.evaluationErrors, + }, + ] + : []), + ]; + + for (const evaluation of evaluations) { + if ( + evaluation.missingContextFields?.length && + !evaluation.errors?.some( + ({ code }) => code === "MISSING_CONTEXT_FIELD", + ) && + this.rateLimiter.isAllowed( + hashObject({ + ...evaluation.rateLimitKey, + missingContextFields: JSON.stringify( + [...evaluation.missingContextFields].sort(), + ), + }), + ) + ) { + missingFieldsReport[evaluation.reportKey] = + evaluation.missingContextFields; + } + } - if ( - flagData.missingContextFields?.length && - this.rateLimiter.isAllowed( - hashObject({ - flagKey: flagData.key, - missingContextFields: flagData.missingContextFields, - context, - }), - ) - ) { - report[flagData.key] = flagData.missingContextFields; + if (Object.keys(missingFieldsReport).length > 0) { + this.logger.warn( + `flag targeting rules might not be correctly evaluated due to missing context fields.`, + missingFieldsReport, + ); } - if ( - config?.missingContextFields?.length && - this.rateLimiter.isAllowed( - hashObject({ - flagKey: flagData.key, - configKey: config.key, - missingContextFields: config.missingContextFields, - context, - }), - ) - ) { - report[`${flagData.key}.config`] = config.missingContextFields; + for (const evaluation of evaluations) { + if ( + evaluation.errors?.length && + this.rateLimiter.isAllowed( + hashObject({ + ...evaluation.rateLimitKey, + evaluationErrors: evaluationErrorsRateLimitKey(evaluation.errors), + }), + ) + ) { + errorReport[evaluation.reportKey] = evaluation.errors; + } } - if (Object.keys(report).length > 0) { + if (Object.keys(errorReport).length > 0) { this.logger.warn( - `flag targeting rules might not be correctly evaluated due to missing context fields.`, - report, + "flag targeting rules could not be fully evaluated.", + errorReport, ); } } @@ -1461,6 +1506,7 @@ export class ReflagClient { value: undefined, ruleEvaluationResults: [], missingContextFields: [], + errors: undefined, } satisfies EvaluationResult), })); @@ -1470,6 +1516,9 @@ export class ReflagClient { isEnabled: res.enabledResult.value ?? false, ruleEvaluationResults: res.enabledResult.ruleEvaluationResults, missingContextFields: res.enabledResult.missingContextFields, + ...(res.enabledResult.errors?.length + ? { evaluationErrors: res.enabledResult.errors } + : {}), targetingVersion: res.targetingVersion, config: { key: res.configResult?.value?.key, @@ -1477,6 +1526,9 @@ export class ReflagClient { targetingVersion: res.configVersion, ruleEvaluationResults: res.configResult?.ruleEvaluationResults, missingContextFields: res.configResult?.missingContextFields, + ...(res.configResult?.errors?.length + ? { evaluationErrors: res.configResult.errors } + : {}), }, }; return acc; @@ -1532,7 +1584,7 @@ export class ReflagClient { return { get isEnabled() { if (enableTracking && enableChecks) { - client._warnMissingFlagContextFields(context, flag); + client._warnFlagEvaluationDiagnostics(flag); void client .sendFlagEvent({ @@ -1555,7 +1607,7 @@ export class ReflagClient { }, get config() { if (enableTracking && enableChecks) { - client._warnMissingFlagContextFields(context, flag); + client._warnFlagEvaluationDiagnostics({ ...flag, config }); void client .sendFlagEvent({ diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index 5602d87a..d7bd0123 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -1,6 +1,10 @@ /* eslint-disable @typescript-eslint/no-empty-object-type */ -import { newEvaluator, RuleFilter } from "@reflag/flag-evaluation"; +import { + EvaluationError, + newEvaluator, + RuleFilter, +} from "@reflag/flag-evaluation"; /** * Describes the meta context associated with tracking. @@ -86,8 +90,14 @@ export type RawFlagRemoteConfig = { /** * The missing fields in the evaluation context (optional). + * @deprecated Use `evaluationErrors` and check for `MISSING_CONTEXT_FIELD`. */ missingContextFields?: string[]; + + /** + * Non-fatal diagnostics produced while evaluating targeting rules. + */ + evaluationErrors?: EvaluationError[]; }; /** @@ -121,8 +131,14 @@ export interface RawFlag { /** * The missing fields in the evaluation context (optional). + * @deprecated Use `evaluationErrors` and check for `MISSING_CONTEXT_FIELD`. */ missingContextFields?: string[]; + + /** + * Non-fatal diagnostics produced while evaluating targeting rules. + */ + evaluationErrors?: EvaluationError[]; } /** diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index 0f592eba..9130478b 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -37,6 +37,12 @@ import { const BULK_ENDPOINT = "https://api.example.com/bulk"; +const missingContextFieldError = (field: string) => ({ + code: "MISSING_CONTEXT_FIELD", + field, + message: `Context field "${field}" is required to evaluate targeting rules.`, +}); + vi.mock("../src/rate-limiter", async (importOriginal) => { const original = (await importOriginal()) as any; @@ -1729,9 +1735,140 @@ describe("ReflagClient", () => { expect(flag.isEnabled).toBe(false); expect(logger.warn).toHaveBeenCalledWith( - "flag targeting rules might not be correctly evaluated due to missing context fields.", + "flag targeting rules could not be fully evaluated.", + { + flag2: [ + { + code: "MISSING_CONTEXT_FIELD", + field: "attributeKey", + message: + 'Context field "attributeKey" is required to evaluate targeting rules.', + }, + ], + }, + ); + + const sameErrorForAnotherUser = client.getFlag( + { ...context, user: { ...context.user, id: "another-user" } }, + "flag2", + ); + expect(sameErrorForAnotherUser.isEnabled).toBe(false); + expect(logger.warn).toHaveBeenCalledTimes(1); + }); + + it("`isEnabled` warns about unsupported array operators", async () => { + const arrayDefinitions: FlagsAPIResponse = { + flagStateVersion: 2, + features: [ + { + key: "array-flag", + description: "Array flag", + targeting: { + version: 1, + rules: [ + { + filter: { + type: "context", + field: "user.roles", + operator: "CONTAINS", + values: ["admin"], + }, + }, + ], + }, + }, + ], + }; + httpClient.get.mockResolvedValue({ + ok: true, + status: 200, + body: { success: true, ...arrayDefinitions }, + }); + + await client.initialize(); + const flag = client.getFlag( + { user: { id: "user123", roles: ["admin"] } }, + "array-flag", + ); + + expect(flag.isEnabled).toBe(false); + expect(logger.warn).toHaveBeenCalledWith( + "flag targeting rules could not be fully evaluated.", + { + "array-flag": [ + { + code: "UNSUPPORTED_ARRAY_OPERATOR", + field: "user.roles", + operator: "CONTAINS", + message: + 'Operator CONTAINS does not support array-valued context field "user.roles".', + }, + ], + }, + ); + + const sameErrorForAnotherUser = client.getFlag( + { user: { id: "another-user", roles: ["admin"] } }, + "array-flag", + ); + expect(sameErrorForAnotherUser.isEnabled).toBe(false); + expect(logger.warn).toHaveBeenCalledTimes(1); + }); + + it("`config` warns about unsupported array operators", async () => { + const arrayConfigDefinitions: FlagsAPIResponse = { + flagStateVersion: 2, + features: [ + { + key: "array-config", + description: "Array config", + targeting: { + version: 1, + rules: [{ filter: { type: "constant", value: true } }], + }, + config: { + version: 1, + variants: [ + { + key: "admin", + payload: { access: true }, + filter: { + type: "context", + field: "user.roles", + operator: "CONTAINS", + values: ["admin"], + }, + }, + ], + }, + }, + ], + }; + httpClient.get.mockResolvedValue({ + ok: true, + status: 200, + body: { success: true, ...arrayConfigDefinitions }, + }); + + await client.initialize(); + const flag = client.getFlag( + { user: { id: "user123", roles: ["admin"] } }, + "array-config", + ); + + expect(flag.config).toEqual({ key: undefined, payload: undefined }); + expect(logger.warn).toHaveBeenCalledWith( + "flag targeting rules could not be fully evaluated.", { - flag2: ["attributeKey"], + "array-config.config": [ + { + code: "UNSUPPORTED_ARRAY_OPERATOR", + field: "user.roles", + operator: "CONTAINS", + message: + 'Operator CONTAINS does not support array-valued context field "user.roles".', + }, + ], }, ); }); @@ -2631,6 +2768,7 @@ describe("ReflagClient", () => { }, ruleEvaluationResults: [false], missingContextFields: ["attributeKey"], + evaluationErrors: [missingContextFieldError("attributeKey")], }, }, flagStateVersion: 1, @@ -2666,10 +2804,12 @@ describe("ReflagClient", () => { payload: undefined, targetingVersion: 1, missingContextFields: ["company.id"], + evaluationErrors: [missingContextFieldError("company.id")], ruleEvaluationResults: [false], }, ruleEvaluationResults: [false], missingContextFields: ["company.id"], + evaluationErrors: [missingContextFieldError("company.id")], }, flag2: { key: "flag2", @@ -2684,6 +2824,7 @@ describe("ReflagClient", () => { }, ruleEvaluationResults: [false], missingContextFields: ["company.id"], + evaluationErrors: [missingContextFieldError("company.id")], }, }, flagStateVersion: 1, @@ -2737,6 +2878,7 @@ describe("ReflagClient", () => { }, ruleEvaluationResults: [false], missingContextFields: ["attributeKey"], + evaluationErrors: [missingContextFieldError("attributeKey")], }, }, flagStateVersion: 1, @@ -2766,10 +2908,12 @@ describe("ReflagClient", () => { payload: undefined, targetingVersion: 1, missingContextFields: ["company.id"], + evaluationErrors: [missingContextFieldError("company.id")], ruleEvaluationResults: [false], }, ruleEvaluationResults: [false], missingContextFields: ["company.id"], + evaluationErrors: [missingContextFieldError("company.id")], }, flag2: { key: "flag2", @@ -2784,6 +2928,7 @@ describe("ReflagClient", () => { }, ruleEvaluationResults: [false], missingContextFields: ["company.id"], + evaluationErrors: [missingContextFieldError("company.id")], }, }, flagStateVersion: 1, @@ -3350,6 +3495,7 @@ describe("BoundReflagClient", () => { }, ruleEvaluationResults: [false], missingContextFields: ["attributeKey"], + evaluationErrors: [missingContextFieldError("attributeKey")], }, }, flagStateVersion: 1,