diff --git a/.changeset/light-flags-revalidate.md b/.changeset/light-flags-revalidate.md new file mode 100644 index 000000000..abeab6324 --- /dev/null +++ b/.changeset/light-flags-revalidate.md @@ -0,0 +1,5 @@ +--- +"@reflag/node-sdk": patch +--- + +Use ETags when refreshing flag definitions to avoid downloading and recompiling unchanged flags. Custom HTTP clients can opt in by returning the response ETag. diff --git a/packages/node-sdk/README.md b/packages/node-sdk/README.md index a2f756e9f..67924ed3c 100644 --- a/packages/node-sdk/README.md +++ b/packages/node-sdk/README.md @@ -180,7 +180,19 @@ This behavior is built in and does not currently require configuration. Flag definitions include the rules needed to determine which flags should be enabled and which config values should be applied to any given user/company. Flag definitions are automatically fetched when calling `initialize()`. -They are then cached and refreshed in the background. +They are then cached and refreshed in the background. Ordinary refreshes use +ETags to avoid downloading unchanged definitions. A `304 Not Modified` response +keeps the cached definitions and resets their freshness without recompiling +rules or writing a new fallback snapshot. Initial fetches and refreshes with a +`waitForVersion` target remain unconditional. When a fallback provider is +configured, the SDK retains the ETag only after the snapshot is saved +successfully. A failed or pending save keeps ordinary refreshes unconditional +so persistence can recover, without delaying flag availability. + +Custom HTTP clients can enable this optimization by returning the exact ETag +value in `HttpClientResponse.etag` and returning 304 responses with +`body: undefined`. Clients that omit the ETag continue using full fetches. + It's also possible to get the currently in use flag definitions: ```typescript diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index 5791e4d7e..cc4c9f3f9 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -20,7 +20,7 @@ import { SDK_VERSION_HEADER_NAME, } from "./config"; import fetchClient, { withRetry } from "./fetch-http-client"; -import { FlagsCache } from "./flags-cache"; +import { FlagsCache, type FlagsCacheSnapshot } from "./flags-cache"; import { createFlagsSyncController, FlagsSyncController, @@ -292,6 +292,7 @@ export class ReflagClient { private flagsSyncController: FlagsSyncController; private initializationFinished = false; private canLoadFlagsFallbackProvider = true; + private flagsFallbackSavePromise = Promise.resolve(); private _initialize = once(async () => { const start = Date.now(); if (!this._config.offline) { @@ -536,10 +537,20 @@ export class ReflagClient { ? "features" : `features?waitForVersion=${encodeURIComponent(String(waitForVersion))}`; - const res = await this.get( + // A 304 cannot prove that a newer requested version was reached. + // Keep version-targeted fetches unconditional. + const etag = + waitForVersion === undefined ? this.flagsCache.getETag() : undefined; + const response = await this.getResponse( path, this._config.flagsFetchRetries, + { etag }, ); + if (response?.status === 304) { + return { notModified: true }; + } + + const res = response?.body; const flagStateVersion = res?.flagStateVersion; if ( !isObject(res) || @@ -554,12 +565,21 @@ export class ReflagClient { : undefined; } - void this.saveFlagsFallbackDefinitions(res.features); - this.canLoadFlagsFallbackProvider = false; - return { + if (flagStateVersion < (this.flagsCache.getFlagStateVersion() ?? -1)) { + return undefined; + } + + const snapshot: FlagsCacheSnapshot = { definitions: compileFlagDefinitions(res.features), flagStateVersion, }; + this.saveFlagsFallbackDefinitions( + res.features, + snapshot, + response?.etag, + ); + this.canLoadFlagsFallbackProvider = false; + return snapshot; }, { logger: this.logger, @@ -627,23 +647,33 @@ export class ReflagClient { } } - private async saveFlagsFallbackDefinitions(flags: FlagAPIResponse[]) { - const provider = this._config.flagsFallbackProvider; - if (!provider) { - return; - } - - try { - await provider.save( - this._config.flagsFallbackProviderContext, - createFlagsFallbackSnapshot(flags), - ); - } catch (error) { - this.logger.error( - "flagsFallbackProvider: failed to save flag definitions", - error, - ); - } + private saveFlagsFallbackDefinitions( + flags: FlagAPIResponse[], + snapshot: FlagsCacheSnapshot, + etag?: string, + ) { + // Serialize background writes so an older save cannot overwrite a newer one. + this.flagsFallbackSavePromise = this.flagsFallbackSavePromise.then( + async () => { + try { + const provider = this._config.flagsFallbackProvider; + if (provider) { + await provider.save( + this._config.flagsFallbackProviderContext, + createFlagsFallbackSnapshot(flags), + ); + } + // Enable revalidation only after saving. A late save updates only its + // own snapshot, never a replacement or destroyed cache. + snapshot.etag = etag; + } catch (error) { + this.logger.error( + "flagsFallbackProvider: failed to save flag definitions", + error, + ); + } + }, + ); } /** @@ -1180,18 +1210,41 @@ export class ReflagClient { * @returns The response from the server. * @throws An error if the path is invalid. **/ - private async get(path: string, retries: number = 3) { + private async getResponse( + path: string, + retries: number = 3, + revalidation?: { etag?: string }, + ) { ok(typeof path === "string" && path.length > 0, "path must be a string"); try { const url = this.buildUrl(path); + const etag = revalidation?.etag; + const headers = etag + ? { ...this._config.headers, "If-None-Match": etag } + : this._config.headers; + let recoveryAvailable = Boolean(revalidation && !etag); + const request = async (requestHeaders: Record) => { + const response = await this.httpClient.get< + TResponse & { success: boolean } + >(url, requestHeaders, this._config.fetchTimeoutMs); + this.logger.debug(`get request to "${url}"`, response); + return response; + }; + return await withRetry( async () => { - const response = await this.httpClient.get< - TResponse & { success: boolean } - >(url, this._config.headers, this._config.fetchTimeoutMs); + let response = await request(headers); + if (response.status === 304 && etag) { + return response; + } - this.logger.debug(`get request to "${url}"`, response); + // An unconditional definition request should not receive a 304. + // Recover once before falling back to normal error handling. + if (response.status === 304 && recoveryAvailable) { + recoveryAvailable = false; + response = await request(this._config.headers); + } if ( !response.ok || @@ -1202,8 +1255,7 @@ export class ReflagClient { `invalid response received from server for "${url}": ${JSON.stringify(response.body)}`, ); } - const { success: _, ...result } = response.body; - return result as TResponse; + return response; }, (error) => { this.logger.warn("failed to fetch flags, will retry", error); @@ -1709,9 +1761,10 @@ export class ReflagClient { } const query = params.toString(); - const res = await this.get( + const response = await this.getResponse( `features/evaluated${query ? `?${query}` : ""}`, ); + const res = response?.body; if (key) { const flag = res?.features[key]; diff --git a/packages/node-sdk/src/fetch-http-client.ts b/packages/node-sdk/src/fetch-http-client.ts index 6d3d26df7..787e969e3 100644 --- a/packages/node-sdk/src/fetch-http-client.ts +++ b/packages/node-sdk/src/fetch-http-client.ts @@ -47,15 +47,16 @@ const fetchClient: HttpClient = { signal: AbortSignal.timeout(timeoutMs), // We must use no-cache to avoid services such as Next.js from caching the response indefinitely. // We also can't use no-store because of Next.js error withRetry https://github.com/vercel/next.js/discussions/54036. - // We also have local caching in the SDKs, so we don't need to cache the response. + // The SDK caches definitions and explicitly revalidates them using ETags. cache: "no-cache", }); - const json = await response.json(); + const body = response.status === 304 ? undefined : await response.json(); return { ok: response.ok, status: response.status, - body: json as TResponse, + body: body as TResponse | undefined, + etag: response.headers.get("etag") ?? undefined, }; }, }; diff --git a/packages/node-sdk/src/flags-cache.ts b/packages/node-sdk/src/flags-cache.ts index 88619581e..21713f918 100644 --- a/packages/node-sdk/src/flags-cache.ts +++ b/packages/node-sdk/src/flags-cache.ts @@ -2,11 +2,15 @@ import type { CachedFlagDefinition, Logger } from "./types"; const DEFAULT_MIN_REFRESH_INTERVAL_MS = 1000; -type FlagsCacheRefreshResult = { - definitions: CachedFlagDefinition[]; - flagStateVersion?: number; +/** ETags belong to their snapshot, even when persistence publishes them later. */ +export type FlagsCacheSnapshot = { + readonly definitions: CachedFlagDefinition[]; + readonly flagStateVersion?: number; + etag?: string; }; +type FlagsCacheRefreshResult = FlagsCacheSnapshot | { notModified: true }; + type CancelScheduledRefresh = () => void; type ScheduledRefresh = { @@ -36,16 +40,14 @@ type FlagsCacheOptions = { * keep the pending refresh queued until the next caller touches the cache. */ export class FlagsCache { - private value: CachedFlagDefinition[] | undefined; - private flagStateVersion: number | undefined; + private snapshot: FlagsCacheSnapshot | undefined; private refreshPromise: Promise | undefined; private scheduledRefresh: ScheduledRefresh | undefined; private lastRefreshAt: number | undefined; private lastRefreshStartedAt: number | undefined; private destroyed = false; - private pendingFullRefresh = false; - private pendingWaitForVersion: number | undefined; + private pendingRefresh: { waitForVersion?: number } | undefined; private readonly logger?: Logger; private readonly minRefreshIntervalMs: number; @@ -64,22 +66,26 @@ export class FlagsCache { } public get() { - return this.value; + return this.snapshot?.definitions; } public getFlagStateVersion() { - return this.flagStateVersion; + return this.snapshot?.flagStateVersion; + } + + public getETag() { + return this.snapshot?.etag; } public async refresh(waitForVersion?: number) { if (this.destroyed) { - return this.value; + return this.get(); } this.queueRefresh(waitForVersion); this.ensureRefreshStartedOrScheduled(); await this.waitForQueuedWork(); - return this.value; + return this.get(); } public async waitRefresh() { @@ -88,12 +94,10 @@ export class FlagsCache { public destroy() { this.destroyed = true; - this.value = undefined; - this.flagStateVersion = undefined; + this.snapshot = undefined; this.refreshPromise = undefined; this.cancelScheduledRefresh(); - this.pendingFullRefresh = false; - this.pendingWaitForVersion = undefined; + this.pendingRefresh = undefined; this.lastRefreshAt = undefined; this.lastRefreshStartedAt = undefined; } @@ -107,70 +111,46 @@ export class FlagsCache { // can wait for. private queueRefresh(waitForVersion?: number) { if (waitForVersion !== undefined) { - if ( - this.flagStateVersion !== undefined && - this.flagStateVersion >= waitForVersion - ) { + const currentVersion = this.getFlagStateVersion(); + if (currentVersion !== undefined && currentVersion >= waitForVersion) { return; } - this.pendingWaitForVersion = - this.pendingWaitForVersion === undefined - ? waitForVersion - : Math.max(this.pendingWaitForVersion, waitForVersion); - this.pendingFullRefresh = false; - return; - } - - if (this.pendingWaitForVersion !== undefined) { + const pendingVersion = this.pendingRefresh?.waitForVersion; + this.pendingRefresh = { + waitForVersion: + pendingVersion === undefined + ? waitForVersion + : Math.max(pendingVersion, waitForVersion), + }; return; } - this.pendingFullRefresh = true; - } - - private hasPendingRefresh() { - return this.pendingWaitForVersion !== undefined || this.pendingFullRefresh; - } - - private takeNextRefreshRequest() { - if (this.pendingWaitForVersion !== undefined) { - const waitForVersion = this.pendingWaitForVersion; - this.pendingWaitForVersion = undefined; - return { waitForVersion }; - } - - if (!this.pendingFullRefresh) { - return undefined; - } - - this.pendingFullRefresh = false; - return { waitForVersion: undefined }; + this.pendingRefresh ??= {}; } private shouldApplyRefreshResult(flagStateVersion?: number) { + const currentVersion = this.getFlagStateVersion(); if (flagStateVersion === undefined) { - return this.flagStateVersion === undefined; + return currentVersion === undefined; } - return ( - this.flagStateVersion === undefined || - flagStateVersion >= this.flagStateVersion - ); + return currentVersion === undefined || flagStateVersion >= currentVersion; } private clearSatisfiedPendingVersion(seenFlagStateVersion?: number) { - if (this.pendingWaitForVersion === undefined) { + const pendingVersion = this.pendingRefresh?.waitForVersion; + if (pendingVersion === undefined) { return; } const latestKnownVersion = Math.max( - this.flagStateVersion ?? -1, + this.getFlagStateVersion() ?? -1, seenFlagStateVersion ?? -1, ); - if (latestKnownVersion >= this.pendingWaitForVersion) { - this.pendingWaitForVersion = undefined; + if (latestKnownVersion >= pendingVersion) { + this.pendingRefresh = undefined; } } @@ -224,7 +204,7 @@ export class FlagsCache { } private ensureRefreshStartedOrScheduled() { - if (this.destroyed || this.refreshPromise || !this.hasPendingRefresh()) { + if (this.destroyed || this.refreshPromise || !this.pendingRefresh) { return; } @@ -239,11 +219,12 @@ export class FlagsCache { } private startNextRefresh() { - const request = this.takeNextRefreshRequest(); + const request = this.pendingRefresh; if (!request) { return; } + this.pendingRefresh = undefined; this.lastRefreshStartedAt = Date.now(); this.refreshPromise = this.fetchAndApplyRefresh( request.waitForVersion, @@ -259,14 +240,23 @@ export class FlagsCache { return; } + if ("notModified" in result) { + if (!this.snapshot?.etag || waitForVersion !== undefined) { + return; + } + + this.lastRefreshAt = Date.now(); + this.logger?.debug("flag definitions unchanged"); + return; + } + this.clearSatisfiedPendingVersion(result.flagStateVersion); if (!this.shouldApplyRefreshResult(result.flagStateVersion)) { return; } - this.value = result.definitions; - this.flagStateVersion = result.flagStateVersion; + this.snapshot = result; this.lastRefreshAt = Date.now(); this.logger?.info("refreshed flag definitions"); } diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index 006c110c7..6f109c515 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -516,6 +516,12 @@ export type HttpClientResponse = { * The body of the response if available. **/ body: TResponse | undefined; + + /** + * The response's ETag, if available. Return its exact value to enable + * conditional flag-definition refreshes with a custom HTTP client. + **/ + etag?: string; }; /** diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index 459465c5d..02b0fe490 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1188,14 +1188,17 @@ describe("ReflagClient", () => { vi.useFakeTimers(); try { - httpClient.get.mockResolvedValue({ - ok: true, - status: 200, - body: { - success: true, - ...flagDefinitions, - }, - }); + httpClient.get + .mockResolvedValueOnce({ + ok: true, + status: 200, + body: { + success: true, + ...flagDefinitions, + }, + etag: 'W/"flags-v1"', + }) + .mockResolvedValue({ ok: false, status: 304, body: undefined }); const client = new ReflagClient({ ...validOptions, @@ -1212,7 +1215,7 @@ describe("ReflagClient", () => { expect(httpClient.get).toHaveBeenNthCalledWith( 2, "https://api.example.com/features", - expectedHeaders, + { ...expectedHeaders, "If-None-Match": 'W/"flags-v1"' }, API_TIMEOUT_MS, ); diff --git a/packages/node-sdk/test/fetch-http-client.test.ts b/packages/node-sdk/test/fetch-http-client.test.ts index 89b2242c8..f1ffeb6f8 100644 --- a/packages/node-sdk/test/fetch-http-client.test.ts +++ b/packages/node-sdk/test/fetch-http-client.test.ts @@ -1,8 +1,12 @@ +import { createServer } from "node:http"; + import { afterEach, describe, expect, it, vi } from "vitest"; import { API_TIMEOUT_MS } from "../src/config"; import fetchClient from "../src/fetch-http-client"; +const originalFetch = global.fetch; + // mock environment variables vi.mock("../src/config", () => ({ API_TIMEOUT_MS: 100 })); @@ -11,6 +15,8 @@ describe("fetchClient", () => { const headers = { "Content-Type": "application/json" }; afterEach(() => { + global.fetch = originalFetch; + vi.restoreAllMocks(); vi.resetAllMocks(); }); @@ -46,11 +52,17 @@ describe("fetchClient", () => { }); it("should make a GET request and return the response", async () => { - const response = { ok: true, status: 200, body: { success: true } }; + const response = { + ok: true, + status: 200, + body: { success: true }, + etag: 'W/"flags-v1"', + }; global.fetch = vi.fn().mockResolvedValue({ ok: true, status: 200, + headers: new Headers({ ETag: 'W/"flags-v1"' }), json: async () => ({ success: true, }), @@ -90,17 +102,20 @@ describe("fetchClient", () => { }); it("should timeout a GET request that takes too long", async () => { - global.fetch = vi - .fn() - .mockImplementation( - () => - new Promise((resolve) => - setTimeout( - () => resolve({ ok: true, json: async () => ({}) }), - API_TIMEOUT_MS + 100, - ), + global.fetch = vi.fn().mockImplementation( + () => + new Promise((resolve) => + setTimeout( + () => + resolve({ + ok: true, + headers: new Headers(), + json: async () => ({}), + }), + API_TIMEOUT_MS + 100, ), - ); + ), + ); await fetchClient.get(url, headers); expect(vi.mocked(global.fetch).mock.calls[0][1]?.signal?.aborted).toBe( @@ -131,11 +146,13 @@ describe("fetchClient", () => { ok: false, status: 400, body: { error: "Something went wrong" }, + etag: undefined, }; global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 400, + headers: new Headers(), json: async () => ({ error: "Something went wrong" }), } as Response); @@ -144,6 +161,75 @@ describe("fetchClient", () => { expect(result).toEqual(response); }); + it("returns a 304 without attempting to parse an empty JSON body", async () => { + const response = new Response(null, { + status: 304, + headers: { ETag: 'W/"flags-v1"' }, + }); + const json = vi.spyOn(response, "json"); + global.fetch = vi.fn().mockResolvedValue(response); + + const result = await fetchClient.get(url, headers, API_TIMEOUT_MS); + + expect(result).toEqual({ + ok: false, + status: 304, + body: undefined, + etag: 'W/"flags-v1"', + }); + expect(json).not.toHaveBeenCalled(); + }); + + it("supports real conditional GETs while retaining cache: no-cache", async () => { + global.fetch = originalFetch; + const etag = 'W/"flags-v1"'; + const receivedValidators: Array = []; + const server = createServer((request, response) => { + const validator = request.headers["if-none-match"]; + receivedValidators.push( + typeof validator === "string" ? validator : undefined, + ); + response.setHeader("ETag", etag); + response.setHeader("Cache-Control", "max-age=59"); + if (validator === etag) { + response.writeHead(304).end(); + } else { + response.setHeader("Content-Type", "application/json"); + response.end(JSON.stringify({ success: true, features: [] })); + } + }); + await new Promise((resolve) => + server.listen(0, "127.0.0.1", resolve), + ); + + try { + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Expected a TCP server address"); + } + const originUrl = `http://127.0.0.1:${address.port}/features`; + const initial = await fetchClient.get(originUrl, headers, 5000); + const revalidated = await fetchClient.get( + originUrl, + { ...headers, "If-None-Match": initial.etag! }, + 5000, + ); + + expect(initial.status).toBe(200); + expect(initial.etag).toBe(etag); + expect(revalidated).toMatchObject({ + status: 304, + ok: false, + body: undefined, + }); + expect(receivedValidators).toEqual([undefined, etag]); + } finally { + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + } + }); + it("should not handle POST exceptions", async () => { global.fetch = vi.fn().mockRejectedValue(new Error("Network error")); diff --git a/packages/node-sdk/test/flags-cache.test.ts b/packages/node-sdk/test/flags-cache.test.ts index 661b2672b..efc775c80 100644 --- a/packages/node-sdk/test/flags-cache.test.ts +++ b/packages/node-sdk/test/flags-cache.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { FlagsCache } from "../src/flags-cache"; +import { FlagsCache, type FlagsCacheSnapshot } from "../src/flags-cache"; function createDefinitions(key: string) { return [{ key }] as any; @@ -8,13 +8,11 @@ function createDefinitions(key: string) { function createDeferred() { let resolve!: (value: T) => void; - let reject!: (reason?: unknown) => void; - const promise = new Promise((innerResolve, innerReject) => { + const promise = new Promise((innerResolve) => { resolve = innerResolve; - reject = innerReject; }); - return { promise, resolve, reject }; + return { promise, resolve }; } function scheduleTrailingRefresh(delayMs: number, callback: () => void) { @@ -49,6 +47,7 @@ describe("FlagsCache", () => { void cache.refresh(21); void cache.refresh(22); + void cache.refresh(); firstRefresh.resolve({ definitions: createDefinitions("first"), @@ -61,6 +60,7 @@ describe("FlagsCache", () => { }); await refreshPromise; + expect(fetchFlags).toHaveBeenCalledTimes(2); expect(cache.get()).toEqual(createDefinitions("newest")); }); @@ -109,6 +109,95 @@ describe("FlagsCache", () => { expect(cache.get()).toEqual(createDefinitions("newer")); }); + it("does not let a 304 satisfy a pending newer version", async () => { + const revalidation = createDeferred(); + const fetchFlags = vi + .fn() + .mockResolvedValueOnce({ + definitions: createDefinitions("first"), + flagStateVersion: 1, + etag: '"v1"', + }) + .mockImplementationOnce(() => revalidation.promise) + .mockResolvedValueOnce({ + definitions: createDefinitions("newer"), + flagStateVersion: 2, + etag: '"v2"', + }); + const cache = new FlagsCache(fetchFlags, { minRefreshIntervalMs: 0 }); + await cache.refresh(); + const refreshPromise = cache.refresh(); + const versionPromise = cache.refresh(2); + revalidation.resolve({ notModified: true }); + await Promise.all([refreshPromise, versionPromise]); + + expect(fetchFlags).toHaveBeenCalledTimes(3); + expect(fetchFlags).toHaveBeenNthCalledWith(3, 2); + expect(cache.getFlagStateVersion()).toBe(2); + expect(cache.getETag()).toBe('"v2"'); + }); + + it.each([ + ["succeeds", '"old"'], + ["fails", undefined], + ])( + "does not replace a newer validator when an older save %s", + async (_, etag) => { + const definitions = createDefinitions("unchanged"); + const older: FlagsCacheSnapshot = { + definitions, + flagStateVersion: 1, + }; + const fetchFlags = vi + .fn() + .mockResolvedValueOnce(older) + .mockResolvedValueOnce({ + definitions, + flagStateVersion: 2, + etag: '"newer"', + }); + const cache = new FlagsCache(fetchFlags, { minRefreshIntervalMs: 0 }); + await cache.refresh(); + expect(cache.getETag()).toBeUndefined(); + await cache.refresh(); + older.etag = etag; + + expect(cache.getFlagStateVersion()).toBe(2); + expect(cache.getETag()).toBe('"newer"'); + }, + ); + + it("does not restore a late validator after destruction", async () => { + const snapshot: FlagsCacheSnapshot = { + definitions: createDefinitions("first"), + flagStateVersion: 1, + }; + const cache = new FlagsCache(async () => snapshot); + await cache.refresh(); + cache.destroy(); + snapshot.etag = '"first"'; + + expect(cache.get()).toBeUndefined(); + expect(cache.getETag()).toBeUndefined(); + }); + + it("does not apply a refresh or validator after destruction", async () => { + const response = createDeferred(); + const cache = new FlagsCache(() => response.promise); + const refreshPromise = cache.refresh(); + cache.destroy(); + response.resolve({ + definitions: createDefinitions("late"), + flagStateVersion: 1, + etag: '"late"', + }); + await refreshPromise; + + expect(cache.get()).toBeUndefined(); + expect(cache.getETag()).toBeUndefined(); + expect(cache.getLastRefreshAt()).toBeUndefined(); + }); + it("throttles refreshes and runs one trailing refresh when scheduling is supported", async () => { vi.useFakeTimers(); diff --git a/packages/node-sdk/test/flags-revalidation.test.ts b/packages/node-sdk/test/flags-revalidation.test.ts new file mode 100644 index 000000000..7483d79f4 --- /dev/null +++ b/packages/node-sdk/test/flags-revalidation.test.ts @@ -0,0 +1,630 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { ReflagClient } from "../src/client"; +import { API_TIMEOUT_MS, FLAGS_REFETCH_MS } from "../src/config"; +import type { + ClientOptions, + FlagsAPIResponse, + FlagsFallbackSnapshot, +} from "../src/types"; + +const etag = 'W/"flags-v1"'; +const notModified = { ok: false, status: 304, body: undefined }; +const definitions: FlagsAPIResponse = { + flagStateVersion: 1, + features: [ + { + key: "flag", + description: null, + targeting: { + version: 1, + rules: [{ filter: { type: "constant", value: true } }], + }, + }, + ], +}; + +function modified(version = 1, responseETag: string | undefined = etag) { + return { + ok: true, + status: 200, + body: { success: true, ...definitions, flagStateVersion: version }, + etag: responseETag, + }; +} + +const httpClient = { get: vi.fn(), post: vi.fn() }; +const logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; +const clients: ReflagClient[] = []; + +function createClient(options: Partial = {}) { + const client = new ReflagClient({ + secretKey: "validSecretKeyWithMoreThan22Chars", + apiBaseUrl: "https://api.example.com/", + httpClient, + logger, + flagsSyncMode: "in-request", + flagsFetchRetries: 0, + offline: false, + batchOptions: { flushOnExit: false }, + ...options, + }); + clients.push(client); + return client; +} + +async function refresh(client: ReflagClient, waitForVersion?: number) { + await vi.advanceTimersByTimeAsync(1001); + await client.refreshFlags(waitForVersion); +} + +function requestHeaders(call: number) { + return httpClient.get.mock.calls[call - 1]?.[1]; +} + +describe("flag-definition revalidation", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-01-01T00:00:00Z")); + httpClient.get.mockReset(); + httpClient.post.mockResolvedValue({ + ok: true, + status: 200, + body: { success: true }, + }); + }); + + afterEach(() => { + for (const client of clients.splice(0)) { + client.destroy(); + } + vi.useRealTimers(); + vi.restoreAllMocks(); + vi.clearAllMocks(); + }); + + it("retains compiled definitions on repeated 304s and replaces them on a changed 200", async () => { + const provider = { + load: vi.fn(), + save: vi.fn().mockResolvedValue(undefined), + }; + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce(notModified) + .mockResolvedValueOnce(notModified) + .mockResolvedValueOnce({ + ...modified(2, '"flags-v2"'), + body: { + success: true, + flagStateVersion: 2, + features: [ + { + ...definitions.features[0], + targeting: { + version: 2, + rules: [{ filter: { type: "constant", value: false } }], + }, + }, + ], + }, + }); + const client = createClient({ flagsFallbackProvider: provider }); + await client.initialize(); + const cache = client["flagsCache"]; + const initialDefinitions = cache.get(); + const initialRefreshAt = cache.getLastRefreshAt(); + + await refresh(client); + expect(cache.get()).toBe(initialDefinitions); + expect(cache.getFlagStateVersion()).toBe(1); + expect(cache.getLastRefreshAt()).toBeGreaterThan(initialRefreshAt!); + expect(cache.getETag()).toBe(etag); + + await refresh(client); + expect(cache.get()).toBe(initialDefinitions); + expect(provider.save).toHaveBeenCalledTimes(1); + expect(provider.load).not.toHaveBeenCalled(); + expect(logger.warn).not.toHaveBeenCalled(); + expect(httpClient.get).toHaveBeenCalledTimes(3); + expect(requestHeaders(1)).not.toHaveProperty("If-None-Match"); + expect(requestHeaders(2)).toEqual({ + ...requestHeaders(1), + "If-None-Match": etag, + }); + expect(requestHeaders(3)).toEqual(requestHeaders(2)); + expect(client["_config"].headers).not.toHaveProperty("If-None-Match"); + + await refresh(client); + expect(cache.get()).not.toBe(initialDefinitions); + expect(cache.getFlagStateVersion()).toBe(2); + expect(cache.getETag()).toBe('"flags-v2"'); + expect(client.getFlag({}, "flag").isEnabled).toBe(false); + expect(provider.save).toHaveBeenCalledTimes(2); + }); + + it("leaves the ETag unset after a failed save so the next refresh naturally retries persistence", async () => { + const error = new Error("temporary storage failure"); + const provider = { + load: vi.fn(), + save: vi.fn().mockRejectedValueOnce(error).mockResolvedValue(undefined), + }; + httpClient.get.mockImplementation(async (_url, headers) => + headers["If-None-Match"] ? notModified : modified(), + ); + const client = createClient({ flagsFallbackProvider: provider }); + await client.initialize(); + + expect(client.getFlag({}, "flag").isEnabled).toBe(true); + expect(client["flagsCache"].getETag()).toBeUndefined(); + expect(provider.save).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenCalledWith( + "flagsFallbackProvider: failed to save flag definitions", + error, + ); + await refresh(client); + + expect(requestHeaders(2)).not.toHaveProperty("If-None-Match"); + expect(provider.save).toHaveBeenCalledTimes(2); + expect(client["flagsCache"].getETag()).toBe(etag); + await refresh(client); + + expect(requestHeaders(3)["If-None-Match"]).toBe(etag); + expect(provider.save).toHaveBeenCalledTimes(2); + }); + + it("publishes live definitions without waiting for a pending fallback save", async () => { + let completeSave!: () => void; + const save = new Promise((resolve) => { + completeSave = resolve; + }); + const provider = { load: vi.fn(), save: vi.fn().mockReturnValue(save) }; + httpClient.get.mockResolvedValue(modified()); + const client = createClient({ flagsFallbackProvider: provider }); + await client.initialize(); + + expect(client.getFlag({}, "flag").isEnabled).toBe(true); + expect(client["flagsCache"].getETag()).toBeUndefined(); + completeSave(); + await vi.advanceTimersByTimeAsync(0); + expect(client["flagsCache"].getETag()).toBe(etag); + }); + + it.each(["succeeds", "fails"])( + "serializes fallback writes when the first save %s, keeping the latest snapshot through 304s", + async (outcome) => { + let completeFirstSave!: () => void; + let rejectFirstSave!: (error: Error) => void; + const firstSave = new Promise((resolve, reject) => { + completeFirstSave = resolve; + rejectFirstSave = reject; + }); + let stored: FlagsFallbackSnapshot | undefined; + const provider = { + load: vi.fn(), + save: vi.fn( + async (_context: unknown, snapshot: FlagsFallbackSnapshot) => { + if (snapshot.flags.length) { + await firstSave; + } + stored = snapshot; + }, + ), + }; + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce({ + ...modified(2, '"v2"'), + body: { success: true, flagStateVersion: 2, features: [] }, + }) + .mockResolvedValue(notModified); + const client = createClient({ flagsFallbackProvider: provider }); + await client.initialize(); + await refresh(client); + + expect(client["flagsCache"].get()).toEqual([]); + expect(client["flagsCache"].getFlagStateVersion()).toBe(2); + expect(client["flagsCache"].getETag()).toBeUndefined(); + expect(provider.save).toHaveBeenCalledTimes(1); + expect(stored).toBeUndefined(); + + if (outcome === "succeeds") { + completeFirstSave(); + } else { + rejectFirstSave(new Error("temporary storage failure")); + } + await vi.advanceTimersByTimeAsync(0); + expect(provider.save).toHaveBeenCalledTimes(2); + expect(stored?.flags).toEqual([]); + expect(client["flagsCache"].getETag()).toBe('"v2"'); + await refresh(client); + + expect(requestHeaders(3)["If-None-Match"]).toBe('"v2"'); + expect(provider.save).toHaveBeenCalledTimes(2); + expect(stored?.flags).toEqual([]); + }, + ); + + it("keeps late fallback saves scoped to their original snapshots", async () => { + let completeFirstSave!: () => void; + let completeSecondSave!: () => void; + const provider = { + load: vi.fn(), + save: vi + .fn() + .mockReturnValueOnce( + new Promise((resolve) => { + completeFirstSave = resolve; + }), + ) + .mockReturnValueOnce( + new Promise((resolve) => { + completeSecondSave = resolve; + }), + ), + }; + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce(modified(2, '"v2"')); + const client = createClient({ flagsFallbackProvider: provider }); + await client.initialize(); + await refresh(client); + + completeFirstSave(); + await vi.advanceTimersByTimeAsync(0); + expect(client["flagsCache"].getFlagStateVersion()).toBe(2); + expect(client["flagsCache"].getETag()).toBeUndefined(); + + client.destroy(); + completeSecondSave(); + await vi.advanceTimersByTimeAsync(0); + expect(client["flagsCache"].get()).toBeUndefined(); + expect(client["flagsCache"].getETag()).toBeUndefined(); + }); + + it("revalidates an empty flag snapshot", async () => { + httpClient.get + .mockResolvedValueOnce({ + ...modified(), + body: { success: true, flagStateVersion: 1, features: [] }, + }) + .mockResolvedValueOnce(notModified); + const client = createClient(); + await client.initialize(); + const snapshot = client["flagsCache"].get(); + await refresh(client); + + expect(snapshot).toEqual([]); + expect(client["flagsCache"].get()).toBe(snapshot); + expect(requestHeaders(2)["If-None-Match"]).toBe(etag); + }); + + it("revalidates through the default fetch transport", async () => { + const initial = modified(); + const changed = modified(2, '"flags-v2"'); + const fetch = vi + .spyOn(global, "fetch") + .mockResolvedValueOnce( + new Response(JSON.stringify(initial.body), { + headers: { ETag: etag }, + }), + ) + .mockResolvedValueOnce(new Response(null, { status: 304 })) + .mockResolvedValueOnce( + new Response(JSON.stringify(changed.body), { + headers: { ETag: '"flags-v2"' }, + }), + ); + const client = createClient({ httpClient: undefined }); + await client.initialize(); + const snapshot = client["flagsCache"].get(); + await refresh(client); + + expect(client["flagsCache"].get()).toBe(snapshot); + expect(fetch).toHaveBeenNthCalledWith( + 2, + "https://api.example.com/features", + expect.objectContaining({ + cache: "no-cache", + headers: { ...client["_config"].headers, "If-None-Match": etag }, + }), + ); + await refresh(client); + expect(client["flagsCache"].getFlagStateVersion()).toBe(2); + expect(client["flagsCache"].getETag()).toBe('"flags-v2"'); + expect(fetch).toHaveBeenCalledTimes(3); + }); + + it("does not send definition validators to evaluated flag requests", async () => { + httpClient.get.mockResolvedValueOnce(modified()).mockResolvedValueOnce({ + ok: true, + status: 200, + body: { success: true, features: {}, remoteContextUsed: false }, + }); + const client = createClient(); + await client.initialize(); + expect(await client.getFlagsRemote()).toEqual({}); + + expect(httpClient.get).toHaveBeenNthCalledWith( + 2, + "https://api.example.com/features/evaluated", + requestHeaders(1), + API_TIMEOUT_MS, + ); + expect(requestHeaders(2)).not.toHaveProperty("If-None-Match"); + }); + + it("keeps custom clients without an ETag working", async () => { + httpClient.get.mockResolvedValue({ + ok: true, + status: 200, + body: { success: true, ...definitions }, + }); + const client = createClient(); + await client.initialize(); + await refresh(client); + + expect(httpClient.get).toHaveBeenCalledTimes(2); + expect(requestHeaders(2)).not.toHaveProperty("If-None-Match"); + expect(client.getFlag({}, "flag").isEnabled).toBe(true); + }); + + it("clears the old ETag when an accepted replacement omits it", async () => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce({ ...modified(2), etag: undefined }) + .mockResolvedValueOnce(modified(3)); + const client = createClient(); + await client.initialize(); + await refresh(client); + expect(client["flagsCache"].getETag()).toBeUndefined(); + await refresh(client); + + expect(requestHeaders(2)["If-None-Match"]).toBe(etag); + expect(requestHeaders(3)).not.toHaveProperty("If-None-Match"); + }); + + it("does not overwrite fallback storage with a rejected older snapshot before a 304", async () => { + const current = modified(2, '"v2"'); + const provider = { + load: vi.fn(), + save: vi.fn().mockResolvedValue(undefined), + }; + httpClient.get + .mockResolvedValueOnce(current) + .mockResolvedValueOnce({ + ...modified(1, '"v1"'), + body: { success: true, flagStateVersion: 1, features: [] }, + }) + .mockResolvedValueOnce(notModified); + const client = createClient({ flagsFallbackProvider: provider }); + await client.initialize(); + const snapshot = client["flagsCache"].get(); + await refresh(client); + await refresh(client); + + expect(client["flagsCache"].get()).toBe(snapshot); + expect(client["flagsCache"].getFlagStateVersion()).toBe(2); + expect(client["flagsCache"].getETag()).toBe('"v2"'); + expect(requestHeaders(3)["If-None-Match"]).toBe('"v2"'); + expect(provider.save).toHaveBeenCalledTimes(1); + expect(provider.save.mock.calls[0]?.[1].flags).toEqual( + current.body.features, + ); + }); + + it.each([ + ["an HTTP error", { ok: false, status: 500, body: { success: false } }], + [ + "malformed definitions", + { + ...modified(2, '"invalid"'), + body: { success: true, flagStateVersion: 2, features: null }, + }, + ], + ["an older version", modified(0, '"older"')], + ])("preserves the snapshot and ETag after %s", async (_, response) => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce(response) + .mockResolvedValueOnce(notModified); + const client = createClient(); + await client.initialize(); + const cache = client["flagsCache"]; + const snapshot = cache.get(); + const lastRefreshAt = cache.getLastRefreshAt(); + await refresh(client); + + expect(cache.get()).toBe(snapshot); + expect(cache.getETag()).toBe(etag); + expect(cache.getLastRefreshAt()).toBe(lastRefreshAt); + await refresh(client); + expect(requestHeaders(3)["If-None-Match"]).toBe(etag); + expect(cache.get()).toBe(snapshot); + expect(cache.getFlagStateVersion()).toBe(1); + }); + + it("preserves the validator after a network failure", async () => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockRejectedValueOnce(new Error("network failure")) + .mockResolvedValueOnce(notModified); + const client = createClient(); + await client.initialize(); + const lastRefreshAt = client["flagsCache"].getLastRefreshAt(); + await refresh(client); + expect(client["flagsCache"].getLastRefreshAt()).toBe(lastRefreshAt); + await refresh(client); + + expect(requestHeaders(3)["If-None-Match"]).toBe(etag); + expect(client.getFlag({}, "flag").isEnabled).toBe(true); + }); + + it("isolates validators between clients with different credentials", async () => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce(modified(2, '"other-environment"')) + .mockResolvedValue(notModified); + const first = createClient(); + const second = createClient({ + secretKey: "otherSecretKeyWithMoreThan22Chars", + }); + await first.initialize(); + await second.initialize(); + await refresh(first); + await second.refreshFlags(); + + expect(requestHeaders(3)["If-None-Match"]).toBe(etag); + expect(requestHeaders(4)["If-None-Match"]).toBe('"other-environment"'); + expect(requestHeaders(3).Authorization).not.toBe( + requestHeaders(4).Authorization, + ); + first.destroy(); + expect(first["flagsCache"].getETag()).toBeUndefined(); + expect(second["flagsCache"].getETag()).toBe('"other-environment"'); + }); + + it("recovers from an unexpected cold-start 304 with one unconditional fetch", async () => { + httpClient.get + .mockResolvedValueOnce(notModified) + .mockResolvedValueOnce(modified()); + const client = createClient(); + await client.initialize(); + + expect(httpClient.get).toHaveBeenCalledTimes(2); + expect(requestHeaders(1)).not.toHaveProperty("If-None-Match"); + expect(requestHeaders(2)).not.toHaveProperty("If-None-Match"); + expect(client.getFlag({}, "flag").isEnabled).toBe(true); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("does not initialize an empty cache or loop on repeated unexpected 304s", async () => { + httpClient.get.mockResolvedValue(notModified); + const client = createClient(); + await client.initialize(); + + expect(httpClient.get).toHaveBeenCalledTimes(2); + expect(client["flagsCache"].get()).toBeUndefined(); + expect(client["flagsCache"].getLastRefreshAt()).toBeUndefined(); + }); + + it("performs only one unexpected-304 recovery across ordinary retries", async () => { + httpClient.get.mockResolvedValue(notModified); + const client = createClient({ flagsFetchRetries: 2 }); + const initialized = client.initialize(); + await vi.advanceTimersByTimeAsync(10000); + await initialized; + + expect(httpClient.get).toHaveBeenCalledTimes(4); + for (const [, headers] of httpClient.get.mock.calls) { + expect(headers).not.toHaveProperty("If-None-Match"); + } + expect(client["flagsCache"].get()).toBeUndefined(); + expect(client["flagsCache"].getLastRefreshAt()).toBeUndefined(); + }); + + it("fetches unconditionally after loading fallback definitions", async () => { + const provider = { + load: vi.fn().mockResolvedValue({ + version: 1, + savedAt: "2025-12-31T00:00:00Z", + flags: definitions.features, + }), + save: vi.fn().mockResolvedValue(undefined), + }; + httpClient.get + .mockResolvedValueOnce({ + ok: false, + status: 503, + body: { success: false }, + }) + .mockResolvedValueOnce(notModified) + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce(notModified); + const client = createClient({ flagsFallbackProvider: provider }); + await client.initialize(); + expect(client["flagsCache"].getETag()).toBeUndefined(); + await refresh(client); + + expect(requestHeaders(2)).not.toHaveProperty("If-None-Match"); + expect(requestHeaders(3)).not.toHaveProperty("If-None-Match"); + expect(provider.save).toHaveBeenCalledTimes(1); + await refresh(client); + expect(requestHeaders(4)["If-None-Match"]).toBe(etag); + expect(provider.save).toHaveBeenCalledTimes(1); + }); + + it("keeps version-targeted refreshes unconditional and uses the new validator afterward", async () => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValueOnce(modified(2, '"flags-v2"')) + .mockResolvedValueOnce(notModified); + const client = createClient(); + await client.initialize(); + await refresh(client, 2); + + expect(httpClient.get).toHaveBeenNthCalledWith( + 2, + "https://api.example.com/features?waitForVersion=2", + requestHeaders(1), + API_TIMEOUT_MS, + ); + expect(client["flagsCache"].getFlagStateVersion()).toBe(2); + await refresh(client); + expect(requestHeaders(3)["If-None-Match"]).toBe('"flags-v2"'); + }); + + it("does not treat an unexpected 304 as reaching a requested version", async () => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValue(notModified); + const client = createClient(); + await client.initialize(); + const lastRefreshAt = client["flagsCache"].getLastRefreshAt(); + await refresh(client, 2); + + expect(httpClient.get).toHaveBeenCalledTimes(3); + expect(requestHeaders(2)).not.toHaveProperty("If-None-Match"); + expect(requestHeaders(3)).not.toHaveProperty("If-None-Match"); + expect(client["flagsCache"].getFlagStateVersion()).toBe(1); + expect(client["flagsCache"].getLastRefreshAt()).toBe(lastRefreshAt); + }); + + it("revalidates on polling intervals without retrying 304s", async () => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValue(notModified); + const client = createClient({ + flagsSyncMode: "polling", + flagsFetchRetries: 2, + }); + await client.initialize(); + await vi.advanceTimersByTimeAsync(FLAGS_REFETCH_MS * 2); + + expect(httpClient.get).toHaveBeenCalledTimes(3); + expect(requestHeaders(2)["If-None-Match"]).toBe(etag); + expect(requestHeaders(3)["If-None-Match"]).toBe(etag); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("resets in-request freshness so subsequent accesses do not immediately refresh again", async () => { + httpClient.get + .mockResolvedValueOnce(modified()) + .mockResolvedValue(notModified); + const client = createClient(); + await client.initialize(); + await vi.advanceTimersByTimeAsync(FLAGS_REFETCH_MS + 1); + client.getFlag({}, "flag"); + await client["flagsCache"].waitRefresh(); + expect(httpClient.get).toHaveBeenCalledTimes(2); + + await vi.advanceTimersByTimeAsync(1001); + client.getFlag({}, "flag"); + await client["flagsCache"].waitRefresh(); + expect(httpClient.get).toHaveBeenCalledTimes(2); + + await vi.advanceTimersByTimeAsync(FLAGS_REFETCH_MS); + client.getFlag({}, "flag"); + await client["flagsCache"].waitRefresh(); + expect(httpClient.get).toHaveBeenCalledTimes(3); + }); +});