diff --git a/.changeset/send-evaluation-diagnostics.md b/.changeset/send-evaluation-diagnostics.md new file mode 100644 index 000000000..707d666b1 --- /dev/null +++ b/.changeset/send-evaluation-diagnostics.md @@ -0,0 +1,8 @@ +--- +"@reflag/node-sdk": patch +"@reflag/browser-sdk": patch +"@reflag/react-sdk": patch +"@reflag/vue-sdk": patch +--- + +Include non-fatal flag evaluation diagnostics in check events sent by the Node, browser, React, and Vue SDKs, including when flags are evaluated before client initialization. diff --git a/packages/browser-sdk/src/bulkQueue.ts b/packages/browser-sdk/src/bulkQueue.ts index 568178e72..4ce9ff06d 100644 --- a/packages/browser-sdk/src/bulkQueue.ts +++ b/packages/browser-sdk/src/bulkQueue.ts @@ -1,4 +1,5 @@ import { BULK_QUEUE_FLUSH_DELAY_MS, BULK_QUEUE_MAX_SIZE } from "./config"; +import type { CheckEvent } from "./flag/flags"; import { Logger } from "./logger"; import { logResponseError } from "./utils/responseError"; @@ -39,6 +40,7 @@ export type BulkEvent = evalContext?: Record; evalRuleResults?: boolean[]; evalMissingFields?: string[]; + evalErrors?: CheckEvent["evaluationErrors"]; } | { type: "prompt-event"; diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 66b942d65..994fd7ce2 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -402,6 +402,22 @@ const defaultConfig: Config = { bootstrapped: false, }; +const CLIENT_NOT_INITIALIZED_EVALUATION_ERROR = { + code: "CLIENT_NOT_INITIALIZED", + field: "", + message: + "ReflagClient was not initialized before this flag was evaluated. Call initialize() before evaluating flags.", +} as const; + +function withClientInitializationDiagnostic( + errors: CheckEvent["evaluationErrors"], + evaluatedBeforeInitialization: boolean, +): CheckEvent["evaluationErrors"] { + return evaluatedBeforeInitialization + ? [...(errors ?? []), CLIENT_NOT_INITIALIZED_EVALUATION_ERROR] + : errors; +} + /** * A remotely managed configuration value for a flag. */ @@ -485,6 +501,7 @@ function shouldShowToolbar(opts: InitOptions) { */ export class ReflagClient { private state: State = "idle"; + private initializationFinished = false; private contextUpdateLoading = false; private readonly publishableKey: string; private context: ReflagContext; @@ -723,6 +740,7 @@ export class ReflagClient { "ms" + (this.config.offline ? " (offline mode)" : ""), ); + this.initializationFinished = true; this.setState("initialized"); } @@ -1397,6 +1415,10 @@ export class ReflagClient { */ getFlag(flagKey: string): Flag { const f = this.getFlags()[flagKey]; + const evaluatedBeforeInitialization = + !this.initializationFinished && + !this.config.offline && + !this.config.bootstrapped; // eslint-disable-next-line @typescript-eslint/no-this-alias const self = this; @@ -1417,6 +1439,10 @@ export class ReflagClient { version: f?.targetingVersion, ruleEvaluationResults: f?.ruleEvaluationResults, missingContextFields: f?.missingContextFields, + evaluationErrors: withClientInitializationDiagnostic( + f?.evaluationErrors, + evaluatedBeforeInitialization, + ), value, }) .catch(() => { @@ -1432,6 +1458,10 @@ export class ReflagClient { version: f?.config?.version, ruleEvaluationResults: f?.config?.ruleEvaluationResults, missingContextFields: f?.config?.missingContextFields, + evaluationErrors: withClientInitializationDiagnostic( + f?.config?.evaluationErrors, + evaluatedBeforeInitialization, + ), value: f?.config && { key: f.config.key, payload: f.config.payload, diff --git a/packages/browser-sdk/src/flag/flagCache.ts b/packages/browser-sdk/src/flag/flagCache.ts index ddad0fd7a..442e86b61 100644 --- a/packages/browser-sdk/src/flag/flagCache.ts +++ b/packages/browser-sdk/src/flag/flagCache.ts @@ -1,5 +1,5 @@ import { StorageAdapter } from "../storage"; -import { RawFlagOptIn, RawFlags } from "./flags"; +import { RawFlag, RawFlagOptIn, RawFlags } from "./flags"; import { isValidFlagStateVersion } from "./flagStateVersion"; const DEFAULT_STORAGE_KEY = "__reflag_fetched_flags"; @@ -28,6 +28,23 @@ function parseOptIn(optIn: any): RawFlagOptIn | null | undefined { }; } +function isEvaluationErrorArray( + value: any, +): value is NonNullable { + return ( + Array.isArray(value) && + value.every( + (error) => + isObject(error) && + typeof error.code === "string" && + typeof error.field === "string" && + typeof error.message === "string" && + (typeof error.operator === "undefined" || + typeof error.operator === "string"), + ) + ); +} + interface cacheEntry { expireAt: number; staleAt: number; @@ -57,6 +74,10 @@ export function parseAPIFlagsResponse(flagsInput: any): RawFlags | undefined { !Array.isArray(flag.missingContextFields)) || (flag.ruleEvaluationResults && !Array.isArray(flag.ruleEvaluationResults)) || + (typeof flag.evaluationErrors !== "undefined" && + !isEvaluationErrorArray(flag.evaluationErrors)) || + (typeof flag.config?.evaluationErrors !== "undefined" && + !isEvaluationErrorArray(flag.config.evaluationErrors)) || (typeof flag.optInEnabled !== "undefined" && typeof flag.optInEnabled !== "boolean") || (typeof flag.optIn !== "undefined" && typeof optIn === "undefined") @@ -71,6 +92,7 @@ export function parseAPIFlagsResponse(flagsInput: any): RawFlags | undefined { config: flag.config, missingContextFields: flag.missingContextFields, ruleEvaluationResults: flag.ruleEvaluationResults, + evaluationErrors: flag.evaluationErrors, ...(typeof flag.optInEnabled !== "undefined" && { optInEnabled: flag.optInEnabled, }), diff --git a/packages/browser-sdk/src/flag/flags.ts b/packages/browser-sdk/src/flag/flags.ts index 1bdb207ff..8ac9b0448 100644 --- a/packages/browser-sdk/src/flag/flags.ts +++ b/packages/browser-sdk/src/flag/flags.ts @@ -87,9 +87,20 @@ export type RawFlag = { /** * Missing context fields. + * @deprecated Use `evaluationErrors` and check for `MISSING_CONTEXT_FIELD`. */ missingContextFields?: string[]; + /** + * Non-fatal diagnostics produced while evaluating targeting rules. + */ + evaluationErrors?: Array<{ + code: string; + field: string; + operator?: string; + message: string; + }>; + /** * Whether end-user opt-in is enabled for this flag. */ @@ -126,8 +137,14 @@ export type RawFlag = { /** * The missing context fields. + * @deprecated Use `evaluationErrors` and check for `MISSING_CONTEXT_FIELD`. */ missingContextFields?: string[]; + + /** + * Non-fatal diagnostics produced while evaluating targeting rules. + */ + evaluationErrors?: RawFlag["evaluationErrors"]; }; }; @@ -243,8 +260,14 @@ export interface CheckEvent { /** * Missing context fields. + * @deprecated Use `evaluationErrors` and check for `MISSING_CONTEXT_FIELD`. */ missingContextFields?: string[]; + + /** + * Non-fatal diagnostics produced while evaluating the flag. + */ + evaluationErrors?: RawFlag["evaluationErrors"]; } const storageOverridesKey = `__reflag_overrides`; @@ -663,18 +686,13 @@ export class FlagsClient { evalResult: checkEvent.value, evalRuleResults: checkEvent.ruleEvaluationResults, evalMissingFields: checkEvent.missingContextFields, + evalErrors: checkEvent.evaluationErrors, }; if (this.enqueueBulkEvent) { this.enqueueBulkEvent({ type: "feature-flag-event", - action: payload.action, - key: payload.key, - targetingVersion: payload.targetingVersion, - evalContext: payload.evalContext, - evalResult: payload.evalResult, - evalRuleResults: payload.evalRuleResults, - evalMissingFields: payload.evalMissingFields, + ...payload, }).catch((e: any) => { this.logger.warn(`failed to enqueue flag check event`, e); }); diff --git a/packages/browser-sdk/test/flagCache.test.ts b/packages/browser-sdk/test/flagCache.test.ts index 70c795b65..1e794e7cc 100644 --- a/packages/browser-sdk/test/flagCache.test.ts +++ b/packages/browser-sdk/test/flagCache.test.ts @@ -50,6 +50,19 @@ describe("parseAPIFlagsResponse", () => { test("rejects malformed flag entries without throwing", () => { expect(parseAPIFlagsResponse({ flagA: null })).toBeUndefined(); }); + + test("rejects malformed evaluation errors", () => { + expect( + parseAPIFlagsResponse({ + flagA: { + isEnabled: true, + key: "flagA", + targetingVersion: 1, + evaluationErrors: [{ code: "MISSING_CONTEXT_FIELD" }], + }, + }), + ).toBeUndefined(); + }); }); describe("cache", () => { diff --git a/packages/browser-sdk/test/mocks/handlers.ts b/packages/browser-sdk/test/mocks/handlers.ts index 79a202f1e..d1d1a08e3 100644 --- a/packages/browser-sdk/test/mocks/handlers.ts +++ b/packages/browser-sdk/test/mocks/handlers.ts @@ -14,6 +14,13 @@ export const flagResponse = { config: undefined, ruleEvaluationResults: [false, true], missingContextFields: ["field1", "field2"], + evaluationErrors: [ + { + code: "MISSING_CONTEXT_FIELD", + field: "field1", + message: 'Context field "field1" is required.', + }, + ], }, flagB: { isEnabled: true, @@ -25,6 +32,14 @@ export const flagResponse = { payload: { model: "gpt-something", temperature: 0.5 }, ruleEvaluationResults: [true, false, false], missingContextFields: ["field3"], + evaluationErrors: [ + { + code: "UNSUPPORTED_ARRAY_OPERATOR", + field: "field3", + operator: "IS", + message: 'Operator "IS" does not support array values.', + }, + ], }, }, }, diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index b61bf8aae..44e001f70 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -29,6 +29,13 @@ import { server } from "./mocks/server"; const KEY = "123"; +const clientNotInitializedError = { + code: "CLIENT_NOT_INITIALIZED", + field: "", + message: + "ReflagClient was not initialized before this flag was evaluated. Call initialize() before evaluating flags.", +}; + vi.mock("../src/sse"); vi.mock("../src/feedback/promptStorage", () => { return { @@ -449,7 +456,63 @@ describe(`sends "check" events `, () => { }); }); - it(`does not send check events when offline`, async () => { + it("adds diagnostics when flags are evaluated before initialization", () => { + const sendCheckEventSpy = vi.spyOn( + FlagsClient.prototype, + "sendCheckEvent", + ); + const client = new ReflagClient({ publishableKey: KEY }); + + const flag = client.getFlag("flagA"); + expect(flag.isEnabled).toBe(false); + expect(flag.config).toEqual({ key: undefined, payload: undefined }); + + expect(sendCheckEventSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + action: "check-is-enabled", + evaluationErrors: [clientNotInitializedError], + }), + expect.any(Function), + ); + expect(sendCheckEventSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + action: "check-config", + evaluationErrors: [clientNotInitializedError], + }), + expect.any(Function), + ); + }); + + it("does not add initialization diagnostics to bootstrapped evaluations", () => { + const sendCheckEventSpy = vi.spyOn( + FlagsClient.prototype, + "sendCheckEvent", + ); + const client = new ReflagClient({ + publishableKey: KEY, + bootstrappedState: { + context: {}, + flags: { + flagA: { key: "flagA", isEnabled: true }, + }, + }, + }); + + expect(client.getFlag("flagA").isEnabled).toBe(true); + + expect(sendCheckEventSpy).toHaveBeenCalledWith( + expect.objectContaining({ evaluationErrors: undefined }), + expect.any(Function), + ); + }); + + it(`does not send check events or add initialization diagnostics when offline`, () => { + const sendCheckEventSpy = vi.spyOn( + FlagsClient.prototype, + "sendCheckEvent", + ); const postSpy = vi.spyOn(HttpClient.prototype, "post"); const client = new ReflagClient({ @@ -458,11 +521,14 @@ describe(`sends "check" events `, () => { company: { id: "cid" }, offline: true, }); - await client.initialize(); const flagA = client.getFlag("flagA"); expect(flagA.isEnabled).toBe(false); + expect(sendCheckEventSpy).toHaveBeenCalledWith( + expect.objectContaining({ evaluationErrors: undefined }), + expect.any(Function), + ); expect(postSpy).not.toHaveBeenCalled(); }); @@ -498,6 +564,7 @@ describe(`sends "check" events `, () => { version: 1, missingContextFields: ["field1", "field2"], ruleEvaluationResults: [false, true], + evaluationErrors: flagsResult.flagA.evaluationErrors, }, expect.any(Function), ); @@ -529,6 +596,7 @@ describe(`sends "check" events `, () => { evalResult: true, evalRuleResults: [false, true], evalMissingFields: ["field1", "field2"], + evalErrors: flagsResult.flagA.evaluationErrors, }), ]), ); @@ -580,6 +648,7 @@ describe(`sends "check" events `, () => { }, evalRuleResults: [true, false, false], evalMissingFields: ["field3"], + evalErrors: flagsResult.flagB.config?.evaluationErrors, }), ]), ); diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index ac5e84032..5791e4d7e 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -86,6 +86,22 @@ function evaluationErrorsRateLimitKey(errors: EvaluationError[]): string { .join("\n"); } +const CLIENT_NOT_INITIALIZED_EVALUATION_ERROR = { + code: "CLIENT_NOT_INITIALIZED", + field: "", + message: + "ReflagClient was not initialized before this flag was evaluated. Call initialize() before evaluating flags.", +} as const; + +function withClientInitializationDiagnostic( + errors: FlagEvent["evalErrors"], + evaluatedBeforeInitialization: boolean, +): FlagEvent["evalErrors"] { + return evaluatedBeforeInitialization + ? [...(errors ?? []), CLIENT_NOT_INITIALIZED_EVALUATION_ERROR] + : errors; +} + type PartialBy = Omit & Partial>; type FlagOverrideLayer = { id: number; @@ -145,19 +161,7 @@ type BulkEvent = attributes?: Attributes; context?: TrackingMeta; } - | { - type: "feature-flag-event"; - action: "check" | "check-config"; - key: string; - targetingVersion?: number; - evalResult: - | boolean - | { key: string; payload: any } - | { key: undefined; payload: undefined }; - evalContext?: Record; - evalRuleResults?: boolean[]; - evalMissingFields?: string[]; - } + | ({ type: "feature-flag-event" } & FlagEvent) | { type: "event"; event: string; @@ -1253,6 +1257,7 @@ export class ReflagClient { * @param event.evalContext - The evaluation context of the flag to send. * @param event.evalRuleResults - The evaluation rule results of the flag to send. * @param event.evalMissingFields - The evaluation missing fields of the flag to send. + * @param event.evalErrors - The non-fatal evaluation diagnostics of the flag to send. * * @throws An error if the event is invalid. * @@ -1293,6 +1298,10 @@ export class ReflagClient { Array.isArray(event.evalMissingFields), "event missing fields must be an array", ); + ok( + event.evalErrors === undefined || Array.isArray(event.evalErrors), + "event evaluation errors must be an array", + ); const contextKey = new URLSearchParams( flattenJSON(event.evalContext || {}), @@ -1318,13 +1327,7 @@ export class ReflagClient { await this.batchBuffer.add({ type: "feature-flag-event", - action: event.action, - key: event.key, - targetingVersion: event.targetingVersion, - evalContext: event.evalContext, - evalResult: event.evalResult, - evalRuleResults: event.evalRuleResults, - evalMissingFields: event.evalMissingFields, + ...event, }); } @@ -1470,7 +1473,7 @@ export class ReflagClient { ): RawFlags | RawFlag | undefined { checkContextWithTracking(options); - if (!this.initializationFinished) { + if (!this.initializationFinished && !this._config.offline) { this.logger.error("getFlag(s): ReflagClient is not initialized yet."); } @@ -1584,6 +1587,16 @@ export class ReflagClient { const simplifiedConfig = config ? { key: config.key, payload: config.payload } : { key: undefined, payload: undefined }; + const evaluatedBeforeInitialization = + !this.initializationFinished && !this._config.offline; + const flagEvaluationErrors = withClientInitializationDiagnostic( + flag.evaluationErrors, + evaluatedBeforeInitialization, + ); + const configEvaluationErrors = withClientInitializationDiagnostic( + config?.evaluationErrors, + evaluatedBeforeInitialization, + ); return { get isEnabled() { @@ -1599,6 +1612,7 @@ export class ReflagClient { evalContext: context, evalRuleResults: flag.ruleEvaluationResults, evalMissingFields: flag.missingContextFields, + evalErrors: flagEvaluationErrors, }) .catch((err) => { client.logger?.error( @@ -1622,6 +1636,7 @@ export class ReflagClient { evalContext: context, evalRuleResults: config?.ruleEvaluationResults, evalMissingFields: config?.missingContextFields, + evalErrors: configEvaluationErrors, }) .catch((err) => { client.logger?.error( diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index c599f5ef0..006c110c7 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -60,8 +60,19 @@ export type FlagEvent = { /** * The missing fields in the evaluation context (optional). + * @deprecated Use `evalErrors` and check for `MISSING_CONTEXT_FIELD`. **/ evalMissingFields?: string[]; + + /** + * Non-fatal diagnostics produced while evaluating the flag (optional). + **/ + evalErrors?: Array<{ + code: string; + field: string; + operator?: string; + message: string; + }>; }; /** diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index 7a2e0155c..459465c5d 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -43,6 +43,13 @@ const missingContextFieldError = (field: string) => ({ message: `Context field "${field}" is required to evaluate targeting rules.`, }); +const clientNotInitializedError = { + code: "CLIENT_NOT_INITIALIZED", + field: "", + message: + "ReflagClient was not initialized before this flag was evaluated. Call initialize() before evaluating flags.", +}; + vi.mock("../src/rate-limiter", async (importOriginal) => { const original = (await importOriginal()) as any; @@ -1574,6 +1581,49 @@ describe("ReflagClient", () => { }); }); + it("sends diagnostics when flags are evaluated before initialization", async () => { + const context = { + company, + user, + other: otherContext, + }; + + const flag = client.getFlag(context, "key"); + expect(flag.isEnabled).toBe(true); + expect(flag.config).toEqual({ key: undefined, payload: undefined }); + await client.flush(); + + const checkEvents = httpClient.post.mock.calls + .flatMap((call) => call[2]) + .filter((item) => item.type === "feature-flag-event"); + + expect(checkEvents).toEqual([ + expect.objectContaining({ + action: "check", + evalErrors: [clientNotInitializedError], + }), + expect.objectContaining({ + action: "check-config", + evalErrors: [clientNotInitializedError], + }), + ]); + }); + + it("does not add initialization diagnostics when offline", () => { + const offlineClient = new ReflagClient({ + ...validOptions, + offline: true, + }); + const sendFlagEvent = vi.spyOn(offlineClient as any, "sendFlagEvent"); + + expect(offlineClient.getFlag({}, "flag").isEnabled).toBe(false); + + expect(sendFlagEvent).toHaveBeenCalledWith( + expect.objectContaining({ evalErrors: undefined }), + ); + expect(logger.error).not.toHaveBeenCalled(); + }); + it("evaluates percentage rollouts using user.id", async () => { const userRolloutDefinitions: FlagsAPIResponse = { flagStateVersion: 2, @@ -1716,10 +1766,34 @@ describe("ReflagClient", () => { evalContext: context, evalRuleResults: [true], evalMissingFields: [], + evalErrors: undefined, }, ]); }); + it("`isEnabled` sends evaluation errors", async () => { + const context = { + company, + user, + other: otherContext, + }; + + await client.initialize(); + expect(client.getFlag(context, "flag2").isEnabled).toBe(false); + await client.flush(); + + const checkEvents = httpClient.post.mock.calls + .flatMap((call) => call[2]) + .filter((item) => item.action === "check"); + + expect(checkEvents).toEqual([ + expect.objectContaining({ + key: "flag2", + evalErrors: [missingContextFieldError("attributeKey")], + }), + ]); + }); + it("`isEnabled` warns about missing context fields", async () => { const context = { company, @@ -1989,6 +2063,7 @@ describe("ReflagClient", () => { evalContext: context, evalRuleResults: [true], evalMissingFields: [], + evalErrors: undefined, }, ]); }); @@ -2023,6 +2098,7 @@ describe("ReflagClient", () => { evalResult: false, evalRuleResults: undefined, evalMissingFields: undefined, + evalErrors: undefined, }, ]); });