From cd4bc4b9a2645379afa2ce4e6b59d40973214796 Mon Sep 17 00:00:00 2001 From: Erik Hughes Date: Thu, 14 Aug 2025 14:41:06 +0200 Subject: [PATCH 1/6] feat: only warn about missing feature context fields when feature is used - Added `noUncheckedIndexedAccess` option to `tsconfig.json` for stricter type checking. - Modified `getFeatureDefinitions` method to now return synchronous`FeatureDefinition[]`. - Enhanced error handling in `_getFeaturesRemote` method (no longer returns undefined, bug). - Updated tests to reflect changes in feature handling and context warnings. --- packages/node-sdk/package.json | 2 +- packages/node-sdk/src/client.ts | 312 +++++++++--------- packages/node-sdk/src/rate-limiter.ts | 5 +- packages/node-sdk/src/types.ts | 4 +- packages/node-sdk/test/client.test.ts | 106 +++--- packages/node-sdk/tsconfig.json | 3 +- .../openfeature-node-provider/package.json | 4 +- .../openfeature-node-provider/src/index.ts | 2 +- 8 files changed, 214 insertions(+), 224 deletions(-) diff --git a/packages/node-sdk/package.json b/packages/node-sdk/package.json index e5da4fbb..2a520100 100644 --- a/packages/node-sdk/package.json +++ b/packages/node-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/node-sdk", - "version": "1.9.3", + "version": "1.10.0", "license": "MIT", "repository": { "type": "git", diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index 3a97556b..12a334f0 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -31,6 +31,7 @@ import type { FeatureOverridesFn, IdType, RawFeature, + TypedFeatureKey, } from "./types"; import { Attributes, @@ -120,7 +121,7 @@ export class BucketClient { refetchInterval: number; staleWarningInterval: number; headers: Record; - fallbackFeatures?: Record; + fallbackFeatures?: Record; featureOverrides: FeatureOverridesFn; offline: boolean; emitEvaluationEvents: boolean; @@ -260,18 +261,18 @@ export class BucketClient { const fallbackFeatures = Array.isArray(options.fallbackFeatures) ? options.fallbackFeatures.reduce( (acc, key) => { - acc[key as keyof TypedFeatures] = { + acc[key as TypedFeatureKey] = { isEnabled: true, key, }; return acc; }, - {} as Record, + {} as Record, ) : isObject(options.fallbackFeatures) ? Object.entries(options.fallbackFeatures).reduce( (acc, [key, fallback]) => { - acc[key as keyof TypedFeatures] = { + acc[key as TypedFeatureKey] = { isEnabled: typeof fallback === "object" ? fallback.isEnabled @@ -287,7 +288,7 @@ export class BucketClient { }; return acc; }, - {} as Record, + {} as Record, ) : undefined; @@ -608,7 +609,7 @@ export class BucketClient { * * @returns The features definitions. */ - public async getFeatureDefinitions(): Promise { + public getFeatureDefinitions(): FeatureDefinition[] { const features = this.featuresCache.get() || []; return features.map((f) => ({ key: f.key, @@ -637,15 +638,7 @@ export class BucketClient { enableTracking = true, ...context }: ContextWithTracking): TypedFeatures { - const options = { enableTracking, ...context }; - const features = this._getFeatures(options); - - return Object.fromEntries( - Object.entries(features).map(([k, v]) => [ - k as keyof TypedFeatures, - this._wrapRawFeature(options, v), - ]), - ); + return this._getFeatures({ enableTracking, ...context }); } /** @@ -658,25 +651,11 @@ export class BucketClient { * @remarks * Call `initialize` before calling this method to ensure the feature definitions are cached, no features will be returned otherwise. **/ - public getFeature( + public getFeature( { enableTracking = true, ...context }: ContextWithTracking, key: TKey, ): TypedFeatures[TKey] { - const options = { enableTracking, ...context }; - const features = this._getFeatures(options); - const feature = features[key]; - - return this._wrapRawFeature( - { ...options, enableChecks: true }, - { - key, - isEnabled: feature?.isEnabled ?? false, - targetingVersion: feature?.targetingVersion, - config: feature?.config, - ruleEvaluationResults: feature?.ruleEvaluationResults, - missingContextFields: feature?.missingContextFields, - }, - ); + return this._getFeatures({ enableTracking, ...context }, key); } /** @@ -694,8 +673,8 @@ export class BucketClient { companyId?: IdType, additionalContext?: Context, ): Promise { - return await this._getFeaturesRemote( - "", + return this._getFeaturesRemote( + undefined, userId, companyId, additionalContext, @@ -713,20 +692,13 @@ export class BucketClient { * * @returns evaluated feature */ - public async getFeatureRemote( + public async getFeatureRemote( key: TKey, userId?: IdType, companyId?: IdType, additionalContext?: Context, ): Promise { - const features = await this._getFeaturesRemote( - key, - userId, - companyId, - additionalContext, - ); - - return features[key]; + return this._getFeaturesRemote(key, userId, companyId, additionalContext); } private buildUrl(path: string) { @@ -822,7 +794,7 @@ export class BucketClient { `get request to "${path}" failed with error after ${retries} retries`, error, ); - return undefined; + throw error; } } @@ -991,55 +963,51 @@ export class BucketClient { } /** - * Warns if any features have targeting rules that require context fields that are missing. + * Warns if a feature has targeting rules that require context fields that are missing. * * @param context - The context. - * @param features - The features to check. + * @param feature - The feature to check. */ - private warnMissingFeatureContextFields( + private _warnMissingFeatureContextFields( context: Context, - features: { + feature: { key: string; missingContextFields?: string[]; config?: { key: string; missingContextFields?: string[]; }; - }[], + }, ) { - const report = features.reduce( - (acc, { config, ...feature }) => { - if ( - feature.missingContextFields?.length && - this.rateLimiter.isAllowed( - hashObject({ - featureKey: feature.key, - missingContextFields: feature.missingContextFields, - context, - }), - ) - ) { - acc[feature.key] = feature.missingContextFields; - } + const report: Record = {}; + const { config, ...featureData } = feature; - if ( - config?.missingContextFields?.length && - this.rateLimiter.isAllowed( - hashObject({ - featureKey: feature.key, - configKey: config.key, - missingContextFields: config.missingContextFields, - context, - }), - ) - ) { - acc[`${feature.key}.config`] = config.missingContextFields; - } + if ( + featureData.missingContextFields?.length && + this.rateLimiter.isAllowed( + hashObject({ + featureKey: featureData.key, + missingContextFields: featureData.missingContextFields, + context, + }), + ) + ) { + report[featureData.key] = featureData.missingContextFields; + } - return acc; - }, - {} as Record, - ); + if ( + config?.missingContextFields?.length && + this.rateLimiter.isAllowed( + hashObject({ + featureKey: featureData.key, + configKey: config.key, + missingContextFields: config.missingContextFields, + context, + }), + ) + ) { + report[`${featureData.key}.config`] = config.missingContextFields; + } if (Object.keys(report).length > 0) { this.logger.warn( @@ -1049,9 +1017,15 @@ export class BucketClient { } } - private _getFeatures( + private _getFeatures(options: ContextWithTracking): TypedFeatures; + private _getFeatures( options: ContextWithTracking, - ): Record { + key: TKey, + ): TypedFeatures[TKey]; + private _getFeatures( + options: ContextWithTracking, + key?: TKey, + ): TypedFeatures | TypedFeatures[TKey] { checkContextWithTracking(options); if (!this.initializationFinished) { @@ -1067,40 +1041,42 @@ export class BucketClient { this.logger.warn( "no feature definitions available, using fallback features.", ); - return this._config.fallbackFeatures || {}; + const fallbackFeatures = this._config.fallbackFeatures || {}; + if (key) { + return this._wrapRawFeature( + { ...options, enableChecks: true }, + { key, isEnabled: false, ...fallbackFeatures[key] }, + ); + } + return Object.fromEntries( + Object.entries(fallbackFeatures).map(([k, v]) => [ + k as TypedFeatureKey, + this._wrapRawFeature(options, v), + ]), + ); } featureDefinitions = featureDefs; } const { enableTracking = true, meta: _, ...context } = options; - const evaluated = featureDefinitions.map((feature) => ({ - featureKey: feature.key, - targetingVersion: feature.targeting.version, - configVersion: feature.config?.version, - enabledResult: feature.enabledEvaluator(context, feature.key), - configResult: - feature.configEvaluator?.(context, feature.key) ?? - ({ - featureKey: feature.key, - context, - value: undefined, - ruleEvaluationResults: [], - missingContextFields: [], - } satisfies EvaluationResult), - })); - - this.warnMissingFeatureContextFields( - context, - evaluated.map(({ featureKey, enabledResult, configResult }) => ({ - key: featureKey, - missingContextFields: enabledResult.missingContextFields ?? [], - config: { - key: configResult.value, - missingContextFields: configResult.missingContextFields ?? [], - }, - })), - ); + const evaluated = featureDefinitions + .filter(({ key: featureKey }) => (key ? key === featureKey : true)) + .map((feature) => ({ + featureKey: feature.key, + targetingVersion: feature.targeting.version, + configVersion: feature.config?.version, + enabledResult: feature.enabledEvaluator(context, feature.key), + configResult: + feature.configEvaluator?.(context, feature.key) ?? + ({ + featureKey: feature.key, + context, + value: undefined, + ruleEvaluationResults: [], + missingContextFields: [], + } satisfies EvaluationResult), + })); if (enableTracking) { const promises = evaluated @@ -1153,7 +1129,7 @@ export class BucketClient { let evaluatedFeatures = evaluated.reduce( (acc, res) => { - acc[res.featureKey as keyof TypedFeatures] = { + acc[res.featureKey as TypedFeatureKey] = { key: res.featureKey, isEnabled: res.enabledResult.value ?? false, ruleEvaluationResults: res.enabledResult.ruleEvaluationResults, @@ -1169,26 +1145,26 @@ export class BucketClient { }; return acc; }, - {} as Record, + {} as Record, ); // apply feature overrides - const overrides = Object.entries( - this._config.featureOverrides(context), - ).map(([key, override]) => [ - key, - isObject(override) - ? { - key, - isEnabled: override.isEnabled, - config: override.config, - } - : { - key, - isEnabled: !!override, - config: undefined, - }, - ]); + const overrides = Object.entries(this._config.featureOverrides(context)) + .filter(([featureKey]) => (key ? key === featureKey : true)) + .map(([featureKey, override]) => [ + featureKey, + isObject(override) + ? { + key: featureKey, + isEnabled: override.isEnabled, + config: override.config, + } + : { + key: featureKey, + isEnabled: !!override, + config: undefined, + }, + ]); if (overrides.length > 0) { // merge overrides into evaluated features @@ -1198,10 +1174,26 @@ export class BucketClient { }; } - return evaluatedFeatures; + if (key) { + return this._wrapRawFeature( + { ...options, enableChecks: true }, + { + key, + isEnabled: false, + ...evaluatedFeatures[key], + }, + ); + } + + return Object.fromEntries( + Object.entries(evaluatedFeatures).map(([k, v]) => [ + k as TypedFeatureKey, + this._wrapRawFeature(options, v), + ]), + ); } - private _wrapRawFeature( + private _wrapRawFeature( { enableTracking, enableChecks = false, @@ -1219,6 +1211,8 @@ export class BucketClient { return { get isEnabled() { if (enableTracking && enableChecks) { + client._warnMissingFeatureContextFields(context, feature); + void client .sendFeatureEvent({ action: "check", @@ -1240,6 +1234,8 @@ export class BucketClient { }, get config() { if (enableTracking && enableChecks) { + client._warnMissingFeatureContextFields(context, feature); + void client .sendFeatureEvent({ action: "check-config", @@ -1278,11 +1274,23 @@ export class BucketClient { } private async _getFeaturesRemote( - key: string, + key: undefined, userId?: IdType, companyId?: IdType, additionalContext?: Context, - ): Promise { + ): Promise; + private async _getFeaturesRemote( + key: TKey, + userId?: IdType, + companyId?: IdType, + additionalContext?: Context, + ): Promise; + private async _getFeaturesRemote( + key?: string, + userId?: IdType, + companyId?: IdType, + additionalContext?: Context, + ): Promise { const context = additionalContext || {}; if (userId) { context.user = { id: userId }; @@ -1306,27 +1314,31 @@ export class BucketClient { params.append("key", key); } - const res = await this.get( - `features/evaluated?${params}`, - ); - - if (res) { - this.warnMissingFeatureContextFields( - context, - Object.values(res.features), + try { + const res = await this.get( + `features/evaluated?${params}`, ); - return Object.fromEntries( - Object.entries(res.features).map(([featureKey, feature]) => { - return [ - featureKey as keyof TypedFeatures, + if (key) { + return this._wrapRawFeature(contextWithTracking, res.features[key]!); + } else { + return Object.fromEntries( + Object.entries(res.features).map(([featureKey, feature]) => [ + featureKey, this._wrapRawFeature(contextWithTracking, feature), - ]; - }) || [], - ); - } else { - this.logger.error("failed to fetch evaluated features"); - return {}; + ]), + ); + } + } catch (err) { + this.logger.error(`failed to fetch evaluated features: ${err}`, err); + if (key) { + return this._wrapRawFeature(contextWithTracking, { + key, + isEnabled: false, + }); + } else { + return {}; + } } } } @@ -1404,7 +1416,7 @@ export class BoundBucketClient { * * @returns Features for the given user/company and whether each one is enabled or not */ - public getFeature( + public getFeature( key: TKey, ): TypedFeatures[TKey] { return this._client.getFeature(this._options, key); diff --git a/packages/node-sdk/src/rate-limiter.ts b/packages/node-sdk/src/rate-limiter.ts index 5617b74e..0fb0a746 100644 --- a/packages/node-sdk/src/rate-limiter.ts +++ b/packages/node-sdk/src/rate-limiter.ts @@ -27,7 +27,10 @@ export function newRateLimiter(windowSizeMs: number) { for (const key of keys) { const lastAllowedTimestamp = lastAllowedTimestampsByKey[key]; - if (lastAllowedTimestamp < expireBeforeTimestamp) { + if ( + lastAllowedTimestamp && + lastAllowedTimestamp < expireBeforeTimestamp + ) { delete lastAllowedTimestampsByKey[key]; } } diff --git a/packages/node-sdk/src/types.ts b/packages/node-sdk/src/types.ts index 25215670..14a1a2b2 100644 --- a/packages/node-sdk/src/types.ts +++ b/packages/node-sdk/src/types.ts @@ -385,7 +385,7 @@ export type EvaluatedFeaturesAPIResponse = { /** * True if request successful. */ - success: boolean; + success: true; /** * True if additional context for user or company was found and used for evaluation on the remote server. @@ -395,7 +395,7 @@ export type EvaluatedFeaturesAPIResponse = { /** * The feature definitions. */ - features: RawFeature[]; + features: Record; }; /** diff --git a/packages/node-sdk/test/client.test.ts b/packages/node-sdk/test/client.test.ts index 22732afa..f63e08a7 100644 --- a/packages/node-sdk/test/client.test.ts +++ b/packages/node-sdk/test/client.test.ts @@ -1073,6 +1073,7 @@ describe("BucketClient", () => { active: true, }, type: "company", + userId: undefined, }, { attributes: { @@ -1110,16 +1111,6 @@ describe("BucketClient", () => { evalRuleResults: [true], evalMissingFields: [], }, - { - type: "feature-flag-event", - action: "evaluate", - key: "feature2", - targetingVersion: 2, - evalContext: flattenJSON(context), - evalResult: false, - evalRuleResults: [false], - evalMissingFields: ["attributeKey"], - }, { type: "event", event: "feature1", @@ -1227,6 +1218,45 @@ describe("BucketClient", () => { ]); }); + it("`isEnabled` warns about missing context fields", async () => { + const context = { + company, + user, + other: otherContext, + }; + + // test that the feature is returned + await client.initialize(); + const feature = client.getFeature(context, "feature2"); + + // trigger the warning + expect(feature.isEnabled).toBe(false); + + expect(logger.warn).toHaveBeenCalledWith( + "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", + { + feature2: ["attributeKey"], + }, + ); + }); + + it("`isEnabled` should not warn about missing context fields if not needed", async () => { + const context = { + company, + user, + other: otherContext, + }; + + // test that the feature is returned + await client.initialize(); + const feature = client.getFeature(context, "feature1"); + + // should not trigger the warning + expect(feature.isEnabled).toBe(true); + + expect(logger.warn).not.toHaveBeenCalled(); + }); + it("`config` sends `check` event", async () => { const context = { company, @@ -1287,21 +1317,6 @@ describe("BucketClient", () => { .filter((e) => e.type === "feature-flag-event"); expect(checkEvents).toStrictEqual([ - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate", - key: "feature1", - }), - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate-config", - key: "feature1", - }), - expect.objectContaining({ - type: "feature-flag-event", - action: "evaluate", - key: "feature2", - }), { type: "feature-flag-event", action: "check", @@ -1428,24 +1443,6 @@ describe("BucketClient", () => { expect(httpClient.post).toHaveBeenCalledTimes(1); }); - it("should warn about missing context fields", async () => { - httpClient.post.mockClear(); - - await client.initialize(); - client.getFeatures({ - company, - user, - other: otherContext, - }); - - expect(logger.warn).toHaveBeenCalledWith( - "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", - { - feature2: ["attributeKey"], - }, - ); - }); - it("should properly define the rate limiter key", async () => { const isAllowedSpy = vi.spyOn(client["rateLimiter"], "isAllowed"); @@ -2073,18 +2070,6 @@ describe("BucketClient", () => { API_TIMEOUT_MS, ); }); - - it("should warn if missing context fields", async () => { - await client.getFeaturesRemote(); - expect(logger.warn).toHaveBeenCalledWith( - "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", - { - feature1: ["something", "funny"], - "feature1.config": ["funny"], - feature2: ["another"], - }, - ); - }); }); describe("getFeatureRemote", () => { @@ -2155,17 +2140,6 @@ describe("BucketClient", () => { API_TIMEOUT_MS, ); }); - - it("should warn if missing context fields", async () => { - await client.getFeatureRemote("feature1"); - expect(logger.warn).toHaveBeenCalledWith( - "feature/remote config targeting rules might not be correctly evaluated due to missing context fields.", - { - feature1: ["one", "two"], - "feature1.config": ["two"], - }, - ); - }); }); describe("offline mode", () => { diff --git a/packages/node-sdk/tsconfig.json b/packages/node-sdk/tsconfig.json index 8b241382..ee9ac47c 100644 --- a/packages/node-sdk/tsconfig.json +++ b/packages/node-sdk/tsconfig.json @@ -4,7 +4,8 @@ "lib": [], "outDir": "./dist/", "declarationDir": "./dist/types", - "declarationMap": true + "declarationMap": true, + "noUncheckedIndexedAccess": true }, "include": ["src"], "typeRoots": ["./node_modules/@types", "./types"] diff --git a/packages/openfeature-node-provider/package.json b/packages/openfeature-node-provider/package.json index e73c0fc5..b56ae5fc 100644 --- a/packages/openfeature-node-provider/package.json +++ b/packages/openfeature-node-provider/package.json @@ -1,6 +1,6 @@ { "name": "@bucketco/openfeature-node-provider", - "version": "0.4.2", + "version": "0.5.0", "license": "MIT", "repository": { "type": "git", @@ -50,7 +50,7 @@ "vitest": "~1.6.0" }, "dependencies": { - "@bucketco/node-sdk": "1.9.3" + "@bucketco/node-sdk": "1.10.0" }, "peerDependencies": { "@openfeature/server-sdk": ">=1.16.1" diff --git a/packages/openfeature-node-provider/src/index.ts b/packages/openfeature-node-provider/src/index.ts index ad771095..7e42ac0d 100644 --- a/packages/openfeature-node-provider/src/index.ts +++ b/packages/openfeature-node-provider/src/index.ts @@ -100,7 +100,7 @@ export class BucketNodeProvider implements Provider { }); } - const features = this._client.getFeatures(context); + const features = this._client.getFeatureDefinitions(); if (flagKey in features) { return resolveFn(this._client.getFeature(context, flagKey)); } From 9d1ea7be71fd668c8e41ed5f0164e282d4eea128 Mon Sep 17 00:00:00 2001 From: Erik Hughes Date: Thu, 14 Aug 2025 14:55:52 +0200 Subject: [PATCH 2/6] fix: update deps and pr suggestion --- packages/node-sdk/src/client.ts | 3 ++- yarn.lock | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index 12a334f0..aed9e05b 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -1320,7 +1320,8 @@ export class BucketClient { ); if (key) { - return this._wrapRawFeature(contextWithTracking, res.features[key]!); + if (!res.features[key]) throw new Error(`Feature ${key} not found`); + return this._wrapRawFeature(contextWithTracking, res.features[key]); } else { return Object.fromEntries( Object.entries(res.features).map(([featureKey, feature]) => [ diff --git a/yarn.lock b/yarn.lock index e8437c9a..fac22225 100644 --- a/yarn.lock +++ b/yarn.lock @@ -774,7 +774,7 @@ __metadata: languageName: unknown linkType: soft -"@bucketco/node-sdk@npm:1.9.3, @bucketco/node-sdk@workspace:packages/node-sdk": +"@bucketco/node-sdk@npm:1.10.0, @bucketco/node-sdk@workspace:packages/node-sdk": version: 0.0.0-use.local resolution: "@bucketco/node-sdk@workspace:packages/node-sdk" dependencies: @@ -824,7 +824,7 @@ __metadata: dependencies: "@babel/core": "npm:~7.24.7" "@bucketco/eslint-config": "npm:~0.0.2" - "@bucketco/node-sdk": "npm:1.9.3" + "@bucketco/node-sdk": "npm:1.10.0" "@bucketco/tsconfig": "npm:~0.0.2" "@openfeature/core": "npm:^1.5.0" "@openfeature/server-sdk": "npm:>=1.16.1" From 78099b3b6ed52858373c274ac6b19d2abc64101f Mon Sep 17 00:00:00 2001 From: Erik Hughes Date: Thu, 14 Aug 2025 15:50:14 +0200 Subject: [PATCH 3/6] revert: get throwing error --- packages/node-sdk/src/client.ts | 36 ++++++++++--------- .../openfeature-node-provider/src/index.ts | 4 +-- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index aed9e05b..150239c7 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -794,7 +794,7 @@ export class BucketClient { `get request to "${path}" failed with error after ${retries} retries`, error, ); - throw error; + return undefined; } } @@ -1314,13 +1314,19 @@ export class BucketClient { params.append("key", key); } - try { - const res = await this.get( - `features/evaluated?${params}`, - ); + const res = await this.get( + `features/evaluated?${params}`, + ); + if (res) { if (key) { - if (!res.features[key]) throw new Error(`Feature ${key} not found`); + if (!res.features[key]) { + this.logger.error(`feature ${key} not found`); + return this._wrapRawFeature(contextWithTracking, { + key, + isEnabled: false, + }); + } return this._wrapRawFeature(contextWithTracking, res.features[key]); } else { return Object.fromEntries( @@ -1330,16 +1336,14 @@ export class BucketClient { ]), ); } - } catch (err) { - this.logger.error(`failed to fetch evaluated features: ${err}`, err); - if (key) { - return this._wrapRawFeature(contextWithTracking, { - key, - isEnabled: false, - }); - } else { - return {}; - } + } + if (key) { + return this._wrapRawFeature(contextWithTracking, { + key, + isEnabled: false, + }); + } else { + return {}; } } } diff --git a/packages/openfeature-node-provider/src/index.ts b/packages/openfeature-node-provider/src/index.ts index 7e42ac0d..bf204882 100644 --- a/packages/openfeature-node-provider/src/index.ts +++ b/packages/openfeature-node-provider/src/index.ts @@ -100,8 +100,8 @@ export class BucketNodeProvider implements Provider { }); } - const features = this._client.getFeatureDefinitions(); - if (flagKey in features) { + const featureDefs = this._client.getFeatureDefinitions(); + if (featureDefs.some(({ key }) => key === flagKey)) { return resolveFn(this._client.getFeature(context, flagKey)); } From 1d9bb74a065027e52b1a9c3634fb68b06273853c Mon Sep 17 00:00:00 2001 From: Erik Hughes Date: Thu, 14 Aug 2025 15:54:40 +0200 Subject: [PATCH 4/6] fix: simplify return logic of _getFeaturesRemote --- packages/node-sdk/src/client.ts | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index 150239c7..ce5d6977 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -1318,32 +1318,25 @@ export class BucketClient { `features/evaluated?${params}`, ); - if (res) { - if (key) { - if (!res.features[key]) { - this.logger.error(`feature ${key} not found`); - return this._wrapRawFeature(contextWithTracking, { - key, - isEnabled: false, - }); - } - return this._wrapRawFeature(contextWithTracking, res.features[key]); - } else { - return Object.fromEntries( - Object.entries(res.features).map(([featureKey, feature]) => [ - featureKey, - this._wrapRawFeature(contextWithTracking, feature), - ]), - ); - } - } if (key) { + const feature = res?.features[key]; + if (!feature) { + this.logger.error(`feature ${key} not found`); + } return this._wrapRawFeature(contextWithTracking, { key, isEnabled: false, + ...feature, }); } else { - return {}; + return res?.features + ? Object.fromEntries( + Object.entries(res?.features).map(([featureKey, feature]) => [ + featureKey, + this._wrapRawFeature(contextWithTracking, feature), + ]), + ) + : {}; } } } From 155d030789cef266421a8d8988214c45d6948bc2 Mon Sep 17 00:00:00 2001 From: Erik Hughes Date: Thu, 14 Aug 2025 16:03:37 +0200 Subject: [PATCH 5/6] fix: drop requirement to send isEnabled: false --- packages/node-sdk/src/client.ts | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/node-sdk/src/client.ts b/packages/node-sdk/src/client.ts index ce5d6977..5813972f 100644 --- a/packages/node-sdk/src/client.ts +++ b/packages/node-sdk/src/client.ts @@ -60,6 +60,8 @@ import { const bucketConfigDefaultFile = "bucketConfig.json"; +type PartialBy = Omit & Partial>; + type BulkEvent = | { type: "company"; @@ -1045,7 +1047,7 @@ export class BucketClient { if (key) { return this._wrapRawFeature( { ...options, enableChecks: true }, - { key, isEnabled: false, ...fallbackFeatures[key] }, + { key, ...fallbackFeatures[key] }, ); } return Object.fromEntries( @@ -1177,11 +1179,7 @@ export class BucketClient { if (key) { return this._wrapRawFeature( { ...options, enableChecks: true }, - { - key, - isEnabled: false, - ...evaluatedFeatures[key], - }, + { key, ...evaluatedFeatures[key] }, ); } @@ -1199,7 +1197,7 @@ export class BucketClient { enableChecks = false, ...context }: { enableTracking: boolean; enableChecks?: boolean } & Context, - { config, ...feature }: RawFeature, + { config, ...feature }: PartialBy, ): TypedFeatures[TKey] { // eslint-disable-next-line @typescript-eslint/no-this-alias const client = this; @@ -1218,7 +1216,7 @@ export class BucketClient { action: "check", key: feature.key, targetingVersion: feature.targetingVersion, - evalResult: feature.isEnabled, + evalResult: feature.isEnabled ?? false, evalContext: context, evalRuleResults: feature.ruleEvaluationResults, evalMissingFields: feature.missingContextFields, @@ -1230,7 +1228,7 @@ export class BucketClient { ); }); } - return feature.isEnabled; + return feature.isEnabled ?? false; }, get config() { if (enableTracking && enableChecks) { @@ -1323,11 +1321,7 @@ export class BucketClient { if (!feature) { this.logger.error(`feature ${key} not found`); } - return this._wrapRawFeature(contextWithTracking, { - key, - isEnabled: false, - ...feature, - }); + return this._wrapRawFeature(contextWithTracking, { key, ...feature }); } else { return res?.features ? Object.fromEntries( From 0db79b242daba5ac9b6846ed4b45fb3b7652c1ef Mon Sep 17 00:00:00 2001 From: Erik Hughes Date: Thu, 14 Aug 2025 16:13:00 +0200 Subject: [PATCH 6/6] test: add getFeatureDefinitions mock --- .../src/index.test.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/openfeature-node-provider/src/index.test.ts b/packages/openfeature-node-provider/src/index.test.ts index 9d2b63b9..00c5aa1b 100644 --- a/packages/openfeature-node-provider/src/index.test.ts +++ b/packages/openfeature-node-provider/src/index.test.ts @@ -16,8 +16,8 @@ vi.mock("@bucketco/node-sdk", () => { }); const bucketClientMock = { - getFeatures: vi.fn(), getFeature: vi.fn(), + getFeatureDefinitions: vi.fn().mockReturnValue([]), initialize: vi.fn().mockResolvedValue({}), flush: vi.fn(), track: vi.fn(), @@ -54,6 +54,7 @@ describe("BucketNodeProvider", () => { enabled: boolean, configKey?: string | null, configPayload?: any, + flagKey = testFlagKey, ) { const config = { key: configKey, @@ -65,15 +66,15 @@ describe("BucketNodeProvider", () => { config, }); - bucketClientMock.getFeatures = vi.fn().mockReturnValue({ - [testFlagKey]: { - isEnabled: enabled, - config: { - key: "key", - payload: configPayload, - }, + // Mock getFeatureDefinitions to return feature definitions that include the specified flag + bucketClientMock.getFeatureDefinitions = vi.fn().mockReturnValue([ + { + key: flagKey, + description: "Test feature flag", + flag: {}, + config: {}, }, - }); + ]); } beforeEach(async () => { @@ -181,8 +182,11 @@ describe("BucketNodeProvider", () => { expect(mockTranslatorFn).toHaveBeenCalledTimes(1); expect(mockTranslatorFn).toHaveBeenCalledWith(context); - expect(bucketClientMock.getFeatures).toHaveBeenCalledTimes(1); - expect(bucketClientMock.getFeatures).toHaveBeenCalledWith(bucketContext); + expect(bucketClientMock.getFeatureDefinitions).toHaveBeenCalledTimes(1); + expect(bucketClientMock.getFeature).toHaveBeenCalledWith( + bucketContext, + testFlagKey, + ); }); }); @@ -239,7 +243,7 @@ describe("BucketNodeProvider", () => { value: true, }); - expect(bucketClientMock.getFeatures).toHaveBeenCalled(); + expect(bucketClientMock.getFeatureDefinitions).toHaveBeenCalled(); expect(bucketClientMock.getFeature).toHaveBeenCalledWith( bucketContext, testFlagKey,