From 2720628cd571d16c8a4ffaa197a5b74edc6f0742 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 17 Feb 2025 23:05:14 +0800 Subject: [PATCH 01/47] support startup retry and timeout --- src/AzureAppConfigurationImpl.ts | 52 +++++++++++++++++++++++++---- src/AzureAppConfigurationOptions.ts | 9 +++-- src/ConfigurationClientManager.ts | 22 +++++++----- src/ConfigurationClientWrapper.ts | 26 ++------------- src/StartupOptions.ts | 12 +++++++ src/common/utils.ts | 10 +++--- src/failover.ts | 39 ++++++++++++++++++++++ src/load.ts | 2 +- 8 files changed, 124 insertions(+), 48 deletions(-) create mode 100644 src/StartupOptions.ts create mode 100644 src/failover.ts diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 90e283f0..a1121bb0 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -40,6 +40,7 @@ import { RequestTracingOptions, getConfigurationSettingWithTrace, listConfigurat import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOptions.js"; import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; +import { getFixedBackoffDuration, calculateDynamicBackoffDuration } from "./failover.js"; type PagedSettingSelector = SettingSelector & { /** @@ -48,6 +49,9 @@ type PagedSettingSelector = SettingSelector & { pageEtags?: string[]; }; +const DEFAULT_STARTUP_TIMEOUT = 100 * 1000; // 100 seconds in milliseconds +const MAX_STARTUP_TIMEOUT = 30 * 60 * 1000; // 15 minutes in milliseconds + export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** * Hosting key-value pairs in the configuration store. @@ -229,13 +233,18 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Loads the configuration store for the first time. */ async load() { - await this.#inspectFmPackage(); - await this.#loadSelectedAndWatchedKeyValues(); - if (this.#featureFlagEnabled) { - await this.#loadFeatureFlags(); + const startupTimeout = this.#options?.startupOptions?.timeoutInMs ?? DEFAULT_STARTUP_TIMEOUT; + let timer; + try { + await Promise.race([ + new Promise((_, reject) => timer = setTimeout(() => reject(new Error("Load operation timed out.")), startupTimeout)), + this.#initialize() + ]); + } catch (error) { + throw new Error(`Failed to load: ${error.message}`); + } finally { + clearTimeout(timer); } - // Mark all settings have loaded at startup. - this.#isInitialLoadCompleted = true; } /** @@ -320,6 +329,37 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return new Disposable(remove); } + /** + * Initializes the configuration provider. + */ + async #initialize() { + if (this.#isInitialLoadCompleted) { + await this.#inspectFmPackage(); + const startTimestamp = Date.now(); + while (startTimestamp + MAX_STARTUP_TIMEOUT > Date.now()) { + try { + await this.#loadSelectedAndWatchedKeyValues(); + if (this.#featureFlagEnabled) { + await this.#loadFeatureFlags(); + } + // Mark all settings have loaded at startup. + this.#isInitialLoadCompleted = true; + break; + } catch (error) { + const timeElapsed = Date.now() - startTimestamp; + let postAttempts = 0; + let backoffDuration = getFixedBackoffDuration(timeElapsed); + if (backoffDuration === undefined) { + postAttempts += 1; + backoffDuration = calculateDynamicBackoffDuration(postAttempts); + } + await new Promise(resolve => setTimeout(resolve, backoffDuration)); + console.warn("Failed to load configuration settings at startup. Retrying..."); + } + } + } + } + /** * Inspects the feature management package version. */ diff --git a/src/AzureAppConfigurationOptions.ts b/src/AzureAppConfigurationOptions.ts index 56b47b50..5dc267af 100644 --- a/src/AzureAppConfigurationOptions.ts +++ b/src/AzureAppConfigurationOptions.ts @@ -6,9 +6,7 @@ import { KeyVaultOptions } from "./keyvault/KeyVaultOptions.js"; import { RefreshOptions } from "./RefreshOptions.js"; import { SettingSelector } from "./types.js"; import { FeatureFlagOptions } from "./featureManagement/FeatureFlagOptions.js"; - -export const MaxRetries = 2; -export const MaxRetryDelayInMs = 60000; +import { StartupOptions } from "./StartupOptions.js"; export interface AzureAppConfigurationOptions { /** @@ -48,6 +46,11 @@ export interface AzureAppConfigurationOptions { */ featureFlagOptions?: FeatureFlagOptions; + /** + * Specifies options used to configure provider startup. + */ + startupOptions?: StartupOptions; + /** * Specifies whether to enable replica discovery or not. * diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index 7ad8e597..b1574c2f 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -4,11 +4,15 @@ import { AppConfigurationClient, AppConfigurationClientOptions } from "@azure/app-configuration"; import { ConfigurationClientWrapper } from "./ConfigurationClientWrapper.js"; import { TokenCredential } from "@azure/identity"; -import { AzureAppConfigurationOptions, MaxRetries, MaxRetryDelayInMs } from "./AzureAppConfigurationOptions.js"; +import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; import { isBrowser, isWebWorker } from "./requestTracing/utils.js"; import * as RequestTracing from "./requestTracing/constants.js"; import { shuffleList } from "./common/utils.js"; +// Configuration client retry options +const CLIENT_MAX_RETRIES = 2; +const CLIENT_MAX_RETRY_DELAY = 60_000; // 1 minute in milliseconds + const TCP_ORIGIN_KEY_NAME = "_origin._tcp"; const ALT_KEY_NAME = "_alt"; const TCP_KEY_NAME = "_tcp"; @@ -17,8 +21,8 @@ const ID_KEY_NAME = "Id"; const SECRET_KEY_NAME = "Secret"; const TRUSTED_DOMAIN_LABELS = [".azconfig.", ".appconfig."]; const FALLBACK_CLIENT_REFRESH_EXPIRE_INTERVAL = 60 * 60 * 1000; // 1 hour in milliseconds -const MINIMAL_CLIENT_REFRESH_INTERVAL = 30 * 1000; // 30 seconds in milliseconds -const SRV_QUERY_TIMEOUT = 30 * 1000; // 30 seconds in milliseconds +const MINIMAL_CLIENT_REFRESH_INTERVAL = 30_000; // 30 seconds in milliseconds +const SRV_QUERY_TIMEOUT = 30_000; // 30 seconds in milliseconds export class ConfigurationClientManager { #isFailoverable: boolean; @@ -143,16 +147,16 @@ export class ConfigurationClientManager { async #discoverFallbackClients(host: string) { let result; - let timeout; + let timer; try { result = await Promise.race([ - new Promise((_, reject) => timeout = setTimeout(() => reject(new Error("SRV record query timed out.")), SRV_QUERY_TIMEOUT)), + new Promise((_, reject) => timer = setTimeout(() => reject(new Error("SRV record query timed out.")), SRV_QUERY_TIMEOUT)), this.#querySrvTargetHost(host) ]); } catch (error) { - throw new Error(`Failed to build fallback clients, ${error.message}`); + throw new Error(`Failed to build fallback clients: ${error.message}`); } finally { - clearTimeout(timeout); + clearTimeout(timer); } const srvTargetHosts = shuffleList(result) as string[]; @@ -269,8 +273,8 @@ function getClientOptions(options?: AzureAppConfigurationOptions): AppConfigurat // retry options const defaultRetryOptions = { - maxRetries: MaxRetries, - maxRetryDelayInMs: MaxRetryDelayInMs, + maxRetries: CLIENT_MAX_RETRIES, + maxRetryDelayInMs: CLIENT_MAX_RETRY_DELAY, }; const retryOptions = Object.assign({}, defaultRetryOptions, options?.clientOptions?.retryOptions); diff --git a/src/ConfigurationClientWrapper.ts b/src/ConfigurationClientWrapper.ts index 7dd6f418..08b95fb7 100644 --- a/src/ConfigurationClientWrapper.ts +++ b/src/ConfigurationClientWrapper.ts @@ -2,11 +2,7 @@ // Licensed under the MIT license. import { AppConfigurationClient } from "@azure/app-configuration"; - -const MaxBackoffDuration = 10 * 60 * 1000; // 10 minutes in milliseconds -const MinBackoffDuration = 30 * 1000; // 30 seconds in milliseconds -const MAX_SAFE_EXPONENTIAL = 30; // Used to avoid overflow. bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. -const JITTER_RATIO = 0.25; +import { calculateDynamicBackoffDuration } from "./failover.js"; export class ConfigurationClientWrapper { endpoint: string; @@ -25,25 +21,7 @@ export class ConfigurationClientWrapper { this.backoffEndTime = Date.now(); } else { this.#failedAttempts += 1; - this.backoffEndTime = Date.now() + calculateBackoffDuration(this.#failedAttempts); + this.backoffEndTime = Date.now() + calculateDynamicBackoffDuration(this.#failedAttempts); } } } - -export function calculateBackoffDuration(failedAttempts: number) { - if (failedAttempts <= 1) { - return MinBackoffDuration; - } - - // exponential: minBackoff * 2 ^ (failedAttempts - 1) - const exponential = Math.min(failedAttempts - 1, MAX_SAFE_EXPONENTIAL); - let calculatedBackoffDuration = MinBackoffDuration * (1 << exponential); - if (calculatedBackoffDuration > MaxBackoffDuration) { - calculatedBackoffDuration = MaxBackoffDuration; - } - - // jitter: random value between [-1, 1) * jitterRatio * calculatedBackoffMs - const jitter = JITTER_RATIO * (Math.random() * 2 - 1); - - return calculatedBackoffDuration * (1 + jitter); -} diff --git a/src/StartupOptions.ts b/src/StartupOptions.ts new file mode 100644 index 00000000..626086b8 --- /dev/null +++ b/src/StartupOptions.ts @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +export interface StartupOptions { + /** + * The amount of time allowed to load data from Azure App Configuration on startup. + * + * @remarks + * If not specified, the default value is 100 seconds. + */ + timeoutInMs?: number; +} diff --git a/src/common/utils.ts b/src/common/utils.ts index 8682484b..5150708c 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -24,9 +24,9 @@ export function jsonSorter(key, value) { } export function shuffleList(array: T[]): T[] { - for (let i = array.length - 1; i > 0; i--) { - const j = Math.floor(Math.random() * (i + 1)); - [array[i], array[j]] = [array[j], array[i]]; - } - return array; + for (let i = array.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)); + [array[i], array[j]] = [array[j], array[i]]; + } + return array; } diff --git a/src/failover.ts b/src/failover.ts new file mode 100644 index 00000000..29a2c0f6 --- /dev/null +++ b/src/failover.ts @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +const MIN_BACKOFF_DURATION = 30_000; // 30 seconds in milliseconds +const MAX_BACKOFF_DURATION = 10 * 60 * 1000; // 10 minutes in milliseconds +const MAX_SAFE_EXPONENTIAL = 30; // Used to avoid overflow. bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. +const JITTER_RATIO = 0.25; + +// Reference: https://github.com/Azure/AppConfiguration-DotnetProvider/blob/main/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/TimeSpanExtensions.cs#L14 +export function getFixedBackoffDuration(timeElapsed: number): number | undefined { + if (timeElapsed <= 100_000) { // 100 seconds in milliseconds + return 5_000; // 5 seconds in milliseconds + } + if (timeElapsed <= 200_000) { // 200 seconds in milliseconds + return 10_000; // 10 seconds in milliseconds + } + if (timeElapsed <= 10 * 60 * 1000) { // 10 minutes in milliseconds + return MIN_BACKOFF_DURATION; + } + return undefined; +} + +export function calculateDynamicBackoffDuration(failedAttempts: number) { + if (failedAttempts <= 1) { + return MIN_BACKOFF_DURATION; + } + + // exponential: minBackoff * 2 ^ (failedAttempts - 1) + const exponential = Math.min(failedAttempts - 1, MAX_SAFE_EXPONENTIAL); + let calculatedBackoffDuration = MIN_BACKOFF_DURATION * (1 << exponential); + if (calculatedBackoffDuration > MAX_BACKOFF_DURATION) { + calculatedBackoffDuration = MAX_BACKOFF_DURATION; + } + + // jitter: random value between [-1, 1) * jitterRatio * calculatedBackoffMs + const jitter = JITTER_RATIO * (Math.random() * 2 - 1); + + return calculatedBackoffDuration * (1 + jitter); +} diff --git a/src/load.ts b/src/load.ts index 4d24174e..44439612 100644 --- a/src/load.ts +++ b/src/load.ts @@ -7,7 +7,7 @@ import { AzureAppConfigurationImpl } from "./AzureAppConfigurationImpl.js"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; import { ConfigurationClientManager, instanceOfTokenCredential } from "./ConfigurationClientManager.js"; -const MIN_DELAY_FOR_UNHANDLED_ERROR: number = 5000; // 5 seconds +const MIN_DELAY_FOR_UNHANDLED_ERROR: number = 5_000; // 5 seconds /** * Loads the data from Azure App Configuration service and returns an instance of AzureAppConfiguration. From c15cf1b613ebe55d497b7d1be61e8183150820e1 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 18 Feb 2025 00:24:50 +0800 Subject: [PATCH 02/47] update --- src/AzureAppConfigurationImpl.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index a1121bb0..b99e27b8 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -50,7 +50,7 @@ type PagedSettingSelector = SettingSelector & { }; const DEFAULT_STARTUP_TIMEOUT = 100 * 1000; // 100 seconds in milliseconds -const MAX_STARTUP_TIMEOUT = 30 * 60 * 1000; // 15 minutes in milliseconds +const MAX_STARTUP_TIMEOUT = 60 * 60 * 1000; // 60 minutes in milliseconds export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** @@ -333,7 +333,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Initializes the configuration provider. */ async #initialize() { - if (this.#isInitialLoadCompleted) { + if (!this.#isInitialLoadCompleted) { await this.#inspectFmPackage(); const startTimestamp = Date.now(); while (startTimestamp + MAX_STARTUP_TIMEOUT > Date.now()) { @@ -357,6 +357,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { console.warn("Failed to load configuration settings at startup. Retrying..."); } } + if (!this.#isInitialLoadCompleted) { + throw new Error("Load operation exceeded the maximum startup timeout."); + } } } From 15c7e54185c661f4ff51e7fea97f251c1083666f Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 18 Feb 2025 00:59:12 +0800 Subject: [PATCH 03/47] update --- src/AzureAppConfigurationImpl.ts | 27 ++++++++++++++------------- src/StartupOptions.ts | 8 ++++++++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index b99e27b8..16389437 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -50,7 +50,6 @@ type PagedSettingSelector = SettingSelector & { }; const DEFAULT_STARTUP_TIMEOUT = 100 * 1000; // 100 seconds in milliseconds -const MAX_STARTUP_TIMEOUT = 60 * 60 * 1000; // 60 minutes in milliseconds export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** @@ -335,8 +334,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #initialize() { if (!this.#isInitialLoadCompleted) { await this.#inspectFmPackage(); + const retryEnabled = this.#options?.startupOptions?.retryEnabled ?? false; const startTimestamp = Date.now(); - while (startTimestamp + MAX_STARTUP_TIMEOUT > Date.now()) { + while (true) { try { await this.#loadSelectedAndWatchedKeyValues(); if (this.#featureFlagEnabled) { @@ -346,20 +346,21 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#isInitialLoadCompleted = true; break; } catch (error) { - const timeElapsed = Date.now() - startTimestamp; - let postAttempts = 0; - let backoffDuration = getFixedBackoffDuration(timeElapsed); - if (backoffDuration === undefined) { - postAttempts += 1; - backoffDuration = calculateDynamicBackoffDuration(postAttempts); + if (retryEnabled) { + const timeElapsed = Date.now() - startTimestamp; + let postAttempts = 0; + let backoffDuration = getFixedBackoffDuration(timeElapsed); + if (backoffDuration === undefined) { + postAttempts += 1; + backoffDuration = calculateDynamicBackoffDuration(postAttempts); + } + await new Promise(resolve => setTimeout(resolve, backoffDuration)); + console.warn("Failed to load configuration settings at startup. Retrying..."); + } else { + throw error; } - await new Promise(resolve => setTimeout(resolve, backoffDuration)); - console.warn("Failed to load configuration settings at startup. Retrying..."); } } - if (!this.#isInitialLoadCompleted) { - throw new Error("Load operation exceeded the maximum startup timeout."); - } } } diff --git a/src/StartupOptions.ts b/src/StartupOptions.ts index 626086b8..64c48d79 100644 --- a/src/StartupOptions.ts +++ b/src/StartupOptions.ts @@ -2,6 +2,14 @@ // Licensed under the MIT license. export interface StartupOptions { + /** + * Specifies whether to enable retry on startup or not. + * + * @remarks + * If not specified, the default value is false. + */ + retryEnabled?: boolean; + /** * The amount of time allowed to load data from Azure App Configuration on startup. * From 90c4159aca6cf004a7df8c93467430d5b0fa0c5b Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 18 Feb 2025 01:06:34 +0800 Subject: [PATCH 04/47] update --- src/AzureAppConfigurationImpl.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 16389437..4a61f677 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -50,6 +50,7 @@ type PagedSettingSelector = SettingSelector & { }; const DEFAULT_STARTUP_TIMEOUT = 100 * 1000; // 100 seconds in milliseconds +const MAX_STARTUP_TIMEOUT = 60 * 60 * 1000; // 60 minutes in milliseconds export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** @@ -336,7 +337,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { await this.#inspectFmPackage(); const retryEnabled = this.#options?.startupOptions?.retryEnabled ?? false; const startTimestamp = Date.now(); - while (true) { + while (startTimestamp + MAX_STARTUP_TIMEOUT > Date.now()) { try { await this.#loadSelectedAndWatchedKeyValues(); if (this.#featureFlagEnabled) { @@ -361,6 +362,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } } } + if (!this.#isInitialLoadCompleted) { + throw new Error("Load operation exceeded the maximum startup timeout limitation."); + } } } From a0e6543bbb769e0865f142176fd8a27e041f18c9 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 18 Feb 2025 03:27:12 +0800 Subject: [PATCH 05/47] add testcase --- src/AzureAppConfigurationImpl.ts | 2 +- src/StartupOptions.ts | 2 +- test/clientOptions.test.ts | 9 +++++ test/failover.test.ts | 3 +- test/keyvault.test.ts | 6 ++- test/requestTracing.test.ts | 50 +++++++++++++++++-------- test/startup.test.ts | 64 ++++++++++++++++++++++++++++++++ 7 files changed, 116 insertions(+), 20 deletions(-) create mode 100644 test/startup.test.ts diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 4a61f677..980d3a6e 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -335,7 +335,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #initialize() { if (!this.#isInitialLoadCompleted) { await this.#inspectFmPackage(); - const retryEnabled = this.#options?.startupOptions?.retryEnabled ?? false; + const retryEnabled = this.#options?.startupOptions?.retryEnabled ?? true; // enable startup retry by default const startTimestamp = Date.now(); while (startTimestamp + MAX_STARTUP_TIMEOUT > Date.now()) { try { diff --git a/src/StartupOptions.ts b/src/StartupOptions.ts index 64c48d79..69fa4797 100644 --- a/src/StartupOptions.ts +++ b/src/StartupOptions.ts @@ -6,7 +6,7 @@ export interface StartupOptions { * Specifies whether to enable retry on startup or not. * * @remarks - * If not specified, the default value is false. + * If not specified, the default value is true. */ retryEnabled?: boolean; diff --git a/test/clientOptions.test.ts b/test/clientOptions.test.ts index 2e9417e9..3de808f8 100644 --- a/test/clientOptions.test.ts +++ b/test/clientOptions.test.ts @@ -48,6 +48,9 @@ describe("custom client options", function () { policy: countPolicy, position: "perRetry" }] + }, + startupOptions: { + retryEnabled: false } }); }; @@ -73,6 +76,9 @@ describe("custom client options", function () { retryOptions: { maxRetries } + }, + startupOptions: { + retryEnabled: false } }); }; @@ -108,6 +114,9 @@ describe("custom client options", function () { policy: countPolicy, position: "perRetry" }] + }, + startupOptions: { + retryEnabled: false } }); }; diff --git a/test/failover.test.ts b/test/failover.test.ts index e1f2f043..db1a9105 100644 --- a/test/failover.test.ts +++ b/test/failover.test.ts @@ -69,7 +69,8 @@ describe("failover", function () { mockConfigurationManagerGetClients([], isFailoverable); const connectionString = createMockedConnectionString(); - return expect(load(connectionString)).eventually.rejectedWith("All clients failed to get configuration settings."); + return expect(load(connectionString, {startupOptions: {retryEnabled: false}})) + .eventually.rejectedWith("All clients failed to get configuration settings."); }); it("should validate endpoint", () => { diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index e88044ea..3fade4c3 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -26,6 +26,7 @@ function mockNewlyCreatedKeyVaultSecretClients() { // eslint-disable-next-line @typescript-eslint/no-unused-vars mockSecretClientGetSecret(mockedData.map(([_key, secretUri, value]) => [secretUri, value])); } + describe("key vault reference", function () { this.timeout(MAX_TIME_OUT); @@ -39,7 +40,7 @@ describe("key vault reference", function () { }); it("require key vault options to resolve reference", async () => { - return expect(load(createMockedConnectionString())).eventually.rejectedWith("Configure keyVaultOptions to resolve Key Vault Reference(s)."); + return expect(load(createMockedConnectionString(), {startupOptions: {retryEnabled: false}})).eventually.rejectedWith("Configure keyVaultOptions to resolve Key Vault Reference(s)."); }); it("should resolve key vault reference with credential", async () => { @@ -93,6 +94,9 @@ describe("key vault reference", function () { secretClients: [ new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()), ] + }, + startupOptions: { + retryEnabled: false } }); return expect(loadKeyVaultPromise).eventually.rejectedWith("No key vault credential or secret resolver callback configured, and no matching secret client could be found."); diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index ed3fdaee..50a75bcc 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -35,7 +35,7 @@ describe("request tracing", function () { it("should have correct user agent prefix", async () => { try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} } ); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; expect(headerPolicy.headers.get("User-Agent")).satisfy((ua: string) => ua.startsWith("javascript-appconfiguration-provider")); @@ -43,7 +43,7 @@ describe("request tracing", function () { it("should have request type in correlation-context header", async () => { try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; expect(headerPolicy.headers.get("Correlation-Context")).eq("RequestType=Startup"); @@ -53,6 +53,9 @@ describe("request tracing", function () { try { await load(createMockedConnectionString(fakeEndpoint), { clientOptions, + startupOptions: { + retryEnabled: false + }, keyVaultOptions: { credential: createMockedTokenCredential() } @@ -68,7 +71,7 @@ describe("request tracing", function () { const replicaCount = 2; sinon.stub(ConfigurationClientManager.prototype, "getReplicaCount").returns(replicaCount); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -80,7 +83,7 @@ describe("request tracing", function () { it("should detect env in correlation-context header", async () => { process.env.NODE_ENV = "development"; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -92,7 +95,7 @@ describe("request tracing", function () { it("should detect host type in correlation-context header", async () => { process.env.WEBSITE_SITE_NAME = "website-name"; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -105,7 +108,7 @@ describe("request tracing", function () { for (const indicator of ["TRUE", "true"]) { process.env.AZURE_APP_CONFIGURATION_TRACING_DISABLED = indicator; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -124,6 +127,9 @@ describe("request tracing", function () { const settings = await load(createMockedConnectionString(fakeEndpoint), { clientOptions, + startupOptions: { + retryEnabled: false + }, refreshOptions: { enabled: true, refreshIntervalInMs: 1000, @@ -157,6 +163,9 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { + startupOptions: { + retryEnabled: false + }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -195,6 +204,9 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { + startupOptions: { + retryEnabled: false + }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -231,6 +243,9 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { + startupOptions: { + retryEnabled: false + }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -268,6 +283,9 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { + startupOptions: { + retryEnabled: false + }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -330,7 +348,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -348,7 +366,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -366,7 +384,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -384,7 +402,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -402,7 +420,7 @@ describe("request tracing", function () { (global as any).importScripts = undefined; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -440,7 +458,7 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -455,7 +473,7 @@ describe("request tracing", function () { (global as any).document = undefined; // not an instance of Document try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -470,7 +488,7 @@ describe("request tracing", function () { (global as any).document = {}; // Not an instance of Document try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -485,7 +503,7 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -500,7 +518,7 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); diff --git a/test/startup.test.ts b/test/startup.test.ts new file mode 100644 index 00000000..0f237e72 --- /dev/null +++ b/test/startup.test.ts @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import * as chai from "chai"; +import * as chaiAsPromised from "chai-as-promised"; +chai.use(chaiAsPromised); +const expect = chai.expect; +import { load } from "./exportedApi"; +import { MAX_TIME_OUT, createMockedConnectionString, createMockedKeyValue, mockAppConfigurationClientListConfigurationSettings, restoreMocks } from "./utils/testHelper.js"; + +describe("startup", function () { + this.timeout(MAX_TIME_OUT); + + afterEach(() => { + restoreMocks(); + }); + + it("should throw exception when timeout", async () => { + expect(load(createMockedConnectionString(), {startupOptions: {timeoutInMs: 1}})) + .eventually.rejectedWith("Load operation timed out."); + }); + + it("should retry for load operation when retryEnabled is true", async () => { + let attempt = 0; + const failForInitialAttempt = () => { + if (attempt < 1) { + attempt += 1; + throw new Error("Failed to list configuration settings."); + } + }; + mockAppConfigurationClientListConfigurationSettings( + [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], + failForInitialAttempt); + + const settings = await load( + createMockedConnectionString(), { + startupOptions: { + retryEnabled: true + } + } + ); + expect(settings).not.undefined; + expect(settings.get("TestKey")).eq("TestValue"); + }); + + it("should not retry for load operation when retryEnabled is false", async () => { + let attempt = 0; + const failForInitialAttempt = () => { + if (attempt < 1) { + attempt += 1; + throw new Error("Test Error"); + } + }; + mockAppConfigurationClientListConfigurationSettings( + [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], + failForInitialAttempt); + + return expect(load(createMockedConnectionString(), { + startupOptions: { + retryEnabled: false + } + })).eventually.rejectedWith("Test Error"); + }); +}); From 435ff080b05d90a34f95b49b5900b7f1edcf389c Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 20 Feb 2025 01:40:44 +0800 Subject: [PATCH 06/47] clarify error type --- package.json | 2 +- src/AzureAppConfigurationImpl.ts | 86 ++++++++++---------- src/ConfigurationClientManager.ts | 8 +- src/StartupOptions.ts | 10 +-- src/error.ts | 32 ++++++++ src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 5 +- test/clientOptions.test.ts | 9 -- test/failover.test.ts | 11 +-- test/keyvault.test.ts | 5 +- test/requestTracing.test.ts | 50 ++++-------- test/startup.test.ts | 29 +------ 11 files changed, 104 insertions(+), 143 deletions(-) create mode 100644 src/error.ts diff --git a/package.json b/package.json index 04b4e526..5cd8ec11 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "dev": "rollup --config --watch", "lint": "eslint src/ test/", "fix-lint": "eslint src/ test/ --fix", - "test": "mocha out/test/*.test.{js,cjs,mjs} --parallel" + "test": "mocha out/test/failover.test.{js,cjs,mjs} --parallel" }, "repository": { "type": "git", diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 980d3a6e..9eca1c42 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -41,6 +41,7 @@ import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOp import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; import { getFixedBackoffDuration, calculateDynamicBackoffDuration } from "./failover.js"; +import { FailoverError, OperationError, isFailoverableError } from "./error.js"; type PagedSettingSelector = SettingSelector & { /** @@ -50,7 +51,6 @@ type PagedSettingSelector = SettingSelector & { }; const DEFAULT_STARTUP_TIMEOUT = 100 * 1000; // 100 seconds in milliseconds -const MAX_STARTUP_TIMEOUT = 60 * 60 * 1000; // 60 minutes in milliseconds export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** @@ -127,10 +127,10 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } else { for (const setting of watchedSettings) { if (setting.key.includes("*") || setting.key.includes(",")) { - throw new Error("The characters '*' and ',' are not supported in key of watched settings."); + throw new RangeError("The characters '*' and ',' are not supported in key of watched settings."); } if (setting.label?.includes("*") || setting.label?.includes(",")) { - throw new Error("The characters '*' and ',' are not supported in label of watched settings."); + throw new RangeError("The characters '*' and ',' are not supported in label of watched settings."); } this.#sentinels.push(setting); } @@ -139,7 +139,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { - throw new Error(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new RangeError(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { this.#kvRefreshInterval = refreshIntervalInMs; } @@ -157,7 +157,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { - throw new Error(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new RangeError(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { this.#ffRefreshInterval = refreshIntervalInMs; } @@ -233,17 +233,29 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Loads the configuration store for the first time. */ async load() { - const startupTimeout = this.#options?.startupOptions?.timeoutInMs ?? DEFAULT_STARTUP_TIMEOUT; - let timer; + const startupTimeout: number = this.#options?.startupOptions?.timeoutInMs ?? DEFAULT_STARTUP_TIMEOUT; + const abortController = new AbortController(); + const abortSignal = abortController.signal; + let timeoutId; try { + // Promise.race will be settled when the first promise in the list is settled + // It will not cancel the remaining promises in the list. + // To avoid memory leaks, we need to cancel other promises when one promise is settled. await Promise.race([ - new Promise((_, reject) => timer = setTimeout(() => reject(new Error("Load operation timed out.")), startupTimeout)), - this.#initialize() + this.#initializeWithRetryPolicy(abortSignal), + // this promise will be rejected after timeout + new Promise((_, reject) => { + timeoutId = setTimeout(() => { + abortController.abort(); // abort the initialization promise + reject(new Error("Load operation timed out.")); + }, + startupTimeout); + }) ]); } catch (error) { throw new Error(`Failed to load: ${error.message}`); } finally { - clearTimeout(timer); + clearTimeout(timeoutId); // cancel the timeout promise } } @@ -254,7 +266,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const separator = options?.separator ?? "."; const validSeparators = [".", ",", ";", "-", "_", "__", "/", ":"]; if (!validSeparators.includes(separator)) { - throw new Error(`Invalid separator '${separator}'. Supported values: ${validSeparators.map(s => `'${s}'`).join(", ")}.`); + throw new RangeError(`Invalid separator '${separator}'. Supported values: ${validSeparators.map(s => `'${s}'`).join(", ")}.`); } // construct hierarchical data object from map @@ -267,7 +279,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const segment = segments[i]; // undefined or empty string if (!segment) { - throw new Error(`invalid key: ${key}`); + throw new OperationError(`Failed to construct configuration object: Invalid key: ${key}`); } // create path if not exist if (current[segment] === undefined) { @@ -275,14 +287,14 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } // The path has been occupied by a non-object value, causing ambiguity. if (typeof current[segment] !== "object") { - throw new Error(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The path '${segments.slice(0, i + 1).join(separator)}' has been occupied.`); + throw new OperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The path '${segments.slice(0, i + 1).join(separator)}' has been occupied.`); } current = current[segment]; } const lastSegment = segments[segments.length - 1]; if (current[lastSegment] !== undefined) { - throw new Error(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The key should not be part of another key.`); + throw new OperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The key should not be part of another key.`); } // set value to the last segment current[lastSegment] = value; @@ -295,7 +307,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ async refresh(): Promise { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new Error("Refresh is not enabled for key-values or feature flags."); + throw new OperationError("Refresh is not enabled for key-values or feature flags."); } if (this.#refreshInProgress) { @@ -314,7 +326,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ onRefresh(listener: () => any, thisArg?: any): Disposable { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new Error("Refresh is not enabled for key-values or feature flags."); + throw new OperationError("Refresh is not enabled for key-values or feature flags."); } const boundedListener = listener.bind(thisArg); @@ -332,12 +344,11 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** * Initializes the configuration provider. */ - async #initialize() { + async #initializeWithRetryPolicy(abortSignal: AbortSignal) { if (!this.#isInitialLoadCompleted) { await this.#inspectFmPackage(); - const retryEnabled = this.#options?.startupOptions?.retryEnabled ?? true; // enable startup retry by default const startTimestamp = Date.now(); - while (startTimestamp + MAX_STARTUP_TIMEOUT > Date.now()) { + while (!abortSignal.aborted) { try { await this.#loadSelectedAndWatchedKeyValues(); if (this.#featureFlagEnabled) { @@ -347,24 +358,17 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#isInitialLoadCompleted = true; break; } catch (error) { - if (retryEnabled) { - const timeElapsed = Date.now() - startTimestamp; - let postAttempts = 0; - let backoffDuration = getFixedBackoffDuration(timeElapsed); - if (backoffDuration === undefined) { - postAttempts += 1; - backoffDuration = calculateDynamicBackoffDuration(postAttempts); - } - await new Promise(resolve => setTimeout(resolve, backoffDuration)); - console.warn("Failed to load configuration settings at startup. Retrying..."); - } else { - throw error; + const timeElapsed = Date.now() - startTimestamp; + let postAttempts = 0; + let backoffDuration = getFixedBackoffDuration(timeElapsed); + if (backoffDuration === undefined) { + postAttempts += 1; + backoffDuration = calculateDynamicBackoffDuration(postAttempts); } + await new Promise(resolve => setTimeout(resolve, backoffDuration)); + console.warn("Failed to load configuration settings at startup. Retrying..."); } } - if (!this.#isInitialLoadCompleted) { - throw new Error("Load operation exceeded the maximum startup timeout limitation."); - } } } @@ -687,7 +691,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } this.#clientManager.refreshClients(); - throw new Error("All clients failed to get configuration settings."); + throw new FailoverError("All fallback clients failed to get configuration settings."); } async #processKeyValues(setting: ConfigurationSetting): Promise<[string, unknown]> { @@ -719,7 +723,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #parseFeatureFlag(setting: ConfigurationSetting): Promise { const rawFlag = setting.value; if (rawFlag === undefined) { - throw new Error("The value of configuration setting cannot be undefined."); + throw new RangeError("The value of configuration setting cannot be undefined."); } const featureFlag = JSON.parse(rawFlag); @@ -943,13 +947,13 @@ function getValidSelectors(selectors: SettingSelector[]): SettingSelector[] { return uniqueSelectors.map(selectorCandidate => { const selector = { ...selectorCandidate }; if (!selector.keyFilter) { - throw new Error("Key filter cannot be null or empty."); + throw new RangeError("Key filter cannot be null or empty."); } if (!selector.labelFilter) { selector.labelFilter = LabelFilter.Null; } if (selector.labelFilter.includes("*") || selector.labelFilter.includes(",")) { - throw new Error("The characters '*' and ',' are not supported in label filters."); + throw new RangeError("The characters '*' and ',' are not supported in label filters."); } return selector; }); @@ -973,9 +977,3 @@ function getValidFeatureFlagSelectors(selectors?: SettingSelector[]): SettingSel }); return getValidSelectors(selectors); } - -function isFailoverableError(error: any): boolean { - // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory - return isRestError(error) && (error.code === "ENOTFOUND" || error.code === "ENOENT" || - (error.statusCode !== undefined && (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 408 || error.statusCode === 429 || error.statusCode >= 500))); -} diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index b1574c2f..9c760b1d 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -8,6 +8,7 @@ import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js" import { isBrowser, isWebWorker } from "./requestTracing/utils.js"; import * as RequestTracing from "./requestTracing/constants.js"; import { shuffleList } from "./common/utils.js"; +import { OperationError } from "./error.js"; // Configuration client retry options const CLIENT_MAX_RETRIES = 2; @@ -60,7 +61,7 @@ export class ConfigurationClientManager { this.#id = regexMatch[2]; this.#secret = regexMatch[3]; } else { - throw new Error(`Invalid connection string. Valid connection strings should match the regex '${ConnectionStringRegex.source}'.`); + throw new RangeError(`Invalid connection string. Valid connection strings should match the regex '${ConnectionStringRegex.source}'.`); } staticClient = new AppConfigurationClient(connectionString, this.#clientOptions); } else if ((connectionStringOrEndpoint instanceof URL || typeof connectionStringOrEndpoint === "string") && credentialPassed) { @@ -77,7 +78,7 @@ export class ConfigurationClientManager { this.#credential = credential; staticClient = new AppConfigurationClient(this.endpoint.origin, this.#credential, this.#clientOptions); } else { - throw new Error("A connection string or an endpoint with credential must be specified to create a client."); + throw new OperationError("A connection string or an endpoint with credential must be specified to create a client."); } this.#staticClients = [new ConfigurationClientWrapper(this.endpoint.origin, staticClient)]; @@ -150,7 +151,8 @@ export class ConfigurationClientManager { let timer; try { result = await Promise.race([ - new Promise((_, reject) => timer = setTimeout(() => reject(new Error("SRV record query timed out.")), SRV_QUERY_TIMEOUT)), + new Promise((_, reject) => + timer = setTimeout(() => reject(new Error("SRV record query timed out.")), SRV_QUERY_TIMEOUT)), this.#querySrvTargetHost(host) ]); } catch (error) { diff --git a/src/StartupOptions.ts b/src/StartupOptions.ts index 69fa4797..febb3808 100644 --- a/src/StartupOptions.ts +++ b/src/StartupOptions.ts @@ -2,19 +2,11 @@ // Licensed under the MIT license. export interface StartupOptions { - /** - * Specifies whether to enable retry on startup or not. - * - * @remarks - * If not specified, the default value is true. - */ - retryEnabled?: boolean; - /** * The amount of time allowed to load data from Azure App Configuration on startup. * * @remarks - * If not specified, the default value is 100 seconds. + * If not specified, the default value is 100 seconds. The maximum allowed value is 1 hour. */ timeoutInMs?: number; } diff --git a/src/error.ts b/src/error.ts new file mode 100644 index 00000000..6eebe846 --- /dev/null +++ b/src/error.ts @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { isRestError } from "@azure/core-rest-pipeline"; + +export class OperationError extends Error { + constructor(message: string) { + super(message); + this.name = "OperationError"; + } +} + +export class FailoverError extends Error { + constructor(message: string) { + super(message); + this.name = "FailoverError"; + } +} + +export function isFailoverableError(error: any): boolean { + // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory + return isRestError(error) && (error.code === "ENOTFOUND" || error.code === "ENOENT" || + (error.statusCode !== undefined && (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 408 || error.statusCode === 429 || error.statusCode >= 500))); +} + +export function isRetriableError(error: any): boolean { + if (error instanceof OperationError || + error instanceof RangeError) { + return false; + } + return true; +} \ No newline at end of file diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 1b6fdcc4..6f5ce301 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -5,6 +5,7 @@ import { ConfigurationSetting, isSecretReference, parseSecretReference } from "@ import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; +import { OperationError } from "../error.js"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { /** @@ -24,7 +25,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { async processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> { // TODO: cache results to save requests. if (!this.#keyVaultOptions) { - throw new Error("Configure keyVaultOptions to resolve Key Vault Reference(s)."); + throw new OperationError("Failed to process the key vault reference. The keyVaultOptions is not configured."); } // precedence: secret clients > credential > secret resolver @@ -43,7 +44,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; } - throw new Error("No key vault credential or secret resolver callback configured, and no matching secret client could be found."); + throw new OperationError("Failed to process the key vault reference. No key vault credential or secret resolver callback configured, and no matching secret client could be found."); } #getSecretClient(vaultUrl: URL): SecretClient | undefined { diff --git a/test/clientOptions.test.ts b/test/clientOptions.test.ts index 3de808f8..2e9417e9 100644 --- a/test/clientOptions.test.ts +++ b/test/clientOptions.test.ts @@ -48,9 +48,6 @@ describe("custom client options", function () { policy: countPolicy, position: "perRetry" }] - }, - startupOptions: { - retryEnabled: false } }); }; @@ -76,9 +73,6 @@ describe("custom client options", function () { retryOptions: { maxRetries } - }, - startupOptions: { - retryEnabled: false } }); }; @@ -114,9 +108,6 @@ describe("custom client options", function () { policy: countPolicy, position: "perRetry" }] - }, - startupOptions: { - retryEnabled: false } }); }; diff --git a/test/failover.test.ts b/test/failover.test.ts index db1a9105..51cc6945 100644 --- a/test/failover.test.ts +++ b/test/failover.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi"; -import { MAX_TIME_OUT, createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockConfigurationManagerGetClients, restoreMocks } from "./utils/testHelper"; +import { MAX_TIME_OUT, createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockConfigurationManagerGetClients, restoreMocks, sleepInMs } from "./utils/testHelper"; import { getValidDomain, isValidEndpoint } from "../src/ConfigurationClientManager"; const mockedKVs = [{ @@ -64,15 +64,6 @@ describe("failover", function () { expect(settings.get("feature_management").feature_flags).not.undefined; }); - it("should throw error when all clients failed", async () => { - const isFailoverable = false; - mockConfigurationManagerGetClients([], isFailoverable); - - const connectionString = createMockedConnectionString(); - return expect(load(connectionString, {startupOptions: {retryEnabled: false}})) - .eventually.rejectedWith("All clients failed to get configuration settings."); - }); - it("should validate endpoint", () => { const fakeHost = "fake.azconfig.io"; const validDomain = getValidDomain(fakeHost); diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 3fade4c3..5e2aa97d 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -40,7 +40,7 @@ describe("key vault reference", function () { }); it("require key vault options to resolve reference", async () => { - return expect(load(createMockedConnectionString(), {startupOptions: {retryEnabled: false}})).eventually.rejectedWith("Configure keyVaultOptions to resolve Key Vault Reference(s)."); + return expect(load(createMockedConnectionString())).eventually.rejectedWith("Configure keyVaultOptions to resolve Key Vault Reference(s)."); }); it("should resolve key vault reference with credential", async () => { @@ -94,9 +94,6 @@ describe("key vault reference", function () { secretClients: [ new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()), ] - }, - startupOptions: { - retryEnabled: false } }); return expect(loadKeyVaultPromise).eventually.rejectedWith("No key vault credential or secret resolver callback configured, and no matching secret client could be found."); diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index 50a75bcc..ed3fdaee 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -35,7 +35,7 @@ describe("request tracing", function () { it("should have correct user agent prefix", async () => { try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} } ); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; expect(headerPolicy.headers.get("User-Agent")).satisfy((ua: string) => ua.startsWith("javascript-appconfiguration-provider")); @@ -43,7 +43,7 @@ describe("request tracing", function () { it("should have request type in correlation-context header", async () => { try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; expect(headerPolicy.headers.get("Correlation-Context")).eq("RequestType=Startup"); @@ -53,9 +53,6 @@ describe("request tracing", function () { try { await load(createMockedConnectionString(fakeEndpoint), { clientOptions, - startupOptions: { - retryEnabled: false - }, keyVaultOptions: { credential: createMockedTokenCredential() } @@ -71,7 +68,7 @@ describe("request tracing", function () { const replicaCount = 2; sinon.stub(ConfigurationClientManager.prototype, "getReplicaCount").returns(replicaCount); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -83,7 +80,7 @@ describe("request tracing", function () { it("should detect env in correlation-context header", async () => { process.env.NODE_ENV = "development"; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -95,7 +92,7 @@ describe("request tracing", function () { it("should detect host type in correlation-context header", async () => { process.env.WEBSITE_SITE_NAME = "website-name"; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -108,7 +105,7 @@ describe("request tracing", function () { for (const indicator of ["TRUE", "true"]) { process.env.AZURE_APP_CONFIGURATION_TRACING_DISABLED = indicator; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -127,9 +124,6 @@ describe("request tracing", function () { const settings = await load(createMockedConnectionString(fakeEndpoint), { clientOptions, - startupOptions: { - retryEnabled: false - }, refreshOptions: { enabled: true, refreshIntervalInMs: 1000, @@ -163,9 +157,6 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { - startupOptions: { - retryEnabled: false - }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -204,9 +195,6 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { - startupOptions: { - retryEnabled: false - }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -243,9 +231,6 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { - startupOptions: { - retryEnabled: false - }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -283,9 +268,6 @@ describe("request tracing", function () { ]], listKvCallback); const settings = await load(createMockedConnectionString(fakeEndpoint), { - startupOptions: { - retryEnabled: false - }, featureFlagOptions: { enabled: true, selectors: [ {keyFilter: "*"} ], @@ -348,7 +330,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -366,7 +348,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -384,7 +366,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -402,7 +384,7 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -420,7 +402,7 @@ describe("request tracing", function () { (global as any).importScripts = undefined; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -458,7 +440,7 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -473,7 +455,7 @@ describe("request tracing", function () { (global as any).document = undefined; // not an instance of Document try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -488,7 +470,7 @@ describe("request tracing", function () { (global as any).document = {}; // Not an instance of Document try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -503,7 +485,7 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -518,7 +500,7 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions, startupOptions: {retryEnabled: false} }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); diff --git a/test/startup.test.ts b/test/startup.test.ts index 0f237e72..a89bd993 100644 --- a/test/startup.test.ts +++ b/test/startup.test.ts @@ -25,40 +25,15 @@ describe("startup", function () { const failForInitialAttempt = () => { if (attempt < 1) { attempt += 1; - throw new Error("Failed to list configuration settings."); + throw new Error("Test Error"); } }; mockAppConfigurationClientListConfigurationSettings( [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], failForInitialAttempt); - const settings = await load( - createMockedConnectionString(), { - startupOptions: { - retryEnabled: true - } - } - ); + const settings = await load(createMockedConnectionString()); expect(settings).not.undefined; expect(settings.get("TestKey")).eq("TestValue"); }); - - it("should not retry for load operation when retryEnabled is false", async () => { - let attempt = 0; - const failForInitialAttempt = () => { - if (attempt < 1) { - attempt += 1; - throw new Error("Test Error"); - } - }; - mockAppConfigurationClientListConfigurationSettings( - [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], - failForInitialAttempt); - - return expect(load(createMockedConnectionString(), { - startupOptions: { - retryEnabled: false - } - })).eventually.rejectedWith("Test Error"); - }); }); From 6a8ceb750e65a57af3f835db98aa985b4e70a6b1 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 20 Feb 2025 04:16:06 +0800 Subject: [PATCH 07/47] update --- package.json | 2 +- src/AzureAppConfigurationImpl.ts | 20 +++-- src/StartupOptions.ts | 4 +- src/error.ts | 10 ++- src/refresh/RefreshTimer.ts | 6 +- test/clientOptions.test.ts | 9 +++ test/failover.test.ts | 2 +- test/keyvault.test.ts | 4 +- test/requestTracing.test.ts | 135 +++++++++++++++++++++++++------ test/startup.test.ts | 48 +++++++++-- 10 files changed, 188 insertions(+), 52 deletions(-) diff --git a/package.json b/package.json index 5cd8ec11..6e4df91a 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "dev": "rollup --config --watch", "lint": "eslint src/ test/", "fix-lint": "eslint src/ test/ --fix", - "test": "mocha out/test/failover.test.{js,cjs,mjs} --parallel" + "test": "mocha out/test/*.test.{js,cjs,mjs}" }, "repository": { "type": "git", diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 9eca1c42..bbc3eab6 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -7,6 +7,7 @@ import { AzureAppConfiguration, ConfigurationObjectConstructionOptions } from ". import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; import { IKeyValueAdapter } from "./IKeyValueAdapter.js"; import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter.js"; +import { DEFAULT_STARTUP_TIMEOUT_IN_MS } from "./StartupOptions.js"; import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./RefreshOptions.js"; import { Disposable } from "./common/disposable.js"; import { base64Helper, jsonSorter } from "./common/utils.js"; @@ -41,7 +42,7 @@ import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOp import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; import { getFixedBackoffDuration, calculateDynamicBackoffDuration } from "./failover.js"; -import { FailoverError, OperationError, isFailoverableError } from "./error.js"; +import { FailoverError, OperationError, isFailoverableError, isRetriableError } from "./error.js"; type PagedSettingSelector = SettingSelector & { /** @@ -50,8 +51,6 @@ type PagedSettingSelector = SettingSelector & { pageEtags?: string[]; }; -const DEFAULT_STARTUP_TIMEOUT = 100 * 1000; // 100 seconds in milliseconds - export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** * Hosting key-value pairs in the configuration store. @@ -233,7 +232,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Loads the configuration store for the first time. */ async load() { - const startupTimeout: number = this.#options?.startupOptions?.timeoutInMs ?? DEFAULT_STARTUP_TIMEOUT; + const startupTimeout: number = this.#options?.startupOptions?.timeoutInMs ?? DEFAULT_STARTUP_TIMEOUT_IN_MS; const abortController = new AbortController(); const abortSignal = abortController.signal; let timeoutId; @@ -344,11 +343,11 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** * Initializes the configuration provider. */ - async #initializeWithRetryPolicy(abortSignal: AbortSignal) { + async #initializeWithRetryPolicy(abortSignal: AbortSignal): Promise { if (!this.#isInitialLoadCompleted) { await this.#inspectFmPackage(); const startTimestamp = Date.now(); - while (!abortSignal.aborted) { + do { // at least try to load once try { await this.#loadSelectedAndWatchedKeyValues(); if (this.#featureFlagEnabled) { @@ -358,6 +357,13 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#isInitialLoadCompleted = true; break; } catch (error) { + + if (!isRetriableError(error)) { + throw error; + } + if (abortSignal.aborted) { + return; + } const timeElapsed = Date.now() - startTimestamp; let postAttempts = 0; let backoffDuration = getFixedBackoffDuration(timeElapsed); @@ -368,7 +374,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { await new Promise(resolve => setTimeout(resolve, backoffDuration)); console.warn("Failed to load configuration settings at startup. Retrying..."); } - } + } while (!abortSignal.aborted); } } diff --git a/src/StartupOptions.ts b/src/StartupOptions.ts index febb3808..78ce048a 100644 --- a/src/StartupOptions.ts +++ b/src/StartupOptions.ts @@ -1,12 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +export const DEFAULT_STARTUP_TIMEOUT_IN_MS = 100 * 1000; // 100 seconds in milliseconds + export interface StartupOptions { /** * The amount of time allowed to load data from Azure App Configuration on startup. * * @remarks - * If not specified, the default value is 100 seconds. The maximum allowed value is 1 hour. + * If not specified, the default value is 100 seconds. Must be greater than 1 second. */ timeoutInMs?: number; } diff --git a/src/error.ts b/src/error.ts index 6eebe846..29409b7b 100644 --- a/src/error.ts +++ b/src/error.ts @@ -3,6 +3,9 @@ import { isRestError } from "@azure/core-rest-pipeline"; +/** + * Error thrown when an operation is not allowed to be performed. + */ export class OperationError extends Error { constructor(message: string) { super(message); @@ -10,6 +13,9 @@ export class OperationError extends Error { } } +/** + * Error thrown when fail to perform failover. + */ export class FailoverError extends Error { constructor(message: string) { super(message); @@ -24,9 +30,9 @@ export function isFailoverableError(error: any): boolean { } export function isRetriableError(error: any): boolean { - if (error instanceof OperationError || + if (error instanceof OperationError || error instanceof RangeError) { return false; } return true; -} \ No newline at end of file +} diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 45fdf0b3..044ee4e4 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -5,11 +5,9 @@ export class RefreshTimer { #backoffEnd: number; // Timestamp #interval: number; - constructor( - interval: number - ) { + constructor(interval: number) { if (interval <= 0) { - throw new Error(`Refresh interval must be greater than 0. Given: ${this.#interval}`); + throw new RangeError(`Refresh interval must be greater than 0. Given: ${this.#interval}`); } this.#interval = interval; diff --git a/test/clientOptions.test.ts b/test/clientOptions.test.ts index 2e9417e9..3401c19a 100644 --- a/test/clientOptions.test.ts +++ b/test/clientOptions.test.ts @@ -48,6 +48,9 @@ describe("custom client options", function () { policy: countPolicy, position: "perRetry" }] + }, + startupOptions: { + timeoutInMs: 5_000 } }); }; @@ -73,6 +76,9 @@ describe("custom client options", function () { retryOptions: { maxRetries } + }, + startupOptions: { + timeoutInMs: 5_000 } }); }; @@ -108,6 +114,9 @@ describe("custom client options", function () { policy: countPolicy, position: "perRetry" }] + }, + startupOptions: { + timeoutInMs: 5_000 } }); }; diff --git a/test/failover.test.ts b/test/failover.test.ts index 51cc6945..e7b491d7 100644 --- a/test/failover.test.ts +++ b/test/failover.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi"; -import { MAX_TIME_OUT, createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockConfigurationManagerGetClients, restoreMocks, sleepInMs } from "./utils/testHelper"; +import { MAX_TIME_OUT, createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockConfigurationManagerGetClients, restoreMocks } from "./utils/testHelper"; import { getValidDomain, isValidEndpoint } from "../src/ConfigurationClientManager"; const mockedKVs = [{ diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 5e2aa97d..0aded3a8 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -40,7 +40,7 @@ describe("key vault reference", function () { }); it("require key vault options to resolve reference", async () => { - return expect(load(createMockedConnectionString())).eventually.rejectedWith("Configure keyVaultOptions to resolve Key Vault Reference(s)."); + return expect(load(createMockedConnectionString())).eventually.rejectedWith("Failed to process the key vault reference. The keyVaultOptions is not configured."); }); it("should resolve key vault reference with credential", async () => { @@ -96,7 +96,7 @@ describe("key vault reference", function () { ] } }); - return expect(loadKeyVaultPromise).eventually.rejectedWith("No key vault credential or secret resolver callback configured, and no matching secret client could be found."); + return expect(loadKeyVaultPromise).eventually.rejectedWith("Failed to process the key vault reference. No key vault credential or secret resolver callback configured, and no matching secret client could be found."); }); it("should fallback to use default credential when corresponding secret client not provided", async () => { diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index ed3fdaee..e9d5edce 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -35,7 +35,12 @@ describe("request tracing", function () { it("should have correct user agent prefix", async () => { try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; expect(headerPolicy.headers.get("User-Agent")).satisfy((ua: string) => ua.startsWith("javascript-appconfiguration-provider")); @@ -43,7 +48,12 @@ describe("request tracing", function () { it("should have request type in correlation-context header", async () => { try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; expect(headerPolicy.headers.get("Correlation-Context")).eq("RequestType=Startup"); @@ -55,6 +65,9 @@ describe("request tracing", function () { clientOptions, keyVaultOptions: { credential: createMockedTokenCredential() + }, + startupOptions: { + timeoutInMs: 1 } }); } catch (e) { /* empty */ } @@ -68,7 +81,12 @@ describe("request tracing", function () { const replicaCount = 2; sinon.stub(ConfigurationClientManager.prototype, "getReplicaCount").returns(replicaCount); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -80,7 +98,12 @@ describe("request tracing", function () { it("should detect env in correlation-context header", async () => { process.env.NODE_ENV = "development"; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -92,7 +115,12 @@ describe("request tracing", function () { it("should detect host type in correlation-context header", async () => { process.env.WEBSITE_SITE_NAME = "website-name"; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -105,7 +133,12 @@ describe("request tracing", function () { for (const indicator of ["TRUE", "true"]) { process.env.AZURE_APP_CONFIGURATION_TRACING_DISABLED = indicator; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -126,13 +159,13 @@ describe("request tracing", function () { clientOptions, refreshOptions: { enabled: true, - refreshIntervalInMs: 1000, + refreshIntervalInMs: 1_000, watchedSettings: [{ key: "app.settings.fontColor" }] } }); - await sleepInMs(1000 + 1); + await sleepInMs(1_000 + 1_000); try { await settings.refresh(); } catch (e) { /* empty */ } @@ -162,7 +195,7 @@ describe("request tracing", function () { selectors: [ {keyFilter: "*"} ], refresh: { enabled: true, - refreshIntervalInMs: 1000 + refreshIntervalInMs: 1_000 } } }); @@ -170,7 +203,7 @@ describe("request tracing", function () { expect(correlationContext).not.undefined; expect(correlationContext?.includes("RequestType=Startup")).eq(true); - await sleepInMs(1000 + 1); + await sleepInMs(1_000 + 1_000); try { await settings.refresh(); } catch (e) { /* empty */ } @@ -200,7 +233,7 @@ describe("request tracing", function () { selectors: [ {keyFilter: "*"} ], refresh: { enabled: true, - refreshIntervalInMs: 1000 + refreshIntervalInMs: 1_000 } } }); @@ -208,7 +241,7 @@ describe("request tracing", function () { expect(correlationContext).not.undefined; expect(correlationContext?.includes("RequestType=Startup")).eq(true); - await sleepInMs(1000 + 1); + await sleepInMs(1_000 + 1_000); try { await settings.refresh(); } catch (e) { /* empty */ } @@ -236,7 +269,7 @@ describe("request tracing", function () { selectors: [ {keyFilter: "*"} ], refresh: { enabled: true, - refreshIntervalInMs: 1000 + refreshIntervalInMs: 1_000 } } }); @@ -244,7 +277,7 @@ describe("request tracing", function () { expect(correlationContext).not.undefined; expect(correlationContext?.includes("RequestType=Startup")).eq(true); - await sleepInMs(1000 + 1); + await sleepInMs(1_000 + 1_000); try { await settings.refresh(); } catch (e) { /* empty */ } @@ -273,7 +306,7 @@ describe("request tracing", function () { selectors: [ {keyFilter: "*"} ], refresh: { enabled: true, - refreshIntervalInMs: 1000 + refreshIntervalInMs: 1_000 } } }); @@ -281,7 +314,7 @@ describe("request tracing", function () { expect(correlationContext).not.undefined; expect(correlationContext?.includes("RequestType=Startup")).eq(true); - await sleepInMs(1000 + 1); + await sleepInMs(1_000 + 1_000); try { await settings.refresh(); } catch (e) { /* empty */ } @@ -330,7 +363,12 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -348,7 +386,12 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -366,7 +409,12 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -384,7 +432,12 @@ describe("request tracing", function () { (global as any).importScripts = function importScripts() { }; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -402,7 +455,12 @@ describe("request tracing", function () { (global as any).importScripts = undefined; try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -440,7 +498,12 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -455,7 +518,12 @@ describe("request tracing", function () { (global as any).document = undefined; // not an instance of Document try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -470,7 +538,12 @@ describe("request tracing", function () { (global as any).document = {}; // Not an instance of Document try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -485,7 +558,12 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -500,7 +578,12 @@ describe("request tracing", function () { (global as any).document = new (global as any).Document(); try { - await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + await load(createMockedConnectionString(fakeEndpoint), { + clientOptions, + startupOptions: { + timeoutInMs: 1 + } + }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); diff --git a/test/startup.test.ts b/test/startup.test.ts index a89bd993..35e9271d 100644 --- a/test/startup.test.ts +++ b/test/startup.test.ts @@ -15,16 +15,11 @@ describe("startup", function () { restoreMocks(); }); - it("should throw exception when timeout", async () => { - expect(load(createMockedConnectionString(), {startupOptions: {timeoutInMs: 1}})) - .eventually.rejectedWith("Load operation timed out."); - }); - - it("should retry for load operation when retryEnabled is true", async () => { + it("should retry for load operation before timeout", async () => { let attempt = 0; const failForInitialAttempt = () => { - if (attempt < 1) { - attempt += 1; + attempt += 1; + if (attempt <= 1) { throw new Error("Test Error"); } }; @@ -33,7 +28,44 @@ describe("startup", function () { failForInitialAttempt); const settings = await load(createMockedConnectionString()); + expect(attempt).eq(2); expect(settings).not.undefined; expect(settings.get("TestKey")).eq("TestValue"); }); + + it("should not retry for load operation after timeout", async () => { + let attempt = 0; + const failForAllAttempts = () => { + attempt += 1; + throw new Error("Test Error"); + }; + mockAppConfigurationClientListConfigurationSettings( + [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], + failForAllAttempts); + + await expect(load(createMockedConnectionString(), { + startupOptions: { + timeoutInMs: 5_000 + } + })).to.be.rejectedWith("Load operation timed out."); + expect(attempt).eq(1); + }); + + it("should not retry on non-retriable error", async () => { + let attempt = 0; + const failForAllAttempts = () => { + attempt += 1; + throw new RangeError("Non-retriable Test Error"); + }; + mockAppConfigurationClientListConfigurationSettings( + [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], + failForAllAttempts); + + await expect(load(createMockedConnectionString(), { + startupOptions: { + timeoutInMs: 10_000 + } + })).to.be.rejectedWith("Non-retriable Test Error"); + expect(attempt).eq(1); + }); }); From fc1aa5b04acf11e819fa43cb2c572fb78d5615a0 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 20 Feb 2025 04:19:59 +0800 Subject: [PATCH 08/47] update --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6e4df91a..04b4e526 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "dev": "rollup --config --watch", "lint": "eslint src/ test/", "fix-lint": "eslint src/ test/ --fix", - "test": "mocha out/test/*.test.{js,cjs,mjs}" + "test": "mocha out/test/*.test.{js,cjs,mjs} --parallel" }, "repository": { "type": "git", From 7de8a0db1406d6d84d28c15d4bd90e29c595814e Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 20 Feb 2025 13:19:18 +0800 Subject: [PATCH 09/47] update --- src/AzureAppConfigurationImpl.ts | 9 ++++----- src/ConfigurationClientWrapper.ts | 4 ++-- src/error.ts | 25 +++++++++++++------------ src/failover.ts | 9 ++++----- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index bbc3eab6..1813234e 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -41,8 +41,8 @@ import { RequestTracingOptions, getConfigurationSettingWithTrace, listConfigurat import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOptions.js"; import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; -import { getFixedBackoffDuration, calculateDynamicBackoffDuration } from "./failover.js"; -import { FailoverError, OperationError, isFailoverableError, isRetriableError } from "./error.js"; +import { getFixedBackoffDuration, calculateBackoffDuration } from "./failover.js"; +import { OperationError, isFailoverableError, isRetriableError } from "./error.js"; type PagedSettingSelector = SettingSelector & { /** @@ -357,7 +357,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#isInitialLoadCompleted = true; break; } catch (error) { - if (!isRetriableError(error)) { throw error; } @@ -369,7 +368,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { let backoffDuration = getFixedBackoffDuration(timeElapsed); if (backoffDuration === undefined) { postAttempts += 1; - backoffDuration = calculateDynamicBackoffDuration(postAttempts); + backoffDuration = calculateBackoffDuration(postAttempts); } await new Promise(resolve => setTimeout(resolve, backoffDuration)); console.warn("Failed to load configuration settings at startup. Retrying..."); @@ -697,7 +696,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } this.#clientManager.refreshClients(); - throw new FailoverError("All fallback clients failed to get configuration settings."); + throw new Error("All fallback clients failed to get configuration settings."); } async #processKeyValues(setting: ConfigurationSetting): Promise<[string, unknown]> { diff --git a/src/ConfigurationClientWrapper.ts b/src/ConfigurationClientWrapper.ts index 08b95fb7..1a3d2c85 100644 --- a/src/ConfigurationClientWrapper.ts +++ b/src/ConfigurationClientWrapper.ts @@ -2,7 +2,7 @@ // Licensed under the MIT license. import { AppConfigurationClient } from "@azure/app-configuration"; -import { calculateDynamicBackoffDuration } from "./failover.js"; +import { calculateBackoffDuration } from "./failover.js"; export class ConfigurationClientWrapper { endpoint: string; @@ -21,7 +21,7 @@ export class ConfigurationClientWrapper { this.backoffEndTime = Date.now(); } else { this.#failedAttempts += 1; - this.backoffEndTime = Date.now() + calculateDynamicBackoffDuration(this.#failedAttempts); + this.backoffEndTime = Date.now() + calculateBackoffDuration(this.#failedAttempts); } } } diff --git a/src/error.ts b/src/error.ts index 29409b7b..9e3e9085 100644 --- a/src/error.ts +++ b/src/error.ts @@ -13,20 +13,21 @@ export class OperationError extends Error { } } -/** - * Error thrown when fail to perform failover. - */ -export class FailoverError extends Error { - constructor(message: string) { - super(message); - this.name = "FailoverError"; - } -} - export function isFailoverableError(error: any): boolean { + if (!isRestError(error)) { + return false; + } // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory - return isRestError(error) && (error.code === "ENOTFOUND" || error.code === "ENOENT" || - (error.statusCode !== undefined && (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 408 || error.statusCode === 429 || error.statusCode >= 500))); + if (error.code == "ENOTFOUND" || error.code === "ENOENT"){ + return true; + } + // 401 Unauthorized, 403 Forbidden, 408 Request Timeout, 429 Too Many Requests, 5xx Server Errors + if (error.statusCode !== undefined && + (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 408 || error.statusCode === 429 || error.statusCode >= 500)) { + return true; + } + + return false; } export function isRetriableError(error: any): boolean { diff --git a/src/failover.ts b/src/failover.ts index 29a2c0f6..b1b03586 100644 --- a/src/failover.ts +++ b/src/failover.ts @@ -3,10 +3,9 @@ const MIN_BACKOFF_DURATION = 30_000; // 30 seconds in milliseconds const MAX_BACKOFF_DURATION = 10 * 60 * 1000; // 10 minutes in milliseconds -const MAX_SAFE_EXPONENTIAL = 30; // Used to avoid overflow. bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. const JITTER_RATIO = 0.25; -// Reference: https://github.com/Azure/AppConfiguration-DotnetProvider/blob/main/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/TimeSpanExtensions.cs#L14 +// The backoff duration algorithm is consistent with the .NET configuration provider's implementation. export function getFixedBackoffDuration(timeElapsed: number): number | undefined { if (timeElapsed <= 100_000) { // 100 seconds in milliseconds return 5_000; // 5 seconds in milliseconds @@ -20,14 +19,14 @@ export function getFixedBackoffDuration(timeElapsed: number): number | undefined return undefined; } -export function calculateDynamicBackoffDuration(failedAttempts: number) { +export function calculateBackoffDuration(failedAttempts: number) { if (failedAttempts <= 1) { return MIN_BACKOFF_DURATION; } // exponential: minBackoff * 2 ^ (failedAttempts - 1) - const exponential = Math.min(failedAttempts - 1, MAX_SAFE_EXPONENTIAL); - let calculatedBackoffDuration = MIN_BACKOFF_DURATION * (1 << exponential); + // The right shift operator is not used in order to avoid potential overflow. Bitwise operations in JavaScript are limited to 32 bits. + let calculatedBackoffDuration = MIN_BACKOFF_DURATION * Math.pow(2, failedAttempts - 1); if (calculatedBackoffDuration > MAX_BACKOFF_DURATION) { calculatedBackoffDuration = MAX_BACKOFF_DURATION; } From 5d233995c258d54a94649b7e628fcb4f03b8a960 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 20 Feb 2025 13:32:14 +0800 Subject: [PATCH 10/47] fix lint --- src/error.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.ts b/src/error.ts index 9e3e9085..b16bcc93 100644 --- a/src/error.ts +++ b/src/error.ts @@ -18,7 +18,7 @@ export function isFailoverableError(error: any): boolean { return false; } // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory - if (error.code == "ENOTFOUND" || error.code === "ENOENT"){ + if (error.code == "ENOTFOUND" || error.code === "ENOENT") { return true; } // 401 Unauthorized, 403 Forbidden, 408 Request Timeout, 429 Too Many Requests, 5xx Server Errors From 233af51134be4e50a2ee1f607c5aa372a181bb1b Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 21 Feb 2025 01:59:36 +0800 Subject: [PATCH 11/47] handle keyvault error --- src/AzureAppConfiguration.ts | 3 +++ src/AzureAppConfigurationImpl.ts | 3 ++- src/ConfigurationClientManager.ts | 28 ++++---------------- src/common/utils.ts | 20 ++++++++++++++ src/error.ts | 8 +++--- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 23 +++++++++------- src/load.ts | 3 ++- test/keyvault.test.ts | 2 +- 8 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/AzureAppConfiguration.ts b/src/AzureAppConfiguration.ts index 3f2918be..dbe2ce48 100644 --- a/src/AzureAppConfiguration.ts +++ b/src/AzureAppConfiguration.ts @@ -3,6 +3,9 @@ import { Disposable } from "./common/disposable.js"; +/** + * Azure App Configuration provider. + */ export type AzureAppConfiguration = { /** * API to trigger refresh operation. diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1813234e..f381d13d 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -482,7 +482,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { await this.#updateWatchedKeyValuesEtag(loadedSettings); } - // process key-values, watched settings have higher priority + // adapt configuration settings to key-values for (const setting of loadedSettings) { const [key, value] = await this.#processKeyValues(setting); keyValues.push([key, value]); @@ -657,6 +657,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return response; } + // Only operations related to Azure App Configuration service should be executed with failover policy. async #executeWithFailoverPolicy(funcToExecute: (client: AppConfigurationClient) => Promise): Promise { let clientWrappers = await this.#clientManager.getClients(); if (this.#options?.loadBalancingEnabled && this.#lastSuccessfulEndpoint !== "" && clientWrappers.length > 1) { diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index f72435cc..dcb9ab33 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -7,8 +7,7 @@ import { TokenCredential } from "@azure/identity"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; import { isBrowser, isWebWorker } from "./requestTracing/utils.js"; import * as RequestTracing from "./requestTracing/constants.js"; -import { shuffleList } from "./common/utils.js"; -import { OperationError } from "./error.js"; +import { instanceOfTokenCredential, shuffleList, getValidUrl } from "./common/utils.js"; // Configuration client retry options const CLIENT_MAX_RETRIES = 2; @@ -82,7 +81,7 @@ export class ConfigurationClientManager { this.#credential = credential; staticClient = new AppConfigurationClient(this.endpoint.origin, this.#credential, this.#clientOptions); } else { - throw new OperationError("A connection string or an endpoint with credential must be specified to create a client."); + throw new RangeError("A connection string or an endpoint with credential must be specified to create a client."); } this.#staticClients = [new ConfigurationClientWrapper(this.endpoint.origin, staticClient)]; @@ -207,12 +206,12 @@ export class ConfigurationClientManager { }); index++; } - } catch (err) { - if (err.code === "ENOTFOUND") { + } catch (error) { + if (error.code === "ENOTFOUND") { // No more SRV records found, return results. return results; } else { - throw new Error(`Failed to lookup SRV records: ${err.message}`); + throw new Error(`Failed to lookup SRV records: ${error.message}`); } } @@ -279,20 +278,3 @@ function getClientOptions(options?: AzureAppConfigurationOptions): AppConfigurat } }); } - -function getValidUrl(endpoint: string): URL { - try { - return new URL(endpoint); - } catch (error) { - if (error.code === "ERR_INVALID_URL") { - throw new RangeError("Invalid endpoint URL.", { cause: error }); - } else { - throw error; - } - } -} - -export function instanceOfTokenCredential(obj: unknown) { - return obj && typeof obj === "object" && "getToken" in obj && typeof obj.getToken === "function"; -} - diff --git a/src/common/utils.ts b/src/common/utils.ts index 5150708c..4365a3b8 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -30,3 +30,23 @@ export function shuffleList(array: T[]): T[] { } return array; } + +export function getValidUrl(endpoint: string): URL { + try { + return new URL(endpoint); + } catch (error) { + if (error.code === "ERR_INVALID_URL") { + throw new RangeError("Invalid endpoint URL.", { cause: error }); + } else { + throw error; + } + } +} + +export function getUrlHost(url: string) { + return new URL(url).host; +} + +export function instanceOfTokenCredential(obj: unknown) { + return obj && typeof obj === "object" && "getToken" in obj && typeof obj.getToken === "function"; +} \ No newline at end of file diff --git a/src/error.ts b/src/error.ts index b16bcc93..cf3572b8 100644 --- a/src/error.ts +++ b/src/error.ts @@ -2,9 +2,10 @@ // Licensed under the MIT license. import { isRestError } from "@azure/core-rest-pipeline"; +import { AuthenticationError } from "@azure/identity"; /** - * Error thrown when an operation is not allowed to be performed. + * Error thrown when an operation cannot be performed by the Azure App Configuration provider. */ export class OperationError extends Error { constructor(message: string) { @@ -31,8 +32,9 @@ export function isFailoverableError(error: any): boolean { } export function isRetriableError(error: any): boolean { - if (error instanceof OperationError || - error instanceof RangeError) { + if (error instanceof AuthenticationError || // this error occurs when using wrong credential to access the key vault + error instanceof RangeError || // this error is caused by misconfiguration of the Azure App Configuration provider + error instanceof OperationError) { return false; } return true; diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 6f5ce301..ff60e330 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -4,8 +4,8 @@ import { ConfigurationSetting, isSecretReference, parseSecretReference } from "@azure/app-configuration"; import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; +import { getUrlHost } from "../common/utils.js"; import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; -import { OperationError } from "../error.js"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { /** @@ -25,7 +25,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { async processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> { // TODO: cache results to save requests. if (!this.#keyVaultOptions) { - throw new OperationError("Failed to process the key vault reference. The keyVaultOptions is not configured."); + throw new RangeError("Failed to process the key vault reference. The keyVaultOptions is not configured."); } // precedence: secret clients > credential > secret resolver @@ -35,7 +35,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { const client = this.#getSecretClient(new URL(vaultUrl)); if (client) { - // TODO: what if error occurs when reading a key vault value? Now it breaks the whole load. + // If the credential of the secret client is wrong, AuthenticationError will be thrown. const secret = await client.getSecret(secretName, { version }); return [setting.key, secret.value]; } @@ -44,14 +44,21 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; } - throw new OperationError("Failed to process the key vault reference. No key vault credential or secret resolver callback configured, and no matching secret client could be found."); + // When code reaches here, it means the key vault secret reference is not resolved. + + throw new RangeError("Failed to process the key vault reference. No key vault credential or secret resolver callback is configured."); } + /** + * + * @param vaultUrl - The url of the key vault. + * @returns + */ #getSecretClient(vaultUrl: URL): SecretClient | undefined { if (this.#secretClients === undefined) { this.#secretClients = new Map(); - for (const c of this.#keyVaultOptions?.secretClients ?? []) { - this.#secretClients.set(getHost(c.vaultUrl), c); + for (const client of this.#keyVaultOptions?.secretClients ?? []) { + this.#secretClients.set(getUrlHost(client.vaultUrl), client); } } @@ -70,7 +77,3 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { return undefined; } } - -function getHost(url: string) { - return new URL(url).host; -} diff --git a/src/load.ts b/src/load.ts index 44439612..2046b064 100644 --- a/src/load.ts +++ b/src/load.ts @@ -5,7 +5,8 @@ import { TokenCredential } from "@azure/identity"; import { AzureAppConfiguration } from "./AzureAppConfiguration.js"; import { AzureAppConfigurationImpl } from "./AzureAppConfigurationImpl.js"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; -import { ConfigurationClientManager, instanceOfTokenCredential } from "./ConfigurationClientManager.js"; +import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; +import { instanceOfTokenCredential } from "./common/utils.js"; const MIN_DELAY_FOR_UNHANDLED_ERROR: number = 5_000; // 5 seconds diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 0aded3a8..c24d7007 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -96,7 +96,7 @@ describe("key vault reference", function () { ] } }); - return expect(loadKeyVaultPromise).eventually.rejectedWith("Failed to process the key vault reference. No key vault credential or secret resolver callback configured, and no matching secret client could be found."); + return expect(loadKeyVaultPromise).eventually.rejectedWith("Failed to process the key vault reference. No key vault credential or secret resolver callback is configured."); }); it("should fallback to use default credential when corresponding secret client not provided", async () => { From a0e07924d486b3978986b9b62f0a9c6cec55e9bc Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 21 Feb 2025 02:45:23 +0800 Subject: [PATCH 12/47] update --- rollup.config.mjs | 10 ++- src/AzureAppConfigurationImpl.ts | 2 +- src/AzureAppConfigurationOptions.ts | 2 +- src/StartupOptions.ts | 2 +- src/common/utils.ts | 2 +- src/featureManagement/FeatureFlagOptions.ts | 2 +- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 4 +- .../refreshOptions.ts} | 88 +++++++++---------- test/load.test.ts | 2 +- 9 files changed, 61 insertions(+), 53 deletions(-) rename src/{RefreshOptions.ts => refresh/refreshOptions.ts} (94%) diff --git a/rollup.config.mjs b/rollup.config.mjs index 1fa9626f..6f78ca44 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -4,7 +4,15 @@ import dts from "rollup-plugin-dts"; export default [ { - external: ["@azure/app-configuration", "@azure/keyvault-secrets", "@azure/core-rest-pipeline", "crypto", "dns/promises", "@microsoft/feature-management"], + external: [ + "@azure/app-configuration", + "@azure/keyvault-secrets", + "@azure/core-rest-pipeline", + "@azure/identity", + "crypto", + "dns/promises", + "@microsoft/feature-management" + ], input: "src/index.ts", output: [ { diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index f381d13d..23ec60f6 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -8,7 +8,7 @@ import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js" import { IKeyValueAdapter } from "./IKeyValueAdapter.js"; import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter.js"; import { DEFAULT_STARTUP_TIMEOUT_IN_MS } from "./StartupOptions.js"; -import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./RefreshOptions.js"; +import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./refresh/refreshOptions.js"; import { Disposable } from "./common/disposable.js"; import { base64Helper, jsonSorter } from "./common/utils.js"; import { diff --git a/src/AzureAppConfigurationOptions.ts b/src/AzureAppConfigurationOptions.ts index 5dc267af..dcf27765 100644 --- a/src/AzureAppConfigurationOptions.ts +++ b/src/AzureAppConfigurationOptions.ts @@ -3,7 +3,7 @@ import { AppConfigurationClientOptions } from "@azure/app-configuration"; import { KeyVaultOptions } from "./keyvault/KeyVaultOptions.js"; -import { RefreshOptions } from "./RefreshOptions.js"; +import { RefreshOptions } from "./refresh/refreshOptions.js"; import { SettingSelector } from "./types.js"; import { FeatureFlagOptions } from "./featureManagement/FeatureFlagOptions.js"; import { StartupOptions } from "./StartupOptions.js"; diff --git a/src/StartupOptions.ts b/src/StartupOptions.ts index 78ce048a..f80644bb 100644 --- a/src/StartupOptions.ts +++ b/src/StartupOptions.ts @@ -8,7 +8,7 @@ export interface StartupOptions { * The amount of time allowed to load data from Azure App Configuration on startup. * * @remarks - * If not specified, the default value is 100 seconds. Must be greater than 1 second. + * If not specified, the default value is 100 seconds. */ timeoutInMs?: number; } diff --git a/src/common/utils.ts b/src/common/utils.ts index 4365a3b8..e1180e9a 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -49,4 +49,4 @@ export function getUrlHost(url: string) { export function instanceOfTokenCredential(obj: unknown) { return obj && typeof obj === "object" && "getToken" in obj && typeof obj.getToken === "function"; -} \ No newline at end of file +} diff --git a/src/featureManagement/FeatureFlagOptions.ts b/src/featureManagement/FeatureFlagOptions.ts index 55ceda4d..6814dbf3 100644 --- a/src/featureManagement/FeatureFlagOptions.ts +++ b/src/featureManagement/FeatureFlagOptions.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { FeatureFlagRefreshOptions } from "../RefreshOptions.js"; +import { FeatureFlagRefreshOptions } from "../refresh/refreshOptions.js"; import { SettingSelector } from "../types.js"; /** diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index ff60e330..6dc1486b 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -50,9 +50,9 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } /** - * + * * @param vaultUrl - The url of the key vault. - * @returns + * @returns */ #getSecretClient(vaultUrl: URL): SecretClient | undefined { if (this.#secretClients === undefined) { diff --git a/src/RefreshOptions.ts b/src/refresh/refreshOptions.ts similarity index 94% rename from src/RefreshOptions.ts rename to src/refresh/refreshOptions.ts index d5e4da5f..202c7340 100644 --- a/src/RefreshOptions.ts +++ b/src/refresh/refreshOptions.ts @@ -1,44 +1,44 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -import { WatchedSetting } from "./WatchedSetting.js"; - -export const DEFAULT_REFRESH_INTERVAL_IN_MS = 30 * 1000; -export const MIN_REFRESH_INTERVAL_IN_MS = 1 * 1000; - -export interface RefreshOptions { - /** - * Specifies whether the provider should automatically refresh when the configuration is changed. - */ - enabled: boolean; - - /** - * Specifies the minimum time that must elapse before checking the server for any new changes. - * Default value is 30 seconds. Must be greater than 1 second. - * Any refresh operation triggered will not update the value for a key until after the interval. - */ - refreshIntervalInMs?: number; - - /** - * One or more configuration settings to be watched for changes on the server. - * Any modifications to watched settings will refresh all settings loaded by the configuration provider when refresh() is called. - * - * @remarks - * If no watched setting is specified, all configuration settings will be watched. - */ - watchedSettings?: WatchedSetting[]; -} - -export interface FeatureFlagRefreshOptions { - /** - * Specifies whether the provider should automatically refresh all feature flags if any feature flag changes. - */ - enabled: boolean; - - /** - * Specifies the minimum time that must elapse before checking the server for any new changes. - * Default value is 30 seconds. Must be greater than 1 second. - * Any refresh operation triggered will not update the value for a key until after the interval. - */ - refreshIntervalInMs?: number; -} +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { WatchedSetting } from "../WatchedSetting.js"; + +export const DEFAULT_REFRESH_INTERVAL_IN_MS = 30 * 1000; +export const MIN_REFRESH_INTERVAL_IN_MS = 1 * 1000; + +export interface RefreshOptions { + /** + * Specifies whether the provider should automatically refresh when the configuration is changed. + */ + enabled: boolean; + + /** + * Specifies the minimum time that must elapse before checking the server for any new changes. + * Default value is 30 seconds. Must be greater than 1 second. + * Any refresh operation triggered will not update the value for a key until after the interval. + */ + refreshIntervalInMs?: number; + + /** + * One or more configuration settings to be watched for changes on the server. + * Any modifications to watched settings will refresh all settings loaded by the configuration provider when refresh() is called. + * + * @remarks + * If no watched setting is specified, all configuration settings will be watched. + */ + watchedSettings?: WatchedSetting[]; +} + +export interface FeatureFlagRefreshOptions { + /** + * Specifies whether the provider should automatically refresh all feature flags if any feature flag changes. + */ + enabled: boolean; + + /** + * Specifies the minimum time that must elapse before checking the server for any new changes. + * Default value is 30 seconds. Must be greater than 1 second. + * Any refresh operation triggered will not update the value for a key until after the interval. + */ + refreshIntervalInMs?: number; +} diff --git a/test/load.test.ts b/test/load.test.ts index d36a3311..d81164e7 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -359,7 +359,7 @@ describe("load", function () { * When constructConfigurationObject() is called, it first constructs from key "app5.settings.fontColor" and then from key "app5.settings". * An error will be thrown when constructing from key "app5.settings" because there is ambiguity between the two keys. */ - it("Edge case 1: Hierarchical key-value pairs with overlapped key prefix.", async () => { + it("Edge case 2: Hierarchical key-value pairs with overlapped key prefix.", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { selectors: [{ From 9e32db4ec1ed26d33783237495e0e06bd140fe99 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 21 Feb 2025 17:04:21 +0800 Subject: [PATCH 13/47] update --- src/AzureAppConfigurationImpl.ts | 18 ++++----- src/ConfigurationClientManager.ts | 11 +++--- src/common/utils.ts | 10 ++--- src/error.ts | 16 +++++++- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 5 ++- src/refresh/RefreshTimer.ts | 4 +- test/load.test.ts | 2 +- test/startup.test.ts | 39 +++++++++++++++++++- 8 files changed, 77 insertions(+), 28 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 23ec60f6..b3535b34 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -42,7 +42,7 @@ import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOp import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; import { getFixedBackoffDuration, calculateBackoffDuration } from "./failover.js"; -import { OperationError, isFailoverableError, isRetriableError } from "./error.js"; +import { OperationError, ArgumentError, isFailoverableError, isRetriableError } from "./error.js"; type PagedSettingSelector = SettingSelector & { /** @@ -126,10 +126,10 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } else { for (const setting of watchedSettings) { if (setting.key.includes("*") || setting.key.includes(",")) { - throw new RangeError("The characters '*' and ',' are not supported in key of watched settings."); + throw new ArgumentError("The characters '*' and ',' are not supported in key of watched settings."); } if (setting.label?.includes("*") || setting.label?.includes(",")) { - throw new RangeError("The characters '*' and ',' are not supported in label of watched settings."); + throw new ArgumentError("The characters '*' and ',' are not supported in label of watched settings."); } this.#sentinels.push(setting); } @@ -138,7 +138,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { - throw new RangeError(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new ArgumentError(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { this.#kvRefreshInterval = refreshIntervalInMs; } @@ -156,7 +156,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { - throw new RangeError(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new ArgumentError(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { this.#ffRefreshInterval = refreshIntervalInMs; } @@ -265,7 +265,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const separator = options?.separator ?? "."; const validSeparators = [".", ",", ";", "-", "_", "__", "/", ":"]; if (!validSeparators.includes(separator)) { - throw new RangeError(`Invalid separator '${separator}'. Supported values: ${validSeparators.map(s => `'${s}'`).join(", ")}.`); + throw new ArgumentError(`Invalid separator '${separator}'. Supported values: ${validSeparators.map(s => `'${s}'`).join(", ")}.`); } // construct hierarchical data object from map @@ -729,7 +729,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #parseFeatureFlag(setting: ConfigurationSetting): Promise { const rawFlag = setting.value; if (rawFlag === undefined) { - throw new RangeError("The value of configuration setting cannot be undefined."); + throw new ArgumentError("The value of configuration setting cannot be undefined."); } const featureFlag = JSON.parse(rawFlag); @@ -953,13 +953,13 @@ function getValidSelectors(selectors: SettingSelector[]): SettingSelector[] { return uniqueSelectors.map(selectorCandidate => { const selector = { ...selectorCandidate }; if (!selector.keyFilter) { - throw new RangeError("Key filter cannot be null or empty."); + throw new ArgumentError("Key filter cannot be null or empty."); } if (!selector.labelFilter) { selector.labelFilter = LabelFilter.Null; } if (selector.labelFilter.includes("*") || selector.labelFilter.includes(",")) { - throw new RangeError("The characters '*' and ',' are not supported in label filters."); + throw new ArgumentError("The characters '*' and ',' are not supported in label filters."); } return selector; }); diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index dcb9ab33..9cc8784d 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -7,7 +7,8 @@ import { TokenCredential } from "@azure/identity"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; import { isBrowser, isWebWorker } from "./requestTracing/utils.js"; import * as RequestTracing from "./requestTracing/constants.js"; -import { instanceOfTokenCredential, shuffleList, getValidUrl } from "./common/utils.js"; +import { instanceOfTokenCredential, shuffleList, getEndpointUrl } from "./common/utils.js"; +import { ArgumentError } from "./error.js"; // Configuration client retry options const CLIENT_MAX_RETRIES = 2; @@ -60,18 +61,18 @@ export class ConfigurationClientManager { const regexMatch = connectionString.match(ConnectionStringRegex); if (regexMatch) { const endpointFromConnectionStr = regexMatch[1]; - this.endpoint = getValidUrl(endpointFromConnectionStr); + this.endpoint = getEndpointUrl(endpointFromConnectionStr); this.#id = regexMatch[2]; this.#secret = regexMatch[3]; } else { - throw new RangeError(`Invalid connection string. Valid connection strings should match the regex '${ConnectionStringRegex.source}'.`); + throw new ArgumentError(`Invalid connection string. Valid connection strings should match the regex '${ConnectionStringRegex.source}'.`); } staticClient = new AppConfigurationClient(connectionString, this.#clientOptions); } else if ((connectionStringOrEndpoint instanceof URL || typeof connectionStringOrEndpoint === "string") && credentialPassed) { let endpoint = connectionStringOrEndpoint; // ensure string is a valid URL. if (typeof endpoint === "string") { - endpoint = getValidUrl(endpoint); + endpoint = getEndpointUrl(endpoint); } const credential = credentialOrOptions as TokenCredential; @@ -81,7 +82,7 @@ export class ConfigurationClientManager { this.#credential = credential; staticClient = new AppConfigurationClient(this.endpoint.origin, this.#credential, this.#clientOptions); } else { - throw new RangeError("A connection string or an endpoint with credential must be specified to create a client."); + throw new ArgumentError("A connection string or an endpoint with credential must be specified to create a client."); } this.#staticClients = [new ConfigurationClientWrapper(this.endpoint.origin, staticClient)]; diff --git a/src/common/utils.ts b/src/common/utils.ts index e1180e9a..591181ac 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -31,19 +31,15 @@ export function shuffleList(array: T[]): T[] { return array; } -export function getValidUrl(endpoint: string): URL { +export function getEndpointUrl(endpoint: string): URL { try { return new URL(endpoint); } catch (error) { - if (error.code === "ERR_INVALID_URL") { - throw new RangeError("Invalid endpoint URL.", { cause: error }); - } else { - throw error; - } + throw new TypeError(`Invalid Endpoint URL: ${endpoint}`); } } -export function getUrlHost(url: string) { +export function getUrlHost(url: string): string { return new URL(url).host; } diff --git a/src/error.ts b/src/error.ts index cf3572b8..cd054b4b 100644 --- a/src/error.ts +++ b/src/error.ts @@ -14,6 +14,16 @@ export class OperationError extends Error { } } +/** + * Error thrown when an argument or configuration is invalid. + */ +export class ArgumentError extends Error { + constructor(message: string) { + super(message); + this.name = "ArgumentError"; + } +} + export function isFailoverableError(error: any): boolean { if (!isRestError(error)) { return false; @@ -33,8 +43,10 @@ export function isFailoverableError(error: any): boolean { export function isRetriableError(error: any): boolean { if (error instanceof AuthenticationError || // this error occurs when using wrong credential to access the key vault - error instanceof RangeError || // this error is caused by misconfiguration of the Azure App Configuration provider - error instanceof OperationError) { + error instanceof ArgumentError || // this error is caused by misconfiguration of the Azure App Configuration provider + error instanceof OperationError || + error instanceof TypeError || + error instanceof RangeError) { return false; } return true; diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 6dc1486b..1f81df4d 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -5,6 +5,7 @@ import { ConfigurationSetting, isSecretReference, parseSecretReference } from "@ import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; import { getUrlHost } from "../common/utils.js"; +import { ArgumentError } from "../error.js"; import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { @@ -25,7 +26,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { async processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> { // TODO: cache results to save requests. if (!this.#keyVaultOptions) { - throw new RangeError("Failed to process the key vault reference. The keyVaultOptions is not configured."); + throw new ArgumentError("Failed to process the key vault reference. The keyVaultOptions is not configured."); } // precedence: secret clients > credential > secret resolver @@ -46,7 +47,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { // When code reaches here, it means the key vault secret reference is not resolved. - throw new RangeError("Failed to process the key vault reference. No key vault credential or secret resolver callback is configured."); + throw new ArgumentError("Failed to process the key vault reference. No key vault credential or secret resolver callback is configured."); } /** diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 044ee4e4..e4e79655 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -1,13 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import { ArgumentError } from "../error.js"; + export class RefreshTimer { #backoffEnd: number; // Timestamp #interval: number; constructor(interval: number) { if (interval <= 0) { - throw new RangeError(`Refresh interval must be greater than 0. Given: ${this.#interval}`); + throw new ArgumentError(`Refresh interval must be greater than 0. Given: ${this.#interval}`); } this.#interval = interval; diff --git a/test/load.test.ts b/test/load.test.ts index d81164e7..20dd9d46 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -119,7 +119,7 @@ describe("load", function () { it("should throw error given invalid endpoint URL", async () => { const credential = createMockedTokenCredential(); - return expect(load("invalid-endpoint-url", credential)).eventually.rejectedWith("Invalid endpoint URL."); + return expect(load("invalid-endpoint-url", credential)).eventually.rejectedWith("Invalid Endpoint URL: invalid-endpoint-url"); }); it("should not include feature flags directly in the settings", async () => { diff --git a/test/startup.test.ts b/test/startup.test.ts index 35e9271d..eb1b7296 100644 --- a/test/startup.test.ts +++ b/test/startup.test.ts @@ -7,6 +7,7 @@ chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi"; import { MAX_TIME_OUT, createMockedConnectionString, createMockedKeyValue, mockAppConfigurationClientListConfigurationSettings, restoreMocks } from "./utils/testHelper.js"; +import { AuthenticationError } from "@azure/identity"; describe("startup", function () { this.timeout(MAX_TIME_OUT); @@ -51,7 +52,25 @@ describe("startup", function () { expect(attempt).eq(1); }); - it("should not retry on non-retriable error", async () => { + it("should not retry on non-retriable TypeError", async () => { + let attempt = 0; + const failForAllAttempts = () => { + attempt += 1; + throw new TypeError("Non-retriable Test Error"); + }; + mockAppConfigurationClientListConfigurationSettings( + [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], + failForAllAttempts); + + await expect(load(createMockedConnectionString(), { + startupOptions: { + timeoutInMs: 10_000 + } + })).to.be.rejectedWith("Non-retriable Test Error"); + expect(attempt).eq(1); + }); + + it("should not retry on non-retriable RangeError", async () => { let attempt = 0; const failForAllAttempts = () => { attempt += 1; @@ -68,4 +87,22 @@ describe("startup", function () { })).to.be.rejectedWith("Non-retriable Test Error"); expect(attempt).eq(1); }); + + it("should not retry on non-retriable AuthenticationError", async () => { + let attempt = 0; + const failForAllAttempts = () => { + attempt += 1; + throw new AuthenticationError(400, "Test Error"); + }; + mockAppConfigurationClientListConfigurationSettings( + [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], + failForAllAttempts); + + await expect(load(createMockedConnectionString(), { + startupOptions: { + timeoutInMs: 10_000 + } + })).to.be.rejectedWith("authority_not_found"); + expect(attempt).eq(1); + }); }); From 3a337380e233a524e15a65c584e90fc69c4cc393 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 14:39:34 +0800 Subject: [PATCH 14/47] update --- src/AzureAppConfigurationImpl.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index b3535b34..8c41ce8c 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -237,9 +237,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const abortSignal = abortController.signal; let timeoutId; try { - // Promise.race will be settled when the first promise in the list is settled + // Promise.race will be settled when the first promise in the list is settled. // It will not cancel the remaining promises in the list. - // To avoid memory leaks, we need to cancel other promises when one promise is settled. + // To avoid memory leaks, we must ensure other promises will be eventually terminated. await Promise.race([ this.#initializeWithRetryPolicy(abortSignal), // this promise will be rejected after timeout @@ -353,7 +353,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (this.#featureFlagEnabled) { await this.#loadFeatureFlags(); } - // Mark all settings have loaded at startup. this.#isInitialLoadCompleted = true; break; } catch (error) { @@ -657,7 +656,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return response; } - // Only operations related to Azure App Configuration service should be executed with failover policy. + // Only operations related to Azure App Configuration should be executed with failover policy. async #executeWithFailoverPolicy(funcToExecute: (client: AppConfigurationClient) => Promise): Promise { let clientWrappers = await this.#clientManager.getClients(); if (this.#options?.loadBalancingEnabled && this.#lastSuccessfulEndpoint !== "" && clientWrappers.length > 1) { From c637682dec928e06c4d1b12e88114017b104f68c Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 18:37:04 +0800 Subject: [PATCH 15/47] update --- src/AzureAppConfigurationImpl.ts | 4 ++-- src/refresh/RefreshTimer.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 8c41ce8c..d1827e7c 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -138,7 +138,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { - throw new ArgumentError(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new RangeError(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { this.#kvRefreshInterval = refreshIntervalInMs; } @@ -156,7 +156,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { - throw new ArgumentError(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new RangeError(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { this.#ffRefreshInterval = refreshIntervalInMs; } diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index e4e79655..17aa283e 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -9,7 +9,7 @@ export class RefreshTimer { constructor(interval: number) { if (interval <= 0) { - throw new ArgumentError(`Refresh interval must be greater than 0. Given: ${this.#interval}`); + throw new RangeError(`Refresh interval must be greater than 0. Given: ${this.#interval}`); } this.#interval = interval; From f079081a1ca8f51573d3f88f8ea75a185c33cf6f Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 18:53:45 +0800 Subject: [PATCH 16/47] add secretRefreshIntervalInMs to KeyVaultOptions --- src/keyvault/KeyVaultOptions.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/keyvault/KeyVaultOptions.ts b/src/keyvault/KeyVaultOptions.ts index 6d476b54..745a0072 100644 --- a/src/keyvault/KeyVaultOptions.ts +++ b/src/keyvault/KeyVaultOptions.ts @@ -18,10 +18,18 @@ export interface KeyVaultOptions { */ credential?: TokenCredential; + /** + * Specifies the refresh interval in milliseconds for periodically reloading secret from Key Vault. + * @remarks + * If specified, the value must be greater than 60 seconds. + * Any refresh operation triggered using {@see AzureAppConfiguration.refresh()} will not update the value for a Key Vault secret until the refresh interval has expired. + */ + secretRefreshIntervalInMs?: number; + /** * Specifies the callback used to resolve key vault references that have no applied SecretClient. * @param keyVaultReference The Key Vault reference to resolve. * @returns The secret value. */ secretResolver?: (keyVaultReference: URL) => string | Promise; -} +} From a9bcea45843a66a581302cc13cfc0284a7fa700c Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 19:06:25 +0800 Subject: [PATCH 17/47] update --- src/error.ts | 4 +--- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 2 +- src/refresh/RefreshTimer.ts | 2 -- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/error.ts b/src/error.ts index cd054b4b..84d53626 100644 --- a/src/error.ts +++ b/src/error.ts @@ -2,7 +2,6 @@ // Licensed under the MIT license. import { isRestError } from "@azure/core-rest-pipeline"; -import { AuthenticationError } from "@azure/identity"; /** * Error thrown when an operation cannot be performed by the Azure App Configuration provider. @@ -42,8 +41,7 @@ export function isFailoverableError(error: any): boolean { } export function isRetriableError(error: any): boolean { - if (error instanceof AuthenticationError || // this error occurs when using wrong credential to access the key vault - error instanceof ArgumentError || // this error is caused by misconfiguration of the Azure App Configuration provider + if (error instanceof ArgumentError || error instanceof OperationError || error instanceof TypeError || error instanceof RangeError) { diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 1f81df4d..cd078269 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -47,7 +47,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { // When code reaches here, it means the key vault secret reference is not resolved. - throw new ArgumentError("Failed to process the key vault reference. No key vault credential or secret resolver callback is configured."); + throw new ArgumentError("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is configured."); } /** diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 17aa283e..044ee4e4 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { ArgumentError } from "../error.js"; - export class RefreshTimer { #backoffEnd: number; // Timestamp #interval: number; From 00e2e6b13add1b166aa5279c39da6073ee61b9a7 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 19:08:04 +0800 Subject: [PATCH 18/47] update --- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index cd078269..8b981175 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -36,7 +36,6 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { const client = this.#getSecretClient(new URL(vaultUrl)); if (client) { - // If the credential of the secret client is wrong, AuthenticationError will be thrown. const secret = await client.getSecret(secretName, { version }); return [setting.key, secret.value]; } @@ -45,8 +44,6 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; } - // When code reaches here, it means the key vault secret reference is not resolved. - throw new ArgumentError("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is configured."); } From 1478e940f1a6d39dabd561eb838fc5458095909e Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 20:27:44 +0800 Subject: [PATCH 19/47] handle keyvault reference error --- src/AzureAppConfigurationImpl.ts | 2 +- src/error.ts | 10 +++++++ src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 29 ++++++++++++-------- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index d1827e7c..1f34aeaa 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -369,8 +369,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { postAttempts += 1; backoffDuration = calculateBackoffDuration(postAttempts); } + console.warn(`Failed to load. Error message: ${error.message}. It Will retry in ${backoffDuration} ms.`); await new Promise(resolve => setTimeout(resolve, backoffDuration)); - console.warn("Failed to load configuration settings at startup. Retrying..."); } } while (!abortSignal.aborted); } diff --git a/src/error.ts b/src/error.ts index 84d53626..a73096e1 100644 --- a/src/error.ts +++ b/src/error.ts @@ -23,6 +23,16 @@ export class ArgumentError extends Error { } } +/** + * Error thrown when it fails to get the secret from the Key Vault. + */ +export class KeyVaultReferenceError extends Error { + constructor(message: string) { + super(message); + this.name = "KeyVaultReferenceError"; + } +} + export function isFailoverableError(error: any): boolean { if (!isRestError(error)) { return false; diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 8b981175..301f641a 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -5,7 +5,7 @@ import { ConfigurationSetting, isSecretReference, parseSecretReference } from "@ import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; import { getUrlHost } from "../common/utils.js"; -import { ArgumentError } from "../error.js"; +import { ArgumentError, KeyVaultReferenceError } from "../error.js"; import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { @@ -29,21 +29,24 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { throw new ArgumentError("Failed to process the key vault reference. The keyVaultOptions is not configured."); } - // precedence: secret clients > credential > secret resolver const { name: secretName, vaultUrl, sourceId, version } = parseKeyVaultSecretIdentifier( parseSecretReference(setting).value.secretId ); - - const client = this.#getSecretClient(new URL(vaultUrl)); - if (client) { - const secret = await client.getSecret(secretName, { version }); - return [setting.key, secret.value]; - } - - if (this.#keyVaultOptions.secretResolver) { - return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; + try { + // precedence: secret clients > credential > secret resolver + const client = this.#getSecretClient(new URL(vaultUrl)); + if (client) { + const secret = await client.getSecret(secretName, { version }); + return [setting.key, secret.value]; + } + if (this.#keyVaultOptions.secretResolver) { + return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; + } + } catch (error) { + throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage(error.message, setting, sourceId)); } + // When code reaches here, it means that the key vault reference cannot be resolved in all possible ways. throw new ArgumentError("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is configured."); } @@ -75,3 +78,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { return undefined; } } + +function buildKeyVaultReferenceErrorMessage(message: string, setting: ConfigurationSetting, secretIdentifier?: string ): string { + return `${message} Key: ${setting.key} Label: ${setting.label ?? ""} ETag: ${setting.etag ?? ""} SecretIdentifier: ${secretIdentifier ?? ""}`; +} From 9b5013558865c3a53c4572bf9540431551503d32 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 21:01:07 +0800 Subject: [PATCH 20/47] update --- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 2 +- test/keyvault.test.ts | 2 +- test/startup.test.ts | 18 ------------------ 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 301f641a..58cd7571 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -47,7 +47,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } // When code reaches here, it means that the key vault reference cannot be resolved in all possible ways. - throw new ArgumentError("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is configured."); + throw new ArgumentError("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); } /** diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index c24d7007..4f5946f0 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -96,7 +96,7 @@ describe("key vault reference", function () { ] } }); - return expect(loadKeyVaultPromise).eventually.rejectedWith("Failed to process the key vault reference. No key vault credential or secret resolver callback is configured."); + return expect(loadKeyVaultPromise).eventually.rejectedWith("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); }); it("should fallback to use default credential when corresponding secret client not provided", async () => { diff --git a/test/startup.test.ts b/test/startup.test.ts index eb1b7296..8b885934 100644 --- a/test/startup.test.ts +++ b/test/startup.test.ts @@ -87,22 +87,4 @@ describe("startup", function () { })).to.be.rejectedWith("Non-retriable Test Error"); expect(attempt).eq(1); }); - - it("should not retry on non-retriable AuthenticationError", async () => { - let attempt = 0; - const failForAllAttempts = () => { - attempt += 1; - throw new AuthenticationError(400, "Test Error"); - }; - mockAppConfigurationClientListConfigurationSettings( - [[{key: "TestKey", value: "TestValue"}].map(createMockedKeyValue)], - failForAllAttempts); - - await expect(load(createMockedConnectionString(), { - startupOptions: { - timeoutInMs: 10_000 - } - })).to.be.rejectedWith("authority_not_found"); - expect(attempt).eq(1); - }); }); From 190f7b1458c495b77af14a6d97772ef06376e693 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 21:21:01 +0800 Subject: [PATCH 21/47] wip --- src/keyvault/KeyVaultOptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/keyvault/KeyVaultOptions.ts b/src/keyvault/KeyVaultOptions.ts index 745a0072..5dda7889 100644 --- a/src/keyvault/KeyVaultOptions.ts +++ b/src/keyvault/KeyVaultOptions.ts @@ -22,7 +22,7 @@ export interface KeyVaultOptions { * Specifies the refresh interval in milliseconds for periodically reloading secret from Key Vault. * @remarks * If specified, the value must be greater than 60 seconds. - * Any refresh operation triggered using {@see AzureAppConfiguration.refresh()} will not update the value for a Key Vault secret until the refresh interval has expired. + * Any refresh operation triggered using @see AzureAppConfiguration.refresh() will not update the value for a Key Vault secret until the refresh interval has expired. */ secretRefreshIntervalInMs?: number; From 39d9a3de10d7f52ddc8a9f3e9310d8ccf59cf4ae Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 21:22:30 +0800 Subject: [PATCH 22/47] fix lint --- test/startup.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/startup.test.ts b/test/startup.test.ts index 8b885934..8c2797d0 100644 --- a/test/startup.test.ts +++ b/test/startup.test.ts @@ -7,7 +7,6 @@ chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi"; import { MAX_TIME_OUT, createMockedConnectionString, createMockedKeyValue, mockAppConfigurationClientListConfigurationSettings, restoreMocks } from "./utils/testHelper.js"; -import { AuthenticationError } from "@azure/identity"; describe("startup", function () { this.timeout(MAX_TIME_OUT); From ddc4a50b81d29a00364c85145bdf59e22300172b Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 24 Feb 2025 00:33:22 +0800 Subject: [PATCH 23/47] add secret provider --- src/AzureAppConfigurationImpl.ts | 57 ++++++++++++++---- src/StartupOptions.ts | 2 +- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 61 ++++---------------- src/keyvault/AzureKeyVaultSecretProvider.ts | 61 ++++++++++++++++++++ src/keyvault/KeyVaultOptions.ts | 2 + src/refresh/RefreshTimer.ts | 10 ++-- src/refresh/refreshOptions.ts | 4 +- src/requestTracing/constants.ts | 1 + src/requestTracing/utils.ts | 6 +- test/keyvault.test.ts | 2 +- test/refresh.test.ts | 4 +- test/startup.test.ts | 1 - 12 files changed, 138 insertions(+), 73 deletions(-) create mode 100644 src/keyvault/AzureKeyVaultSecretProvider.ts diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1f34aeaa..cd66dab3 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions, featureFlagPrefix, isFeatureFlag } from "@azure/app-configuration"; +import { AppConfigurationClient, ConfigurationSetting, ConfigurationSettingId, GetConfigurationSettingOptions, GetConfigurationSettingResponse, ListConfigurationSettingsOptions, featureFlagPrefix, isFeatureFlag, isSecretReference } from "@azure/app-configuration"; import { isRestError } from "@azure/core-rest-pipeline"; import { AzureAppConfiguration, ConfigurationObjectConstructionOptions } from "./AzureAppConfiguration.js"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; @@ -9,6 +9,7 @@ import { IKeyValueAdapter } from "./IKeyValueAdapter.js"; import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter.js"; import { DEFAULT_STARTUP_TIMEOUT_IN_MS } from "./StartupOptions.js"; import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./refresh/refreshOptions.js"; +import { MIN_SECRET_REFRESH_INTERVAL_IN_MS } from "./keyvault/KeyVaultOptions.js"; import { Disposable } from "./common/disposable.js"; import { base64Helper, jsonSorter } from "./common/utils.js"; import { @@ -87,6 +88,10 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #ffRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #ffRefreshTimer: RefreshTimer; + // Key Vault references + #secretRefreshEnabled: boolean = false; + #secretReferences: ConfigurationSetting[] = []; // cached key vault references + #secretRefreshTimer: RefreshTimer; /** * Selectors of key-values obtained from @see AzureAppConfigurationOptions.selectors */ @@ -139,9 +144,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { throw new RangeError(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); - } else { - this.#kvRefreshInterval = refreshIntervalInMs; } + this.#kvRefreshInterval = refreshIntervalInMs; } this.#kvRefreshTimer = new RefreshTimer(this.#kvRefreshInterval); } @@ -157,16 +161,25 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { throw new RangeError(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); - } else { - this.#ffRefreshInterval = refreshIntervalInMs; } + this.#ffRefreshInterval = refreshIntervalInMs; } this.#ffRefreshTimer = new RefreshTimer(this.#ffRefreshInterval); } } - this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions)); + if (options?.keyVaultOptions) { + const { secretRefreshIntervalInMs } = options.keyVaultOptions; + if (secretRefreshIntervalInMs !== undefined) { + if (secretRefreshIntervalInMs < MIN_SECRET_REFRESH_INTERVAL_IN_MS) { + throw new RangeError(`The key vault secret refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + } + this.#secretRefreshEnabled = true; + this.#secretRefreshTimer = new RefreshTimer(secretRefreshIntervalInMs); + } + } + this.#adapters.push(new AzureKeyVaultKeyValueAdapter(options?.keyVaultOptions, this.#secretRefreshTimer)); this.#adapters.push(new JsonKeyValueAdapter()); } @@ -305,8 +318,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Refreshes the configuration. */ async refresh(): Promise { - if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new OperationError("Refresh is not enabled for key-values or feature flags."); + if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled && !this.#secretRefreshEnabled) { + throw new OperationError("Refresh is not enabled for key-values, key vault secrets or feature flags."); } if (this.#refreshInProgress) { @@ -324,8 +337,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Registers a callback function to be called when the configuration is refreshed. */ onRefresh(listener: () => any, thisArg?: any): Disposable { - if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new OperationError("Refresh is not enabled for key-values or feature flags."); + if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled && !this.#secretRefreshEnabled) { + throw new OperationError("Refresh is not enabled for key-values, key vault secrets or feature flags."); } const boundedListener = listener.bind(thisArg); @@ -399,6 +412,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (this.#featureFlagRefreshEnabled) { refreshTasks.push(this.#refreshFeatureFlags()); } + if (this.#secretRefreshEnabled) { + refreshTasks.push(this.#refreshSecrets()); + } // wait until all tasks are either resolved or rejected const results = await Promise.allSettled(refreshTasks); @@ -481,8 +497,12 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { await this.#updateWatchedKeyValuesEtag(loadedSettings); } - // adapt configuration settings to key-values + // clear all cached key vault references + this.#secretReferences = []; for (const setting of loadedSettings) { + if (this.#secretRefreshEnabled && isSecretReference(setting)) { + this.#secretReferences.push(setting); + } const [key, value] = await this.#processKeyValues(setting); keyValues.push([key, value]); } @@ -597,6 +617,21 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return Promise.resolve(needRefresh); } + async #refreshSecrets(): Promise { + // if still within refresh interval/backoff, return + if (!this.#secretRefreshTimer.canRefresh()) { + return Promise.resolve(false); + } + + for (const setting of this.#secretReferences) { + const [key, value] = await this.#processKeyValues(setting); + this.#configMap.set(key, value); + } + + this.#secretRefreshTimer.reset(); + return Promise.resolve(true); + } + /** * Checks whether the key-value collection has changed. * @param selectors - The @see PagedSettingSelector of the kev-value collection. diff --git a/src/StartupOptions.ts b/src/StartupOptions.ts index f80644bb..74c677db 100644 --- a/src/StartupOptions.ts +++ b/src/StartupOptions.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -export const DEFAULT_STARTUP_TIMEOUT_IN_MS = 100 * 1000; // 100 seconds in milliseconds +export const DEFAULT_STARTUP_TIMEOUT_IN_MS = 100_000; // 100 seconds in milliseconds export interface StartupOptions { /** diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 58cd7571..36a2f9fb 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -3,20 +3,19 @@ import { ConfigurationSetting, isSecretReference, parseSecretReference } from "@azure/app-configuration"; import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; +import { AzureKeyVaultSecretProvider } from "./AzureKeyVaultSecretProvider.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; -import { getUrlHost } from "../common/utils.js"; +import { RefreshTimer } from "../refresh/RefreshTimer.js"; import { ArgumentError, KeyVaultReferenceError } from "../error.js"; -import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; +import { parseKeyVaultSecretIdentifier, KeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { - /** - * Map vault hostname to corresponding secret client. - */ - #secretClients: Map; #keyVaultOptions: KeyVaultOptions | undefined; + #keyVaultSecretProvider: AzureKeyVaultSecretProvider; - constructor(keyVaultOptions: KeyVaultOptions | undefined) { + constructor(keyVaultOptions: KeyVaultOptions | undefined, refreshTimer?: RefreshTimer) { this.#keyVaultOptions = keyVaultOptions; + this.#keyVaultSecretProvider = new AzureKeyVaultSecretProvider(keyVaultOptions, refreshTimer); } canProcess(setting: ConfigurationSetting): boolean { @@ -24,58 +23,22 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } async processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> { - // TODO: cache results to save requests. if (!this.#keyVaultOptions) { throw new ArgumentError("Failed to process the key vault reference. The keyVaultOptions is not configured."); } - const { name: secretName, vaultUrl, sourceId, version } = parseKeyVaultSecretIdentifier( + const secretIdentifier: KeyVaultSecretIdentifier = parseKeyVaultSecretIdentifier( parseSecretReference(setting).value.secretId ); try { - // precedence: secret clients > credential > secret resolver - const client = this.#getSecretClient(new URL(vaultUrl)); - if (client) { - const secret = await client.getSecret(secretName, { version }); - return [setting.key, secret.value]; - } - if (this.#keyVaultOptions.secretResolver) { - return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; - } + const secretValue = await this.#keyVaultSecretProvider.getSecretValue(secretIdentifier); + return [setting.key, secretValue]; } catch (error) { - throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage(error.message, setting, sourceId)); - } - - // When code reaches here, it means that the key vault reference cannot be resolved in all possible ways. - throw new ArgumentError("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); - } - - /** - * - * @param vaultUrl - The url of the key vault. - * @returns - */ - #getSecretClient(vaultUrl: URL): SecretClient | undefined { - if (this.#secretClients === undefined) { - this.#secretClients = new Map(); - for (const client of this.#keyVaultOptions?.secretClients ?? []) { - this.#secretClients.set(getUrlHost(client.vaultUrl), client); + if (error instanceof ArgumentError) { + throw error; } + throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage(error.message, setting, secretIdentifier.sourceId)); } - - let client: SecretClient | undefined; - client = this.#secretClients.get(vaultUrl.host); - if (client !== undefined) { - return client; - } - - if (this.#keyVaultOptions?.credential) { - client = new SecretClient(vaultUrl.toString(), this.#keyVaultOptions.credential); - this.#secretClients.set(vaultUrl.host, client); - return client; - } - - return undefined; } } diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts new file mode 100644 index 00000000..70ada28c --- /dev/null +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { KeyVaultOptions } from "./KeyVaultOptions.js"; +import { RefreshTimer } from "../refresh/RefreshTimer.js"; +import { getUrlHost } from "../common/utils.js"; +import { ArgumentError } from "../error.js"; +import { SecretClient, KeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; + +export class AzureKeyVaultSecretProvider { + #keyVaultOptions: KeyVaultOptions | undefined; + #secretClients: Map; // map key vault hostname to corresponding secret client + + constructor(keyVaultOptions: KeyVaultOptions | undefined, refreshTimer?: RefreshTimer) { + if (keyVaultOptions?.secretRefreshIntervalInMs !== undefined) { + if (refreshTimer === undefined) { + throw new ArgumentError("Refresh timer must be specified when Key Vault secret refresh is enabled."); + } + if (refreshTimer.interval !== keyVaultOptions.secretRefreshIntervalInMs) { + throw new ArgumentError("Refresh timer does not match the secret refresh interval."); + } + } + this.#keyVaultOptions = keyVaultOptions; + this.#secretClients = new Map(); + for (const client of this.#keyVaultOptions?.secretClients ?? []) { + this.#secretClients.set(getUrlHost(client.vaultUrl), client); + } + } + + async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { + if (!this.#keyVaultOptions) { + throw new ArgumentError("Failed to get secret value. The keyVaultOptions is not configured."); + } + const { name: secretName, vaultUrl, sourceId, version } = secretIdentifier; + // precedence: secret clients > custom secret resolver + const client = this.#getSecretClient(new URL(vaultUrl)); + if (client) { + const secret = await client.getSecret(secretName, { version }); + return secret.value; + } + if (this.#keyVaultOptions.secretResolver) { + return await this.#keyVaultOptions.secretResolver(new URL(sourceId)); + } + // When code reaches here, it means that the key vault reference cannot be resolved in all possible ways. + throw new ArgumentError("Failed to get secret value. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); + } + + #getSecretClient(vaultUrl: URL): SecretClient | undefined { + let client = this.#secretClients.get(vaultUrl.host); + if (client !== undefined) { + return client; + } + if (this.#keyVaultOptions?.credential) { + client = new SecretClient(vaultUrl.toString(), this.#keyVaultOptions.credential); + this.#secretClients.set(vaultUrl.host, client); + return client; + } + + return undefined; + } +} diff --git a/src/keyvault/KeyVaultOptions.ts b/src/keyvault/KeyVaultOptions.ts index 5dda7889..294678fa 100644 --- a/src/keyvault/KeyVaultOptions.ts +++ b/src/keyvault/KeyVaultOptions.ts @@ -4,6 +4,8 @@ import { TokenCredential } from "@azure/identity"; import { SecretClient } from "@azure/keyvault-secrets"; +export const MIN_SECRET_REFRESH_INTERVAL_IN_MS = 60_000; + /** * Options used to resolve Key Vault references. */ diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 044ee4e4..6eeb0527 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -3,15 +3,15 @@ export class RefreshTimer { #backoffEnd: number; // Timestamp - #interval: number; + readonly interval: number; constructor(interval: number) { if (interval <= 0) { - throw new RangeError(`Refresh interval must be greater than 0. Given: ${this.#interval}`); + throw new RangeError(`Refresh interval must be greater than 0. Given: ${this.interval}`); } - this.#interval = interval; - this.#backoffEnd = Date.now() + this.#interval; + this.interval = interval; + this.#backoffEnd = Date.now() + this.interval; } canRefresh(): boolean { @@ -19,6 +19,6 @@ export class RefreshTimer { } reset(): void { - this.#backoffEnd = Date.now() + this.#interval; + this.#backoffEnd = Date.now() + this.interval; } } diff --git a/src/refresh/refreshOptions.ts b/src/refresh/refreshOptions.ts index 202c7340..9b82b6d9 100644 --- a/src/refresh/refreshOptions.ts +++ b/src/refresh/refreshOptions.ts @@ -3,8 +3,8 @@ import { WatchedSetting } from "../WatchedSetting.js"; -export const DEFAULT_REFRESH_INTERVAL_IN_MS = 30 * 1000; -export const MIN_REFRESH_INTERVAL_IN_MS = 1 * 1000; +export const DEFAULT_REFRESH_INTERVAL_IN_MS = 30_000; +export const MIN_REFRESH_INTERVAL_IN_MS = 1_000; export interface RefreshOptions { /** diff --git a/src/requestTracing/constants.ts b/src/requestTracing/constants.ts index 74ca58bb..bd91a02f 100644 --- a/src/requestTracing/constants.ts +++ b/src/requestTracing/constants.ts @@ -49,6 +49,7 @@ export const REPLICA_COUNT_KEY = "ReplicaCount"; // Tag names export const KEY_VAULT_CONFIGURED_TAG = "UsesKeyVault"; +export const KEY_VAULT_REFRESH_CONFIGURED_TAG = "RefreshesKeyVault"; export const FAILOVER_REQUEST_TAG = "Failover"; // Compact feature tags diff --git a/src/requestTracing/utils.ts b/src/requestTracing/utils.ts index 2e8b1124..35e7e7ad 100644 --- a/src/requestTracing/utils.ts +++ b/src/requestTracing/utils.ts @@ -17,6 +17,7 @@ import { HOST_TYPE_KEY, HostType, KEY_VAULT_CONFIGURED_TAG, + KEY_VAULT_REFRESH_CONFIGURED_TAG, KUBERNETES_ENV_VAR, NODEJS_DEV_ENV_VAL, NODEJS_ENV_VAR, @@ -100,10 +101,13 @@ export function createCorrelationContextHeader(requestTracingOptions: RequestTra const appConfigOptions = requestTracingOptions.appConfigOptions; if (appConfigOptions?.keyVaultOptions) { - const { credential, secretClients, secretResolver } = appConfigOptions.keyVaultOptions; + const { credential, secretClients, secretRefreshIntervalInMs, secretResolver } = appConfigOptions.keyVaultOptions; if (credential !== undefined || secretClients?.length || secretResolver !== undefined) { tags.push(KEY_VAULT_CONFIGURED_TAG); } + if (secretRefreshIntervalInMs !== undefined) { + tags.push(KEY_VAULT_REFRESH_CONFIGURED_TAG); + } } const featureFlagTracing = requestTracingOptions.featureFlagTracing; diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 4f5946f0..70d97f8c 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -96,7 +96,7 @@ describe("key vault reference", function () { ] } }); - return expect(loadKeyVaultPromise).eventually.rejectedWith("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); + return expect(loadKeyVaultPromise).eventually.rejectedWith("Failed to get secret value. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); }); it("should fallback to use default credential when corresponding secret client not provided", async () => { diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 6457cb1a..99245d9e 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -55,7 +55,7 @@ describe("dynamic refresh", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); const refreshCall = settings.refresh(); - return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values or feature flags."); + return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values, key vault secrets or feature flags."); }); it("should not allow refresh interval less than 1 second", async () => { @@ -117,7 +117,7 @@ describe("dynamic refresh", function () { it("should throw error when calling onRefresh when refresh is not enabled", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); - expect(() => settings.onRefresh(() => { })).throws("Refresh is not enabled for key-values or feature flags."); + expect(() => settings.onRefresh(() => { })).throws("Refresh is not enabled for key-values, key vault secrets or feature flags."); }); it("should only update values after refreshInterval", async () => { diff --git a/test/startup.test.ts b/test/startup.test.ts index 8b885934..8c2797d0 100644 --- a/test/startup.test.ts +++ b/test/startup.test.ts @@ -7,7 +7,6 @@ chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi"; import { MAX_TIME_OUT, createMockedConnectionString, createMockedKeyValue, mockAppConfigurationClientListConfigurationSettings, restoreMocks } from "./utils/testHelper.js"; -import { AuthenticationError } from "@azure/identity"; describe("startup", function () { this.timeout(MAX_TIME_OUT); From 540ec3a6d5635378cd5c607512e5883da2a822a0 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 24 Feb 2025 14:52:34 +0800 Subject: [PATCH 24/47] add testcases --- src/AzureAppConfigurationImpl.ts | 7 ++- src/IKeyValueAdapter.ts | 7 ++- src/JsonKeyValueAdapter.ts | 4 ++ src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 5 ++ src/keyvault/AzureKeyVaultSecretProvider.ts | 29 +++++++++- src/keyvault/KeyVaultOptions.ts | 1 - test/exportedApi.ts | 2 +- test/keyvault.test.ts | 61 +++++++++++++++++++- test/refresh.test.ts | 2 +- test/utils/testHelper.ts | 2 +- 10 files changed, 111 insertions(+), 9 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index cd66dab3..9fcebdd7 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -173,7 +173,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const { secretRefreshIntervalInMs } = options.keyVaultOptions; if (secretRefreshIntervalInMs !== undefined) { if (secretRefreshIntervalInMs < MIN_SECRET_REFRESH_INTERVAL_IN_MS) { - throw new RangeError(`The key vault secret refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new RangeError(`The key vault secret refresh interval cannot be less than ${MIN_SECRET_REFRESH_INTERVAL_IN_MS} milliseconds.`); } this.#secretRefreshEnabled = true; this.#secretRefreshTimer = new RefreshTimer(secretRefreshIntervalInMs); @@ -497,7 +497,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { await this.#updateWatchedKeyValuesEtag(loadedSettings); } - // clear all cached key vault references + // clear all cached key vault reference configuration settings this.#secretReferences = []; for (const setting of loadedSettings) { if (this.#secretRefreshEnabled && isSecretReference(setting)) { @@ -591,6 +591,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } if (needRefresh) { + for (const adapter of this.#adapters) { + await adapter.onChangeDetected(); + } await this.#loadSelectedAndWatchedKeyValues(); } diff --git a/src/IKeyValueAdapter.ts b/src/IKeyValueAdapter.ts index afd7bdc3..ab22c3ab 100644 --- a/src/IKeyValueAdapter.ts +++ b/src/IKeyValueAdapter.ts @@ -13,4 +13,9 @@ export interface IKeyValueAdapter { * This method process the original configuration setting, and returns processed key and value in an array. */ processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]>; -} + + /** + * This method is called when a change is detected in the configuration setting. + */ + onChangeDetected(setting?: ConfigurationSetting): void; +} diff --git a/src/JsonKeyValueAdapter.ts b/src/JsonKeyValueAdapter.ts index d9157a45..f22439a3 100644 --- a/src/JsonKeyValueAdapter.ts +++ b/src/JsonKeyValueAdapter.ts @@ -33,6 +33,10 @@ export class JsonKeyValueAdapter implements IKeyValueAdapter { } return [setting.key, parsedValue]; } + + async onChangeDetected(): Promise { + return; + } } // Determine whether a content type string is a valid JSON content type. diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 36a2f9fb..7cec7ad1 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -40,6 +40,11 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage(error.message, setting, secretIdentifier.sourceId)); } } + + async onChangeDetected(): Promise { + this.#keyVaultSecretProvider.clearCache(); + return; + } } function buildKeyVaultReferenceErrorMessage(message: string, setting: ConfigurationSetting, secretIdentifier?: string ): string { diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts index 70ada28c..39696e5e 100644 --- a/src/keyvault/AzureKeyVaultSecretProvider.ts +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -5,11 +5,13 @@ import { KeyVaultOptions } from "./KeyVaultOptions.js"; import { RefreshTimer } from "../refresh/RefreshTimer.js"; import { getUrlHost } from "../common/utils.js"; import { ArgumentError } from "../error.js"; -import { SecretClient, KeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; +import { SecretClient, KeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; export class AzureKeyVaultSecretProvider { #keyVaultOptions: KeyVaultOptions | undefined; + #refreshTimer: RefreshTimer | undefined; #secretClients: Map; // map key vault hostname to corresponding secret client + #cachedSecretValue: Map = new Map(); // map secret identifier to secret value constructor(keyVaultOptions: KeyVaultOptions | undefined, refreshTimer?: RefreshTimer) { if (keyVaultOptions?.secretRefreshIntervalInMs !== undefined) { @@ -21,6 +23,7 @@ export class AzureKeyVaultSecretProvider { } } this.#keyVaultOptions = keyVaultOptions; + this.#refreshTimer = refreshTimer; this.#secretClients = new Map(); for (const client of this.#keyVaultOptions?.secretClients ?? []) { this.#secretClients.set(getUrlHost(client.vaultUrl), client); @@ -28,6 +31,29 @@ export class AzureKeyVaultSecretProvider { } async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { + if (this.#refreshTimer && !this.#refreshTimer.canRefresh()) { + // return the cached secret value if it exists + if (this.#cachedSecretValue.has(secretIdentifier.sourceId)) { + const cachedValue = this.#cachedSecretValue.get(secretIdentifier.sourceId); + return cachedValue; + } + // not found in cache, get the secret value from key vault + const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); + this.#cachedSecretValue.set(secretIdentifier.sourceId, secretValue); + return secretValue; + } + + // Always reload the secret value from key vault when the refresh timer expires. + const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); + this.#cachedSecretValue.set(secretIdentifier.sourceId, secretValue); + return secretValue; + } + + clearCache(): void { + this.#cachedSecretValue.clear(); + } + + async #getSecretValueFromKeyVault(secretIdentifier: KeyVaultSecretIdentifier): Promise { if (!this.#keyVaultOptions) { throw new ArgumentError("Failed to get secret value. The keyVaultOptions is not configured."); } @@ -43,6 +69,7 @@ export class AzureKeyVaultSecretProvider { } // When code reaches here, it means that the key vault reference cannot be resolved in all possible ways. throw new ArgumentError("Failed to get secret value. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); + } #getSecretClient(vaultUrl: URL): SecretClient | undefined { diff --git a/src/keyvault/KeyVaultOptions.ts b/src/keyvault/KeyVaultOptions.ts index 294678fa..6dc4818f 100644 --- a/src/keyvault/KeyVaultOptions.ts +++ b/src/keyvault/KeyVaultOptions.ts @@ -24,7 +24,6 @@ export interface KeyVaultOptions { * Specifies the refresh interval in milliseconds for periodically reloading secret from Key Vault. * @remarks * If specified, the value must be greater than 60 seconds. - * Any refresh operation triggered using @see AzureAppConfiguration.refresh() will not update the value for a Key Vault secret until the refresh interval has expired. */ secretRefreshIntervalInMs?: number; diff --git a/test/exportedApi.ts b/test/exportedApi.ts index c49bc1a3..a285e471 100644 --- a/test/exportedApi.ts +++ b/test/exportedApi.ts @@ -1,4 +1,4 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -export { load } from "../src"; +export { load } from "../src"; diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 70d97f8c..7e9e50cb 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { MAX_TIME_OUT, sinon, createMockedConnectionString, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, mockSecretClientGetSecret, restoreMocks, createMockedKeyVaultReference } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, sinon, createMockedConnectionString, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, mockSecretClientGetSecret, restoreMocks, createMockedKeyVaultReference, sleepInMs } from "./utils/testHelper.js"; import { KeyVaultSecret, SecretClient } from "@azure/keyvault-secrets"; const mockedData = [ @@ -113,3 +113,62 @@ describe("key vault reference", function () { expect(settings.get("TestKey2")).eq("SecretValue2"); }); }); + +describe("key vault secret refresh", function () { + this.timeout(MAX_TIME_OUT); + + beforeEach(() => { + const data = [ + ["TestKey", "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName", "SecretValue"] + ]; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const kvs = data.map(([key, vaultUri, _value]) => createMockedKeyVaultReference(key, vaultUri)); + mockAppConfigurationClientListConfigurationSettings([kvs]); + }); + + afterEach(() => { + restoreMocks(); + }); + + it("should not allow secret refresh interval less than 1 minute", async () => { + const connectionString = createMockedConnectionString(); + const loadWithInvalidSecretRefreshInterval = load(connectionString, { + keyVaultOptions: { + secretClients: [ + new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()), + ], + secretRefreshIntervalInMs: 59999 + } + }); + return expect(loadWithInvalidSecretRefreshInterval).eventually.rejectedWith("The key vault secret refresh interval cannot be less than 60000 milliseconds."); + }); + + it("should reload key vault secret when there is no change to key-values", async () => { + const client = new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()); + const stub = sinon.stub(client, "getSecret"); + stub.onCall(0).resolves({ value: "SecretValue" } as KeyVaultSecret); + stub.onCall(1).resolves({ value: "SecretValue - Updated" } as KeyVaultSecret); + + const settings = await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [ + client + ], + credential: createMockedTokenCredential(), + secretRefreshIntervalInMs: 60_000 + } + }); + expect(settings).not.undefined; + expect(settings.get("TestKey")).eq("SecretValue"); + + await sleepInMs(30_000); + await settings.refresh(); + // use cached value + expect(settings.get("TestKey")).eq("SecretValue"); + + await sleepInMs(30_000); + await settings.refresh(); + // secret refresh interval expires, reload secret value + expect(settings.get("TestKey")).eq("SecretValue - Updated"); + }); +}); diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 99245d9e..ef170c26 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -438,7 +438,7 @@ describe("dynamic refresh", function () { }); describe("dynamic refresh feature flags", function () { - this.timeout(10000); + this.timeout(MAX_TIME_OUT); beforeEach(() => { }); diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 85f7ac80..37ecf745 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -13,7 +13,7 @@ import * as crypto from "crypto"; import { ConfigurationClientManager } from "../../src/ConfigurationClientManager.js"; import { ConfigurationClientWrapper } from "../../src/ConfigurationClientWrapper.js"; -const MAX_TIME_OUT = 20000; +const MAX_TIME_OUT = 100_000; const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; From 19de21811996a41fdd859a911ba4566b60439ab2 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 24 Feb 2025 15:07:17 +0800 Subject: [PATCH 25/47] update --- src/AzureAppConfigurationImpl.ts | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 9fcebdd7..55accd56 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -79,12 +79,15 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { /** * Aka watched settings. */ + #refreshEnabled: boolean = false; #sentinels: ConfigurationSettingId[] = []; #watchAll: boolean = false; #kvRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #kvRefreshTimer: RefreshTimer; // Feature flags + #featureFlagEnabled: boolean = false; + #featureFlagRefreshEnabled: boolean = false; #ffRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #ffRefreshTimer: RefreshTimer; @@ -117,14 +120,15 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#featureFlagTracing = new FeatureFlagTracingOptions(); } - if (options?.trimKeyPrefixes) { + if (options?.trimKeyPrefixes !== undefined) { this.#sortedTrimKeyPrefixes = [...options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); } // if no selector is specified, always load key values using the default selector: key="*" and label="\0" this.#kvSelectors = getValidKeyValueSelectors(options?.selectors); - if (options?.refreshOptions?.enabled) { + if (options?.refreshOptions?.enabled === true) { + this.#refreshEnabled = true; const { refreshIntervalInMs, watchedSettings } = options.refreshOptions; if (watchedSettings === undefined || watchedSettings.length === 0) { this.#watchAll = true; // if no watched settings is specified, then watch all @@ -151,11 +155,13 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } // feature flag options - if (options?.featureFlagOptions?.enabled) { + if (options?.featureFlagOptions?.enabled === true) { + this.#featureFlagEnabled = true; // validate feature flag selectors, only load feature flags when enabled this.#ffSelectors = getValidFeatureFlagSelectors(options.featureFlagOptions.selectors); - if (options.featureFlagOptions.refresh?.enabled) { + if (options.featureFlagOptions.refresh?.enabled === true) { + this.#featureFlagRefreshEnabled = true; const { refreshIntervalInMs } = options.featureFlagOptions.refresh; // custom refresh interval if (refreshIntervalInMs !== undefined) { @@ -169,7 +175,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } } - if (options?.keyVaultOptions) { + if (options?.keyVaultOptions !== undefined) { const { secretRefreshIntervalInMs } = options.keyVaultOptions; if (secretRefreshIntervalInMs !== undefined) { if (secretRefreshIntervalInMs < MIN_SECRET_REFRESH_INTERVAL_IN_MS) { @@ -183,18 +189,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#adapters.push(new JsonKeyValueAdapter()); } - get #refreshEnabled(): boolean { - return !!this.#options?.refreshOptions?.enabled; - } - - get #featureFlagEnabled(): boolean { - return !!this.#options?.featureFlagOptions?.enabled; - } - - get #featureFlagRefreshEnabled(): boolean { - return this.#featureFlagEnabled && !!this.#options?.featureFlagOptions?.refresh?.enabled; - } - get #requestTraceOptions(): RequestTracingOptions { return { enabled: this.#requestTracingEnabled, From 3eab635f96b805abf3950df7a71bcb2b585d2508 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 24 Feb 2025 15:10:37 +0800 Subject: [PATCH 26/47] remove extra blank line --- src/keyvault/AzureKeyVaultSecretProvider.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts index 39696e5e..df8b83b8 100644 --- a/src/keyvault/AzureKeyVaultSecretProvider.ts +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -82,7 +82,6 @@ export class AzureKeyVaultSecretProvider { this.#secretClients.set(vaultUrl.host, client); return client; } - return undefined; } } From 546d66afa9a7606f3d8054219b38e685bd58d581 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 24 Feb 2025 16:33:38 +0800 Subject: [PATCH 27/47] update --- src/refresh/RefreshTimer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 6eeb0527..cf207f8c 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -7,7 +7,7 @@ export class RefreshTimer { constructor(interval: number) { if (interval <= 0) { - throw new RangeError(`Refresh interval must be greater than 0. Given: ${this.interval}`); + throw new RangeError(`Refresh interval must be greater than 0. Given: ${interval}`); } this.interval = interval; From f8b76ed88ab7da6c6604f4f3ab4005de8c716632 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 26 Feb 2025 12:45:41 +0800 Subject: [PATCH 28/47] update --- src/AzureAppConfigurationImpl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1f34aeaa..1dceae10 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -369,7 +369,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { postAttempts += 1; backoffDuration = calculateBackoffDuration(postAttempts); } - console.warn(`Failed to load. Error message: ${error.message}. It Will retry in ${backoffDuration} ms.`); + console.warn(`Failed to load. Error message: ${error.message}. Retrying in ${backoffDuration} ms.`); await new Promise(resolve => setTimeout(resolve, backoffDuration)); } } while (!abortSignal.aborted); From 7e63ad5983adb4045d2c5e22698f5a4e14c818ee Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 26 Feb 2025 12:46:37 +0800 Subject: [PATCH 29/47] update --- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 58cd7571..f75e7bef 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -80,5 +80,5 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } function buildKeyVaultReferenceErrorMessage(message: string, setting: ConfigurationSetting, secretIdentifier?: string ): string { - return `${message} Key: ${setting.key} Label: ${setting.label ?? ""} ETag: ${setting.etag ?? ""} SecretIdentifier: ${secretIdentifier ?? ""}`; + return `${message} Key: '${setting.key}' Label: '${setting.label ?? ""}' ETag: '${setting.etag ?? ""}' SecretIdentifier: '${secretIdentifier ?? ""}'`; } From fe9ad2f7ad91f2d7a5817e8f12250550e3d6bb3e Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 28 Feb 2025 02:07:28 +0800 Subject: [PATCH 30/47] add boot loop protection --- src/AzureAppConfigurationImpl.ts | 14 +++++++++++++- src/error.ts | 9 +++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1f34aeaa..dab542f9 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -42,7 +42,9 @@ import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOp import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; import { getFixedBackoffDuration, calculateBackoffDuration } from "./failover.js"; -import { OperationError, ArgumentError, isFailoverableError, isRetriableError } from "./error.js"; +import { OperationError, ArgumentError, isFailoverableError, isRetriableError, isInstantlyThrowError } from "./error.js"; + +const MIN_DELAY_FOR_UNHANDLED_FAILURE = 5_000; // 5 seconds type PagedSettingSelector = SettingSelector & { /** @@ -232,6 +234,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Loads the configuration store for the first time. */ async load() { + const startTimestamp = Date.now(); const startupTimeout: number = this.#options?.startupOptions?.timeoutInMs ?? DEFAULT_STARTUP_TIMEOUT_IN_MS; const abortController = new AbortController(); const abortSignal = abortController.signal; @@ -252,6 +255,15 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { }) ]); } catch (error) { + if (!isInstantlyThrowError(error)) { + const timeElapsed = Date.now() - startTimestamp; + if (timeElapsed < MIN_DELAY_FOR_UNHANDLED_FAILURE) { + // load() method is called in the application's startup code path. + // Unhandled exceptions cause application crash which can result in crash loops as orchestrators attempt to restart the application. + // Knowing the intended usage of the provider in startup code path, we mitigate back-to-back crash loops from overloading the server with requests by waiting a minimum time to propogate fatal errors. + await new Promise(resolve => setTimeout(resolve, MIN_DELAY_FOR_UNHANDLED_FAILURE - timeElapsed)); + } + } throw new Error(`Failed to load: ${error.message}`); } finally { clearTimeout(timeoutId); // cancel the timeout promise diff --git a/src/error.ts b/src/error.ts index a73096e1..13e09b37 100644 --- a/src/error.ts +++ b/src/error.ts @@ -59,3 +59,12 @@ export function isRetriableError(error: any): boolean { } return true; } + +export function isInstantlyThrowError(error: any): boolean { + if (error instanceof ArgumentError || + error instanceof TypeError || + error instanceof RangeError) { + return true; + } + return false; +} From b19732d55864ee35a4eeb41b2122388dc93be016 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Tue, 4 Mar 2025 12:56:11 +0800 Subject: [PATCH 31/47] update --- src/AzureAppConfigurationImpl.ts | 4 ++-- src/refresh/RefreshTimer.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 279f9779..5bf061f6 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -252,7 +252,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (timeElapsed < MIN_DELAY_FOR_UNHANDLED_FAILURE) { // load() method is called in the application's startup code path. // Unhandled exceptions cause application crash which can result in crash loops as orchestrators attempt to restart the application. - // Knowing the intended usage of the provider in startup code path, we mitigate back-to-back crash loops from overloading the server with requests by waiting a minimum time to propogate fatal errors. + // Knowing the intended usage of the provider in startup code path, we mitigate back-to-back crash loops from overloading the server with requests by waiting a minimum time to propagate fatal errors. await new Promise(resolve => setTimeout(resolve, MIN_DELAY_FOR_UNHANDLED_FAILURE - timeElapsed)); } } @@ -351,6 +351,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (!this.#isInitialLoadCompleted) { await this.#inspectFmPackage(); const startTimestamp = Date.now(); + let postAttempts = 0; do { // at least try to load once try { await this.#loadSelectedAndWatchedKeyValues(); @@ -367,7 +368,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return; } const timeElapsed = Date.now() - startTimestamp; - let postAttempts = 0; let backoffDuration = getFixedBackoffDuration(timeElapsed); if (backoffDuration === undefined) { postAttempts += 1; diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index 044ee4e4..5dff67fd 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -7,7 +7,7 @@ export class RefreshTimer { constructor(interval: number) { if (interval <= 0) { - throw new RangeError(`Refresh interval must be greater than 0. Given: ${this.#interval}`); + throw new RangeError(`Refresh interval must be greater than 0. Given: ${interval}`); } this.#interval = interval; From 009ccd5ba52b60a4d63f67fb9e3189e2c38aabbf Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 13 Mar 2025 13:03:15 +0800 Subject: [PATCH 32/47] update --- src/AzureAppConfigurationImpl.ts | 14 +++++++------- src/ConfigurationClientManager.ts | 6 +++--- src/common/utils.ts | 12 ------------ src/error.ts | 12 +++++------- src/failover.ts | 1 - src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 6 +++--- 6 files changed, 18 insertions(+), 33 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 5bf061f6..78e46b59 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -34,7 +34,7 @@ import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOp import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; import { getFixedBackoffDuration, calculateBackoffDuration } from "./failover.js"; -import { OperationError, ArgumentError, isFailoverableError, isRetriableError, isInstantlyThrowError } from "./error.js"; +import { InvalidOperationError, ArgumentError, isFailoverableError, isRetriableError, isArgumentError } from "./error.js"; const MIN_DELAY_FOR_UNHANDLED_FAILURE = 5_000; // 5 seconds @@ -247,7 +247,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { }) ]); } catch (error) { - if (!isInstantlyThrowError(error)) { + if (!isArgumentError(error)) { const timeElapsed = Date.now() - startTimestamp; if (timeElapsed < MIN_DELAY_FOR_UNHANDLED_FAILURE) { // load() method is called in the application's startup code path. @@ -282,7 +282,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const segment = segments[i]; // undefined or empty string if (!segment) { - throw new OperationError(`Failed to construct configuration object: Invalid key: ${key}`); + throw new InvalidOperationError(`Failed to construct configuration object: Invalid key: ${key}`); } // create path if not exist if (current[segment] === undefined) { @@ -290,14 +290,14 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } // The path has been occupied by a non-object value, causing ambiguity. if (typeof current[segment] !== "object") { - throw new OperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The path '${segments.slice(0, i + 1).join(separator)}' has been occupied.`); + throw new InvalidOperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The path '${segments.slice(0, i + 1).join(separator)}' has been occupied.`); } current = current[segment]; } const lastSegment = segments[segments.length - 1]; if (current[lastSegment] !== undefined) { - throw new OperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The key should not be part of another key.`); + throw new InvalidOperationError(`Ambiguity occurs when constructing configuration object from key '${key}', value '${value}'. The key should not be part of another key.`); } // set value to the last segment current[lastSegment] = value; @@ -310,7 +310,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ async refresh(): Promise { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new OperationError("Refresh is not enabled for key-values or feature flags."); + throw new InvalidOperationError("Refresh is not enabled for key-values or feature flags."); } if (this.#refreshInProgress) { @@ -329,7 +329,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ onRefresh(listener: () => any, thisArg?: any): Disposable { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new OperationError("Refresh is not enabled for key-values or feature flags."); + throw new InvalidOperationError("Refresh is not enabled for key-values or feature flags."); } const boundedListener = listener.bind(thisArg); diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index 9cc8784d..6cbd62d3 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -7,7 +7,7 @@ import { TokenCredential } from "@azure/identity"; import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; import { isBrowser, isWebWorker } from "./requestTracing/utils.js"; import * as RequestTracing from "./requestTracing/constants.js"; -import { instanceOfTokenCredential, shuffleList, getEndpointUrl } from "./common/utils.js"; +import { instanceOfTokenCredential, shuffleList } from "./common/utils.js"; import { ArgumentError } from "./error.js"; // Configuration client retry options @@ -61,7 +61,7 @@ export class ConfigurationClientManager { const regexMatch = connectionString.match(ConnectionStringRegex); if (regexMatch) { const endpointFromConnectionStr = regexMatch[1]; - this.endpoint = getEndpointUrl(endpointFromConnectionStr); + this.endpoint = new URL(endpointFromConnectionStr); this.#id = regexMatch[2]; this.#secret = regexMatch[3]; } else { @@ -72,7 +72,7 @@ export class ConfigurationClientManager { let endpoint = connectionStringOrEndpoint; // ensure string is a valid URL. if (typeof endpoint === "string") { - endpoint = getEndpointUrl(endpoint); + endpoint = new URL(endpoint); } const credential = credentialOrOptions as TokenCredential; diff --git a/src/common/utils.ts b/src/common/utils.ts index 2359262b..18667874 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -9,18 +9,6 @@ export function shuffleList(array: T[]): T[] { return array; } -export function getEndpointUrl(endpoint: string): URL { - try { - return new URL(endpoint); - } catch (error) { - throw new TypeError(`Invalid Endpoint URL: ${endpoint}`); - } -} - -export function getUrlHost(url: string): string { - return new URL(url).host; -} - export function instanceOfTokenCredential(obj: unknown) { return obj && typeof obj === "object" && "getToken" in obj && typeof obj.getToken === "function"; } diff --git a/src/error.ts b/src/error.ts index 13e09b37..6bb46ad6 100644 --- a/src/error.ts +++ b/src/error.ts @@ -6,10 +6,10 @@ import { isRestError } from "@azure/core-rest-pipeline"; /** * Error thrown when an operation cannot be performed by the Azure App Configuration provider. */ -export class OperationError extends Error { +export class InvalidOperationError extends Error { constructor(message: string) { super(message); - this.name = "OperationError"; + this.name = "InvalidOperationError"; } } @@ -38,7 +38,7 @@ export function isFailoverableError(error: any): boolean { return false; } // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory - if (error.code == "ENOTFOUND" || error.code === "ENOENT") { + if (error.code === "ENOTFOUND" || error.code === "ENOENT") { return true; } // 401 Unauthorized, 403 Forbidden, 408 Request Timeout, 429 Too Many Requests, 5xx Server Errors @@ -51,16 +51,14 @@ export function isFailoverableError(error: any): boolean { } export function isRetriableError(error: any): boolean { - if (error instanceof ArgumentError || - error instanceof OperationError || - error instanceof TypeError || + if (isArgumentError(error) || error instanceof RangeError) { return false; } return true; } -export function isInstantlyThrowError(error: any): boolean { +export function isArgumentError(error: any): boolean { if (error instanceof ArgumentError || error instanceof TypeError || error instanceof RangeError) { diff --git a/src/failover.ts b/src/failover.ts index b1b03586..0d9eed54 100644 --- a/src/failover.ts +++ b/src/failover.ts @@ -5,7 +5,6 @@ const MIN_BACKOFF_DURATION = 30_000; // 30 seconds in milliseconds const MAX_BACKOFF_DURATION = 10 * 60 * 1000; // 10 minutes in milliseconds const JITTER_RATIO = 0.25; -// The backoff duration algorithm is consistent with the .NET configuration provider's implementation. export function getFixedBackoffDuration(timeElapsed: number): number | undefined { if (timeElapsed <= 100_000) { // 100 seconds in milliseconds return 5_000; // 5 seconds in milliseconds diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index f75e7bef..5135f166 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -4,14 +4,13 @@ import { ConfigurationSetting, isSecretReference, parseSecretReference } from "@azure/app-configuration"; import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; -import { getUrlHost } from "../common/utils.js"; import { ArgumentError, KeyVaultReferenceError } from "../error.js"; import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { /** * Map vault hostname to corresponding secret client. - */ + */ #secretClients: Map; #keyVaultOptions: KeyVaultOptions | undefined; @@ -59,7 +58,8 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { if (this.#secretClients === undefined) { this.#secretClients = new Map(); for (const client of this.#keyVaultOptions?.secretClients ?? []) { - this.#secretClients.set(getUrlHost(client.vaultUrl), client); + const clientUrl = new URL(client.vaultUrl); + this.#secretClients.set(clientUrl.host, client); } } From d61aba9aecd4380a138f9d54ea05c1fc9de1df11 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 13 Mar 2025 13:10:38 +0800 Subject: [PATCH 33/47] update testcase --- test/load.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/load.test.ts b/test/load.test.ts index 20dd9d46..599392a4 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -114,12 +114,12 @@ describe("load", function () { }); it("should throw error given invalid connection string", async () => { - return expect(load("invalid-connection-string")).eventually.rejectedWith("Invalid connection string."); + return expect(load("invalid-connection-string")).eventually.rejectedWith("Invalid connection string"); }); it("should throw error given invalid endpoint URL", async () => { const credential = createMockedTokenCredential(); - return expect(load("invalid-endpoint-url", credential)).eventually.rejectedWith("Invalid Endpoint URL: invalid-endpoint-url"); + return expect(load("invalid-endpoint-url", credential)).eventually.rejectedWith("Invalid URL"); }); it("should not include feature flags directly in the settings", async () => { From 73a24d4740b609f0fd97696e20149c5381ea0b4d Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 23 Apr 2025 17:13:04 +0800 Subject: [PATCH 34/47] update --- src/ConfigurationClientManager.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index 622f1453..1e429f13 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -43,9 +43,6 @@ export class ConfigurationClientManager { #lastFallbackClientUpdateTime: number = 0; // enforce to discover fallback client when it is expired #lastFallbackClientRefreshAttempt: number = 0; // avoid refreshing clients before the minimal refresh interval - // This property is public to allow recording the last successful endpoint for failover. - endpoint: URL; - constructor ( connectionStringOrEndpoint?: string | URL, credentialOrOptions?: TokenCredential | AzureAppConfigurationOptions, From 2c362ced2dcbceaa63ff61470f1268fe81d25886 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 23 Apr 2025 17:30:02 +0800 Subject: [PATCH 35/47] update testcase --- test/requestTracing.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index 4f4ec9fa..0b18f4b5 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -82,6 +82,9 @@ describe("request tracing", function () { await load(createMockedConnectionString(fakeEndpoint), { clientOptions, loadBalancingEnabled: true, + startupOptions: { + timeoutInMs: 1 + } }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; From bb180c2c3689110dc4726ca0c5dd323a973017a8 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Tue, 6 May 2025 15:20:24 +0800 Subject: [PATCH 36/47] revert unintended change --- src/failover.ts | 37 ------------------------------------- 1 file changed, 37 deletions(-) delete mode 100644 src/failover.ts diff --git a/src/failover.ts b/src/failover.ts deleted file mode 100644 index 1d7436fc..00000000 --- a/src/failover.ts +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -const MIN_BACKOFF_DURATION_IN_MS = 30_000; -const MAX_BACKOFF_DURATION_IN_MS = 10 * 60 * 1000; -const JITTER_RATIO = 0.25; - -export function getFixedBackoffDuration(timeElapsedInMs: number): number | undefined { - if (timeElapsedInMs <= 100_000) { - return 5_000; - } - if (timeElapsedInMs <= 200_000) { - return 10_000; - } - if (timeElapsedInMs <= 10 * 60 * 1000) { - return MIN_BACKOFF_DURATION_IN_MS; - } - return undefined; -} - -export function calculateBackoffDuration(failedAttempts: number) { - if (failedAttempts <= 1) { - return MIN_BACKOFF_DURATION_IN_MS; - } - - // exponential: minBackoff * 2 ^ (failedAttempts - 1) - // The right shift operator is not used in order to avoid potential overflow. Bitwise operations in JavaScript are limited to 32 bits. - let calculatedBackoffDuration = MIN_BACKOFF_DURATION_IN_MS * Math.pow(2, failedAttempts - 1); - if (calculatedBackoffDuration > MAX_BACKOFF_DURATION_IN_MS) { - calculatedBackoffDuration = MAX_BACKOFF_DURATION_IN_MS; - } - - // jitter: random value between [-1, 1) * jitterRatio * calculatedBackoffMs - const jitter = JITTER_RATIO * (Math.random() * 2 - 1); - - return calculatedBackoffDuration * (1 + jitter); -} From 11b648cf68d90e84654af3e3aef0a2db2e34bc6c Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 7 May 2025 11:37:45 +0800 Subject: [PATCH 37/47] update --- src/AzureAppConfigurationImpl.ts | 2 +- src/IKeyValueAdapter.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 287e989f..09488958 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -321,7 +321,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ async refresh(): Promise { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled && !this.#secretRefreshEnabled) { - throw new InvalidOperationError("Refresh is not enabled for key-values, key vault secrets or feature flags."); + throw new InvalidOperationError("Refresh is not enabled for key-values, feature flags or Key Vault secrets."); } if (this.#refreshInProgress) { diff --git a/src/IKeyValueAdapter.ts b/src/IKeyValueAdapter.ts index ab22c3ab..52d8342e 100644 --- a/src/IKeyValueAdapter.ts +++ b/src/IKeyValueAdapter.ts @@ -17,5 +17,5 @@ export interface IKeyValueAdapter { /** * This method is called when a change is detected in the configuration setting. */ - onChangeDetected(setting?: ConfigurationSetting): void; + onChangeDetected(setting?: ConfigurationSetting): Promise; } From 7a4b2711c8c2a9bbec82f475a67ceade6fcbdcaf Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 7 May 2025 12:29:53 +0800 Subject: [PATCH 38/47] update testcase --- src/AzureAppConfigurationImpl.ts | 2 +- test/refresh.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 09488958..ded6c622 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -340,7 +340,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ onRefresh(listener: () => any, thisArg?: any): Disposable { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled && !this.#secretRefreshEnabled) { - throw new InvalidOperationError("Refresh is not enabled for key-values, key vault secrets or feature flags."); + throw new InvalidOperationError("Refresh is not enabled for key-values, feature flags or Key Vault secrets."); } const boundedListener = listener.bind(thisArg); diff --git a/test/refresh.test.ts b/test/refresh.test.ts index ef170c26..c37c04ad 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -55,7 +55,7 @@ describe("dynamic refresh", function () { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); const refreshCall = settings.refresh(); - return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values, key vault secrets or feature flags."); + return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values, feature flags or Key Vault secrets."); }); it("should not allow refresh interval less than 1 second", async () => { @@ -117,7 +117,7 @@ describe("dynamic refresh", function () { it("should throw error when calling onRefresh when refresh is not enabled", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString); - expect(() => settings.onRefresh(() => { })).throws("Refresh is not enabled for key-values, key vault secrets or feature flags."); + expect(() => settings.onRefresh(() => { })).throws("Refresh is not enabled for key-values, feature flags or Key Vault secrets."); }); it("should only update values after refreshInterval", async () => { From 6d8c41f73df4daa0757b33c9c84bafca81e13be2 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 8 May 2025 02:10:03 +0800 Subject: [PATCH 39/47] always cached secret with version --- src/keyvault/AzureKeyVaultSecretProvider.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts index 8ed924b6..3bb72bf2 100644 --- a/src/keyvault/AzureKeyVaultSecretProvider.ts +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -31,26 +31,34 @@ export class AzureKeyVaultSecretProvider { } async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { + // The map key is a combination of sourceId and version: "{sourceId}\n{version}" + const identifierKey = `${secretIdentifier.sourceId}\n${secretIdentifier.version ?? ""}`; if (this.#refreshTimer && !this.#refreshTimer.canRefresh()) { // return the cached secret value if it exists - if (this.#cachedSecretValue.has(secretIdentifier.sourceId)) { - const cachedValue = this.#cachedSecretValue.get(secretIdentifier.sourceId); + if (this.#cachedSecretValue.has(identifierKey)) { + const cachedValue = this.#cachedSecretValue.get(identifierKey); return cachedValue; } // not found in cache, get the secret value from key vault const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); - this.#cachedSecretValue.set(secretIdentifier.sourceId, secretValue); + this.#cachedSecretValue.set(identifierKey, secretValue); return secretValue; } // Always reload the secret value from key vault when the refresh timer expires. const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); - this.#cachedSecretValue.set(secretIdentifier.sourceId, secretValue); + this.#cachedSecretValue.set(identifierKey, secretValue); return secretValue; } clearCache(): void { - this.#cachedSecretValue.clear(); + // If the secret identifier has specified a version, it is not removed from the cache. + // If the secret identifier has not specified a version, it means that the latest version should be used. Remove the cached value to force a reload. + for (const key of this.#cachedSecretValue.keys()) { + if (key.endsWith("\n")) { + this.#cachedSecretValue.delete(key); + } + } } async #getSecretValueFromKeyVault(secretIdentifier: KeyVaultSecretIdentifier): Promise { From e11cbd0d4e5b9ed73a20e532dcf6cdf61d9e6760 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 8 May 2025 11:34:58 +0800 Subject: [PATCH 40/47] update --- src/keyvault/AzureKeyVaultSecretProvider.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts index 3bb72bf2..264f9696 100644 --- a/src/keyvault/AzureKeyVaultSecretProvider.ts +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -31,21 +31,22 @@ export class AzureKeyVaultSecretProvider { } async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { - // The map key is a combination of sourceId and version: "{sourceId}\n{version}" + // The map key is a combination of sourceId and version: "{sourceId}\n{version}". const identifierKey = `${secretIdentifier.sourceId}\n${secretIdentifier.version ?? ""}`; + + // If the secret has a version, always use the cached value if available. + if (secretIdentifier.version && this.#cachedSecretValue.has(identifierKey)) { + return this.#cachedSecretValue.get(identifierKey); + } + if (this.#refreshTimer && !this.#refreshTimer.canRefresh()) { - // return the cached secret value if it exists + // If the refresh interval is not expired, return the cached value if available. if (this.#cachedSecretValue.has(identifierKey)) { - const cachedValue = this.#cachedSecretValue.get(identifierKey); - return cachedValue; + return this.#cachedSecretValue.get(identifierKey); } - // not found in cache, get the secret value from key vault - const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); - this.#cachedSecretValue.set(identifierKey, secretValue); - return secretValue; } - // Always reload the secret value from key vault when the refresh timer expires. + // Fallback to fetching the secret value from Key Vault. const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); this.#cachedSecretValue.set(identifierKey, secretValue); return secretValue; From 969c0dd1ff0e36e15147903986ed13aa6ef65fa6 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 8 May 2025 17:59:02 +0800 Subject: [PATCH 41/47] update error mesage --- src/AzureAppConfigurationImpl.ts | 2 +- test/keyvault.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index ded6c622..e91fd191 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -176,7 +176,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const { secretRefreshIntervalInMs } = options.keyVaultOptions; if (secretRefreshIntervalInMs !== undefined) { if (secretRefreshIntervalInMs < MIN_SECRET_REFRESH_INTERVAL_IN_MS) { - throw new RangeError(`The key vault secret refresh interval cannot be less than ${MIN_SECRET_REFRESH_INTERVAL_IN_MS} milliseconds.`); + throw new RangeError(`The Key Vault secret refresh interval cannot be less than ${MIN_SECRET_REFRESH_INTERVAL_IN_MS} milliseconds.`); } this.#secretRefreshEnabled = true; this.#secretRefreshTimer = new RefreshTimer(secretRefreshIntervalInMs); diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index eafd3516..b92db6f1 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -155,7 +155,7 @@ describe("key vault secret refresh", function () { secretRefreshIntervalInMs: 59999 // less than 60_000 milliseconds } }); - return expect(loadWithInvalidSecretRefreshInterval).eventually.rejectedWith("The key vault secret refresh interval cannot be less than 60000 milliseconds."); + return expect(loadWithInvalidSecretRefreshInterval).eventually.rejectedWith("The Key Vault secret refresh interval cannot be less than 60000 milliseconds."); }); it("should reload key vault secret when there is no change to key-values", async () => { From 16dff9f83c1e35b623c25e3335ee7d7d6ecdf338 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 11 May 2025 14:07:45 +0800 Subject: [PATCH 42/47] prevent refresh secrets operation when key alue refreshed --- src/AzureAppConfigurationImpl.ts | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index ded6c622..38ec650f 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -408,15 +408,24 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #refreshTasks(): Promise { const refreshTasks: Promise[] = []; - if (this.#refreshEnabled) { - refreshTasks.push(this.#refreshKeyValues()); + if (this.#refreshEnabled || this.#secretRefreshEnabled) { + refreshTasks.push( + this.#refreshKeyValues() + .then(keyValueRefreshed => { + // Only refresh secrets if key values didn't change and secret refresh is enabled + // If key values are refreshed, all secret references will be refreshed as well. + if (!keyValueRefreshed && this.#secretRefreshEnabled) { + // Returns the refreshSecrets promise directly. + // in a Promise chain, this automatically flattens nested Promises without requiring await. + return this.#refreshSecrets(); + } + return keyValueRefreshed; + }) + ); } if (this.#featureFlagRefreshEnabled) { refreshTasks.push(this.#refreshFeatureFlags()); } - if (this.#secretRefreshEnabled) { - refreshTasks.push(this.#refreshSecrets()); - } // wait until all tasks are either resolved or rejected const results = await Promise.allSettled(refreshTasks); @@ -579,7 +588,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ async #refreshKeyValues(): Promise { // if still within refresh interval/backoff, return - if (!this.#kvRefreshTimer.canRefresh()) { + if (this.#kvRefreshTimer === undefined || !this.#kvRefreshTimer.canRefresh()) { return Promise.resolve(false); } @@ -619,7 +628,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ async #refreshFeatureFlags(): Promise { // if still within refresh interval/backoff, return - if (!this.#ffRefreshTimer.canRefresh()) { + if (this.#ffRefreshInterval === undefined || !this.#ffRefreshTimer.canRefresh()) { return Promise.resolve(false); } @@ -634,7 +643,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { async #refreshSecrets(): Promise { // if still within refresh interval/backoff, return - if (!this.#secretRefreshTimer.canRefresh()) { + if (this.#secretRefreshTimer === undefined || !this.#secretRefreshTimer.canRefresh()) { return Promise.resolve(false); } From bf3d97153de8c67c7ceb570bd50c171995f1860c Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 14 May 2025 15:29:39 +0800 Subject: [PATCH 43/47] refresh secrets in parallel --- src/AzureAppConfigurationImpl.ts | 54 ++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index fece77ad..10df8c1c 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -517,19 +517,10 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#aiConfigurationTracing.reset(); } - const secretResolutionPromises: Promise[] = []; for (const setting of loadedSettings) { if (isSecretReference(setting)) { - if (this.#secretRefreshEnabled) { - this.#secretReferences.push(setting); - } + this.#secretReferences.push(setting); // cache secret references for resolve/refresh secret separately if (this.#resolveSecretInParallel) { - // secret references are resolved asynchronously to improve performance - const secretResolutionPromise = this.#processKeyValue(setting) - .then(([key, value]) => { - keyValues.push([key, value]); - }); - secretResolutionPromises.push(secretResolutionPromise); continue; } } @@ -537,9 +528,11 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { const [key, value] = await this.#processKeyValue(setting); keyValues.push([key, value]); } - if (secretResolutionPromises.length > 0) { - // wait for all secret resolution promises to be resolved - await Promise.all(secretResolutionPromises); + + if (this.#resolveSecretInParallel) { + await this.#resolveSecretsInParallel(this.#secretReferences, (key, value) => { + keyValues.push([key, value]); + }); } this.#clearLoadedKeyValues(); // clear existing key-values in case of configuration setting deletion @@ -666,9 +659,20 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return Promise.resolve(false); } - for (const setting of this.#secretReferences) { - const [key, value] = await this.#processKeyValue(setting); - this.#configMap.set(key, value); + // if no cached key vault references, return + if (this.#secretReferences.length === 0) { + return Promise.resolve(false); + } + + if (this.#resolveSecretInParallel) { + await this.#resolveSecretsInParallel(this.#secretReferences, (key, value) => { + this.#configMap.set(key, value); + }); + } else { + for (const setting of this.#secretReferences) { + const [key, value] = await this.#processKeyValue(setting); + this.#configMap.set(key, value); + } } this.#secretRefreshTimer.reset(); @@ -777,6 +781,24 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { throw new Error("All fallback clients failed to get configuration settings."); } + async #resolveSecretsInParallel(secretReferences: ConfigurationSetting[], resultHandler: (key: string, value: unknown) => void): Promise { + if (secretReferences.length === 0) { + return; + } + + const secretResolutionPromises: Promise[] = []; + for (const setting of secretReferences) { + const secretResolutionPromise = this.#processKeyValue(setting) + .then(([key, value]) => { + resultHandler(key, value); + }); + secretResolutionPromises.push(secretResolutionPromise); + } + + // Wait for all secret resolution promises to be resolved + await Promise.all(secretResolutionPromises); + } + async #processKeyValue(setting: ConfigurationSetting): Promise<[string, unknown]> { this.#setAIConfigurationTracing(setting); From a865bfd24283c1895da75917486f0e5d18460bcf Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 14 May 2025 15:54:08 +0800 Subject: [PATCH 44/47] update --- src/AzureAppConfigurationImpl.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index b35a8eb2..f5d7ee14 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -527,9 +527,11 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { keyValues.push([key, value]); } - await this.#resolveSecretReferences(this.#secretReferences, (key, value) => { - keyValues.push([key, value]); - }); + if (this.#secretReferences.length > 0) { + await this.#resolveSecretReferences(this.#secretReferences, (key, value) => { + keyValues.push([key, value]); + }); + } this.#clearLoadedKeyValues(); // clear existing key-values in case of configuration setting deletion for (const [k, v] of keyValues) { @@ -771,10 +773,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } async #resolveSecretReferences(secretReferences: ConfigurationSetting[], resultHandler: (key: string, value: unknown) => void): Promise { - if (secretReferences.length === 0) { - return; - } - if (this.#resolveSecretsInParallel) { const secretResolutionPromises: Promise[] = []; for (const setting of secretReferences) { From cbf32042cdffe9aa9651dabe8220b8780522d499 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Mon, 7 Jul 2025 16:50:07 +0800 Subject: [PATCH 45/47] enforce cache expiration for versioned secret --- src/IKeyValueAdapter.ts | 2 +- src/keyvault/AzureKeyVaultSecretProvider.ts | 13 ++++++++++--- src/keyvault/KeyVaultOptions.ts | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/IKeyValueAdapter.ts b/src/IKeyValueAdapter.ts index cb3360f1..222461dd 100644 --- a/src/IKeyValueAdapter.ts +++ b/src/IKeyValueAdapter.ts @@ -17,5 +17,5 @@ export interface IKeyValueAdapter { /** * This method is called when a change is detected in the configuration setting. */ - onChangeDetected(setting?: ConfigurationSetting): Promise; + onChangeDetected(): Promise; } diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts index 65ce292f..76e03e90 100644 --- a/src/keyvault/AzureKeyVaultSecretProvider.ts +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -8,7 +8,8 @@ import { SecretClient, KeyVaultSecretIdentifier } from "@azure/keyvault-secrets" export class AzureKeyVaultSecretProvider { #keyVaultOptions: KeyVaultOptions | undefined; - #refreshTimer: RefreshTimer | undefined; + #secretRefreshTimer: RefreshTimer | undefined; + #cacheRefreshTimer: RefreshTimer = new RefreshTimer(24*60*60*1000); // Enforce cache expiration every 24 hours #secretClients: Map; // map key vault hostname to corresponding secret client #cachedSecretValue: Map = new Map(); // map secret identifier to secret value @@ -22,7 +23,7 @@ export class AzureKeyVaultSecretProvider { } } this.#keyVaultOptions = keyVaultOptions; - this.#refreshTimer = refreshTimer; + this.#secretRefreshTimer = refreshTimer; this.#secretClients = new Map(); for (const client of this.#keyVaultOptions?.secretClients ?? []) { const clientUrl = new URL(client.vaultUrl); @@ -39,7 +40,7 @@ export class AzureKeyVaultSecretProvider { return this.#cachedSecretValue.get(identifierKey); } - if (this.#refreshTimer && !this.#refreshTimer.canRefresh()) { + if (this.#secretRefreshTimer && !this.#secretRefreshTimer.canRefresh()) { // If the refresh interval is not expired, return the cached value if available. if (this.#cachedSecretValue.has(identifierKey)) { return this.#cachedSecretValue.get(identifierKey); @@ -53,6 +54,12 @@ export class AzureKeyVaultSecretProvider { } clearCache(): void { + if (this.#cacheRefreshTimer.canRefresh()) { + // Clear the cache if the cache expiration timer has expired. + this.#cachedSecretValue.clear(); + this.#cacheRefreshTimer.reset(); + return; + } // If the secret identifier has specified a version, it is not removed from the cache. // If the secret identifier has not specified a version, it means that the latest version should be used. Remove the cached value to force a reload. for (const key of this.#cachedSecretValue.keys()) { diff --git a/src/keyvault/KeyVaultOptions.ts b/src/keyvault/KeyVaultOptions.ts index c4fc2af7..7f960872 100644 --- a/src/keyvault/KeyVaultOptions.ts +++ b/src/keyvault/KeyVaultOptions.ts @@ -44,7 +44,7 @@ export interface KeyVaultOptions { parallelSecretResolutionEnabled?: boolean; /** - * Specifies the refresh interval in milliseconds for periodically reloading secret from Key Vault. + * Specifies the refresh interval in milliseconds for periodically reloading all secrets from Key Vault. * * @remarks * If specified, the value must be greater than 60 seconds. From 349d678bfede96627a609099cdccbb8367e3ce26 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 8 Jul 2025 11:32:53 +0800 Subject: [PATCH 46/47] always clear cache --- src/keyvault/AzureKeyVaultSecretProvider.ts | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts index 76e03e90..b218718f 100644 --- a/src/keyvault/AzureKeyVaultSecretProvider.ts +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -9,7 +9,6 @@ import { SecretClient, KeyVaultSecretIdentifier } from "@azure/keyvault-secrets" export class AzureKeyVaultSecretProvider { #keyVaultOptions: KeyVaultOptions | undefined; #secretRefreshTimer: RefreshTimer | undefined; - #cacheRefreshTimer: RefreshTimer = new RefreshTimer(24*60*60*1000); // Enforce cache expiration every 24 hours #secretClients: Map; // map key vault hostname to corresponding secret client #cachedSecretValue: Map = new Map(); // map secret identifier to secret value @@ -32,8 +31,7 @@ export class AzureKeyVaultSecretProvider { } async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { - // The map key is a combination of sourceId and version: "{sourceId}\n{version}". - const identifierKey = `${secretIdentifier.sourceId}\n${secretIdentifier.version ?? ""}`; + const identifierKey = secretIdentifier.sourceId; // If the secret has a version, always use the cached value if available. if (secretIdentifier.version && this.#cachedSecretValue.has(identifierKey)) { @@ -54,19 +52,7 @@ export class AzureKeyVaultSecretProvider { } clearCache(): void { - if (this.#cacheRefreshTimer.canRefresh()) { - // Clear the cache if the cache expiration timer has expired. - this.#cachedSecretValue.clear(); - this.#cacheRefreshTimer.reset(); - return; - } - // If the secret identifier has specified a version, it is not removed from the cache. - // If the secret identifier has not specified a version, it means that the latest version should be used. Remove the cached value to force a reload. - for (const key of this.#cachedSecretValue.keys()) { - if (key.endsWith("\n")) { - this.#cachedSecretValue.delete(key); - } - } + this.#cachedSecretValue.clear(); } async #getSecretValueFromKeyVault(secretIdentifier: KeyVaultSecretIdentifier): Promise { From e8dd7f85ff9d0a48a5689c25aaed62ac1d422723 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 10 Jul 2025 10:50:57 +0800 Subject: [PATCH 47/47] update --- src/keyvault/AzureKeyVaultSecretProvider.ts | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/keyvault/AzureKeyVaultSecretProvider.ts b/src/keyvault/AzureKeyVaultSecretProvider.ts index b218718f..546b4be5 100644 --- a/src/keyvault/AzureKeyVaultSecretProvider.ts +++ b/src/keyvault/AzureKeyVaultSecretProvider.ts @@ -10,7 +10,7 @@ export class AzureKeyVaultSecretProvider { #keyVaultOptions: KeyVaultOptions | undefined; #secretRefreshTimer: RefreshTimer | undefined; #secretClients: Map; // map key vault hostname to corresponding secret client - #cachedSecretValue: Map = new Map(); // map secret identifier to secret value + #cachedSecretValues: Map = new Map(); // map secret identifier to secret value constructor(keyVaultOptions: KeyVaultOptions | undefined, refreshTimer?: RefreshTimer) { if (keyVaultOptions?.secretRefreshIntervalInMs !== undefined) { @@ -33,26 +33,20 @@ export class AzureKeyVaultSecretProvider { async getSecretValue(secretIdentifier: KeyVaultSecretIdentifier): Promise { const identifierKey = secretIdentifier.sourceId; - // If the secret has a version, always use the cached value if available. - if (secretIdentifier.version && this.#cachedSecretValue.has(identifierKey)) { - return this.#cachedSecretValue.get(identifierKey); - } - - if (this.#secretRefreshTimer && !this.#secretRefreshTimer.canRefresh()) { - // If the refresh interval is not expired, return the cached value if available. - if (this.#cachedSecretValue.has(identifierKey)) { - return this.#cachedSecretValue.get(identifierKey); - } + // If the refresh interval is not expired, return the cached value if available. + if (this.#cachedSecretValues.has(identifierKey) && + (!this.#secretRefreshTimer || !this.#secretRefreshTimer.canRefresh())) { + return this.#cachedSecretValues.get(identifierKey); } // Fallback to fetching the secret value from Key Vault. const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier); - this.#cachedSecretValue.set(identifierKey, secretValue); + this.#cachedSecretValues.set(identifierKey, secretValue); return secretValue; } clearCache(): void { - this.#cachedSecretValue.clear(); + this.#cachedSecretValues.clear(); } async #getSecretValueFromKeyVault(secretIdentifier: KeyVaultSecretIdentifier): Promise {