From 84a9aac42f3e06a2dd8ee3a2cc40cdd717c9a7a1 Mon Sep 17 00:00:00 2001 From: Ivan Peters Date: Fri, 26 Jun 2026 15:56:45 +1200 Subject: [PATCH] Fix feature flag refresh for string status codes The underlying HTTP response status can sometimes be a string (e.g., "200") instead of a number (200). Explicitly cast the status to a number to ensure conditional refresh logic works correctly in all cases. --- src/appConfigurationImpl.ts | 2 +- test/refresh.test.ts | 44 ++++++++++++++++++++++++++++++++++++- test/utils/testHelper.ts | 17 ++++++++++++-- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/appConfigurationImpl.ts b/src/appConfigurationImpl.ts index 42cebc5..28e9ead 100644 --- a/src/appConfigurationImpl.ts +++ b/src/appConfigurationImpl.ts @@ -750,7 +750,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { for await (const page of pageIterator) { // when conditional request is sent, the response will be 304 if not changed - if (page._response.status === 200) { // created or changed + if (Number(page._response.status) === 200) { // created or changed return true; } } diff --git a/test/refresh.test.ts b/test/refresh.test.ts index fa1efba..6482293 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -7,7 +7,7 @@ import chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "../src/index.js"; -import { mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue, sleepInMs, createMockedFeatureFlag } from "./utils/testHelper.js"; +import { mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientListConfigurationSettingsWithStringStatus, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue, sleepInMs, createMockedFeatureFlag } from "./utils/testHelper.js"; import * as uuid from "uuid"; let mockedKVs: any[] = []; @@ -574,6 +574,48 @@ describe("dynamic refresh feature flags", function () { }); + it("should refresh feature flags when page response status is string", async () => { + mockedKVs = [ + createMockedFeatureFlag("Beta", { enabled: true }) + ]; + mockAppConfigurationClientListConfigurationSettingsWithStringStatus([mockedKVs], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting(mockedKVs, getKvCallback); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "Beta" + }], + refresh: { + enabled: true, + refreshIntervalInMs: 2000 // 2 seconds for quick test. + } + } + }); + expect(listKvRequestCount).eq(2); // one listKv request for kvs and one listKv request for feature flags + expect(getKvRequestCount).eq(0); + expect(settings).not.undefined; + expect(settings.get("feature_management").feature_flags[0].enabled).eq(true); + + // change feature flag Beta to false + updateSetting(".appconfig.featureflag/Beta", JSON.stringify({ + "id": "Beta", + "description": "", + "enabled": false, + "conditions": { + "client_filters": [] + } + })); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(listKvRequestCount).eq(4); // 2 + 2 more requests: one conditional request to detect change and one request to reload all feature flags + expect(getKvRequestCount).eq(0); + expect(settings.get("feature_management").feature_flags[0].enabled).eq(false); + }); + it("should refresh feature flags based on page etags, only on change", async () => { // mock multiple pages of feature flags const page1 = [ diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 778979b..811e8be 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -63,7 +63,7 @@ function _filterKVs(unfilteredKvs: ConfigurationSetting[], listOptions: any) { }); } -function getMockedIterator(pages: ConfigurationSetting[][], kvs: ConfigurationSetting[], listOptions: any) { +function getMockedIterator(pages: ConfigurationSetting[][], kvs: ConfigurationSetting[], listOptions: any, useStringStatus: boolean = false) { const mockIterator: AsyncIterableIterator & { byPage(): AsyncIterableIterator } = { [Symbol.asyncIterator](): AsyncIterableIterator { kvs = _filterKVs(pages.flat(), listOptions); @@ -95,7 +95,7 @@ function getMockedIterator(pages: ConfigurationSetting[][], kvs: ConfigurationSe value: { items, etag, - _response: { status: statusCode } + _response: { status: useStringStatus ? `${statusCode}` : statusCode } } }; } @@ -126,6 +126,18 @@ function mockAppConfigurationClientListConfigurationSettings(pages: Configuratio }); } +function mockAppConfigurationClientListConfigurationSettingsWithStringStatus(pages: ConfigurationSetting[][], customCallback?: (listOptions) => any) { + + sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettings").callsFake((listOptions) => { + if (customCallback) { + customCallback(listOptions); + } + + const kvs = _filterKVs(pages.flat(), listOptions); + return getMockedIterator(pages, kvs, listOptions, true); + }); +} + function mockAppConfigurationClientLoadBalanceMode(pages: ConfigurationSetting[][], clientWrapper: ConfigurationClientWrapper, countObject: { count: number }) { sinon.stub(clientWrapper.client, "listConfigurationSettings").callsFake((listOptions) => { countObject.count += 1; @@ -323,6 +335,7 @@ class HttpRequestHeadersPolicy { export { sinon, mockAppConfigurationClientListConfigurationSettings, + mockAppConfigurationClientListConfigurationSettingsWithStringStatus, mockAppConfigurationClientGetConfigurationSetting, mockAppConfigurationClientGetSnapshot, mockAppConfigurationClientListConfigurationSettingsForSnapshot,