From 384ee738e05fc8dd05174c2e3859f959f2790816 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:50:52 +0800 Subject: [PATCH 1/2] handle corner case response status is string (#335) --- src/appConfigurationImpl.ts | 8 +-- test/refresh.test.ts | 105 +++++++++++++++++++++++++++++++++++- test/utils/testHelper.ts | 35 ++++++++---- 3 files changed, 132 insertions(+), 16 deletions(-) diff --git a/src/appConfigurationImpl.ts b/src/appConfigurationImpl.ts index 42cebc5..e247b31 100644 --- a/src/appConfigurationImpl.ts +++ b/src/appConfigurationImpl.ts @@ -659,7 +659,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const watcher: SettingWatcher = this.#sentinels.get(watchedSetting)!; // watcher should always exist for sentinels const isDeleted = response === undefined && watcher.etag !== undefined; // previously existed, now deleted - const isChanged = response && response.statusCode === 200 && watcher.etag !== response.etag; // etag changed + const isChanged = response && Number(response.statusCode) === 200 && watcher.etag !== response.etag; // etag changed if (isDeleted || isChanged) { changedSentinel = watchedSetting; changedSentinelWatcher = { etag: isChanged ? response.etag : undefined }; @@ -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; } } @@ -779,7 +779,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { try { response = await this.#executeWithFailoverPolicy(funcToExecute); } catch (error) { - if (isRestError(error) && error.statusCode === 404) { + if (isRestError(error) && Number(error.statusCode) === 404) { response = undefined; } else { throw error; @@ -822,7 +822,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { try { response = await this.#executeWithFailoverPolicy(funcToExecute); } catch (error) { - if (isRestError(error) && error.statusCode === 404) { + if (isRestError(error) && Number(error.statusCode) === 404) { response = undefined; } else { throw error; diff --git a/test/refresh.test.ts b/test/refresh.test.ts index fa1efba..bf7619d 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[] = []; @@ -513,6 +513,54 @@ describe("dynamic refresh", function () { // Verify changes are reflected expect(settings.get("TestTagKey")).eq("newValue"); }); + + it("should refresh key values when page response status is string", async () => { + restoreMocks(); // restore the mock set up in beforeEach + const initialKVs = [ + { value: "red", key: "app.settings.fontColor" }, + { value: "40", key: "app.settings.fontSize" } + ].map(createMockedKeyValue); + mockAppConfigurationClientListConfigurationSettingsWithStringStatus([initialKVs], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting(initialKVs, getKvCallback); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000 + } + }); + expect(listKvRequestCount).eq(1); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + + let refreshSuccessfulCount = 0; + settings.onRefresh(() => { + refreshSuccessfulCount++; + }); + + // no change yet, the page response status is "304" + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(listKvRequestCount).eq(2); // one more conditional request to detect change + expect(refreshSuccessfulCount).eq(0); // no change in key values, because page etags are the same. + + // change key value + restoreMocks(); + const changedKVs = [ + { value: "blue", key: "app.settings.fontColor" }, + { value: "40", key: "app.settings.fontSize" } + ].map(createMockedKeyValue); + mockAppConfigurationClientListConfigurationSettingsWithStringStatus([changedKVs], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting(changedKVs, getKvCallback); + + // the page response status is "200" when changed + 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 key values + expect(refreshSuccessfulCount).eq(1); // change in key values, because page etags are different. + expect(settings.get("app.settings.fontColor")).eq("blue"); + }); }); describe("dynamic refresh feature flags", function () { @@ -672,5 +720,60 @@ describe("dynamic refresh feature flags", function () { expect(updatedFeatureManagement.feature_flags[0].id).eq("DevFeature"); expect(updatedFeatureManagement.feature_flags[0].enabled).eq(false); }); + + it("should refresh feature flags when page response status is string", async () => { + // mock multiple pages of feature flags + const page1 = [ + createMockedFeatureFlag("Alpha_1", { enabled: true }), + createMockedFeatureFlag("Alpha_2", { enabled: true }), + ]; + const page2 = [ + createMockedFeatureFlag("Beta_1", { enabled: true }), + createMockedFeatureFlag("Beta_2", { enabled: true }), + ]; + mockAppConfigurationClientListConfigurationSettingsWithStringStatus([page1, page2], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2], getKvCallback); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "*" + }], + refresh: { + enabled: true, + refreshIntervalInMs: 2000 // 2 seconds for quick test. + } + } + }); + expect(listKvRequestCount).eq(2); + expect(getKvRequestCount).eq(0); + + let refreshSuccessfulCount = 0; + settings.onRefresh(() => { + refreshSuccessfulCount++; + }); + + // no change yet, the page response status is "304" + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(listKvRequestCount).eq(3); // one conditional request to detect change + expect(getKvRequestCount).eq(0); + expect(refreshSuccessfulCount).eq(0); // no change in feature flags, because page etags are the same. + + // change feature flag Beta_1 to false + page2[0] = createMockedFeatureFlag("Beta_1", { enabled: false }); + restoreMocks(); + mockAppConfigurationClientListConfigurationSettingsWithStringStatus([page1, page2], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2], getKvCallback); + + // the page response status is "200" when changed + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(listKvRequestCount).eq(5); // 3 + 2 more requests: one conditional request to detect change and one request to reload all feature flags + expect(getKvRequestCount).eq(0); + expect(refreshSuccessfulCount).eq(1); // change in feature flags, because page etags are different. + }); }); /* eslint-enable @typescript-eslint/no-unused-expressions */ diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 778979b..6487c0b 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -6,7 +6,7 @@ import { AppConfigurationClient, ConfigurationSetting, featureFlagContentType, s import { ClientSecretCredential } from "@azure/identity"; import { KeyVaultSecret, SecretClient } from "@azure/keyvault-secrets"; import * as uuid from "uuid"; -import { RestError } from "@azure/core-rest-pipeline"; +import { RestError, PipelineRequest, PipelineResponse, SendRequest } from "@azure/core-rest-pipeline"; import { ConfigurationClientManager } from "../../src/configurationClientManager.js"; import { ConfigurationClientWrapper } from "../../src/configurationClientWrapper.js"; @@ -51,7 +51,7 @@ function _filterKVs(unfilteredKvs: ConfigurationSetting[], listOptions: any) { } let tagsMatched = true; if (tagsFilter.length > 0) { - tagsMatched = tagsFilter.every(tag => { + tagsMatched = tagsFilter.every((tag: string) => { const [tagName, tagValue] = tag.split("="); if (tagValue === "\0") { return kv.tags && kv.tags[tagName] === null; @@ -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); @@ -74,7 +74,7 @@ function getMockedIterator(pages: ConfigurationSetting[][], kvs: ConfigurationSe return Promise.resolve({ done: !value, value }); }, byPage(): AsyncIterableIterator { - let remainingPages; + let remainingPages: ConfigurationSetting[][]; const pageEtags = listOptions?.pageEtags ? [...listOptions.pageEtags] : undefined; // a copy of the original list return { [Symbol.asyncIterator](): AsyncIterableIterator { @@ -95,7 +95,7 @@ function getMockedIterator(pages: ConfigurationSetting[][], kvs: ConfigurationSe value: { items, etag, - _response: { status: statusCode } + _response: { status: useStringStatus ? `${statusCode}` : statusCode } } }; } @@ -114,7 +114,7 @@ function getMockedIterator(pages: ConfigurationSetting[][], kvs: ConfigurationSe * * @param pages List of pages, each page is a list of ConfigurationSetting */ -function mockAppConfigurationClientListConfigurationSettings(pages: ConfigurationSetting[][], customCallback?: (listOptions) => any) { +function mockAppConfigurationClientListConfigurationSettings(pages: ConfigurationSetting[][], customCallback?: (listOptions: any) => any) { sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettings").callsFake((listOptions) => { if (customCallback) { @@ -126,6 +126,18 @@ function mockAppConfigurationClientListConfigurationSettings(pages: Configuratio }); } +function mockAppConfigurationClientListConfigurationSettingsWithStringStatus(pages: ConfigurationSetting[][], customCallback?: (listOptions: any) => 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; @@ -163,7 +175,7 @@ function mockConfigurationManagerGetClients(fakeClientWrappers: ConfigurationCli }); } -function mockAppConfigurationClientGetConfigurationSetting(kvList: any[], customCallback?: (options) => any) { +function mockAppConfigurationClientGetConfigurationSetting(kvList: any[], customCallback?: (options: any) => any) { sinon.stub(AppConfigurationClient.prototype, "getConfigurationSetting").callsFake((settingId, options) => { if (customCallback) { customCallback(options); @@ -182,7 +194,7 @@ function mockAppConfigurationClientGetConfigurationSetting(kvList: any[], custom }); } -function mockAppConfigurationClientGetSnapshot(snapshotResponses: Map, customCallback?: (options) => any) { +function mockAppConfigurationClientGetSnapshot(snapshotResponses: Map, customCallback?: (options: any) => any) { sinon.stub(AppConfigurationClient.prototype, "getSnapshot").callsFake((name, options) => { if (customCallback) { customCallback(options); @@ -196,7 +208,7 @@ function mockAppConfigurationClientGetSnapshot(snapshotResponses: Map, customCallback?: (options) => any) { +function mockAppConfigurationClientListConfigurationSettingsForSnapshot(snapshotResponses: Map, customCallback?: (options: any) => any) { sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettingsForSnapshot").callsFake((name, listOptions) => { if (customCallback) { customCallback(listOptions); @@ -218,7 +230,7 @@ function mockSecretClientGetSecret(uriValueList: [string, string][]) { dict.set(uri, value); } - sinon.stub(SecretClient.prototype, "getSecret").callsFake(async function (secretName, options) { + sinon.stub(SecretClient.prototype, "getSecret").callsFake(async function (this: SecretClient, secretName, options) { const url = new URL(this.vaultUrl); url.pathname = `/secrets/${secretName}`; if (options?.version) { @@ -314,7 +326,7 @@ class HttpRequestHeadersPolicy { this.headers = {}; this.name = "HttpRequestHeadersPolicy"; } - sendRequest(req, next) { + sendRequest(req: PipelineRequest, next: SendRequest): Promise { this.headers = req.headers; return next(req).then(resp => resp); } @@ -323,6 +335,7 @@ class HttpRequestHeadersPolicy { export { sinon, mockAppConfigurationClientListConfigurationSettings, + mockAppConfigurationClientListConfigurationSettingsWithStringStatus, mockAppConfigurationClientGetConfigurationSetting, mockAppConfigurationClientGetSnapshot, mockAppConfigurationClientListConfigurationSettingsForSnapshot, From ee23ddb8b2dd87d07ca3b208039272fb8a55ce47 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Fri, 26 Jun 2026 14:02:38 +0800 Subject: [PATCH 2/2] version bump 2.5.1 (#337) --- package-lock.json | 4 ++-- package.json | 2 +- src/version.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 00e6892..30cbd47 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@azure/app-configuration-provider", - "version": "2.5.0", + "version": "2.5.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@azure/app-configuration-provider", - "version": "2.5.0", + "version": "2.5.1", "license": "MIT", "dependencies": { "@azure/app-configuration": "^1.11.0", diff --git a/package.json b/package.json index 14a8f91..cddad01 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@azure/app-configuration-provider", - "version": "2.5.0", + "version": "2.5.1", "description": "The JavaScript configuration provider for Azure App Configuration", "files": [ "dist/", diff --git a/src/version.ts b/src/version.ts index b611498..6428e51 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1,4 +1,4 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -export const VERSION = "2.5.0"; +export const VERSION = "2.5.1";