From 2720628cd571d16c8a4ffaa197a5b74edc6f0742 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 17 Feb 2025 23:05:14 +0800 Subject: [PATCH 01/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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 a9bcea45843a66a581302cc13cfc0284a7fa700c Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 19:06:25 +0800 Subject: [PATCH 16/37] 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 17/37] 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 18/37] 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 19/37] 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 39d9a3de10d7f52ddc8a9f3e9310d8ccf59cf4ae Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 23 Feb 2025 21:22:30 +0800 Subject: [PATCH 20/37] 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 f8b76ed88ab7da6c6604f4f3ab4005de8c716632 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 26 Feb 2025 12:45:41 +0800 Subject: [PATCH 21/37] 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 22/37] 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 23/37] 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 24/37] 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 25/37] 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 26/37] 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 27/37] 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 28/37] 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 56f6265ea764b9650597df2b98f29c660322cd94 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 24 Apr 2025 12:40:47 +0800 Subject: [PATCH 29/37] update --- src/AzureAppConfigurationImpl.ts | 2 +- src/ConfigurationClientManager.ts | 2 +- src/ConfigurationClientWrapper.ts | 2 +- src/{failover.ts => backoffDuration.ts} | 6 +++--- src/error.ts | 6 ++++-- 5 files changed, 10 insertions(+), 8 deletions(-) rename src/{failover.ts => backoffDuration.ts} (86%) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index e0bf085e..eddb9635 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -34,7 +34,7 @@ import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOp import { AIConfigurationTracingOptions } from "./requestTracing/AIConfigurationTracingOptions.js"; import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; -import { getFixedBackoffDuration, calculateBackoffDuration } from "./failover.js"; +import { getFixedBackoffDuration, calculateBackoffDuration } from "./backoffDuration.js"; import { InvalidOperationError, ArgumentError, isFailoverableError, isRetriableError, isArgumentError } from "./error.js"; const MIN_DELAY_FOR_UNHANDLED_FAILURE = 5_000; // 5 seconds diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index 1e429f13..eedbb570 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 } from "./common/utils.js"; +import { shuffleList, instanceOfTokenCredential } from "./common/utils.js"; import { ArgumentError } from "./error.js"; // Configuration client retry options diff --git a/src/ConfigurationClientWrapper.ts b/src/ConfigurationClientWrapper.ts index 1a3d2c85..1f6ba2e1 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 { calculateBackoffDuration } from "./failover.js"; +import { calculateBackoffDuration } from "./backoffDuration.js"; export class ConfigurationClientWrapper { endpoint: string; diff --git a/src/failover.ts b/src/backoffDuration.ts similarity index 86% rename from src/failover.ts rename to src/backoffDuration.ts index 0d9eed54..3fa421ff 100644 --- a/src/failover.ts +++ b/src/backoffDuration.ts @@ -6,13 +6,13 @@ const MAX_BACKOFF_DURATION = 10 * 60 * 1000; // 10 minutes in milliseconds const JITTER_RATIO = 0.25; export function getFixedBackoffDuration(timeElapsed: number): number | undefined { - if (timeElapsed <= 100_000) { // 100 seconds in milliseconds + if (timeElapsed < 100_000) { // 100 seconds in milliseconds return 5_000; // 5 seconds in milliseconds } - if (timeElapsed <= 200_000) { // 200 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 + if (timeElapsed < 10 * 60 * 1000) { // 10 minutes in milliseconds return MIN_BACKOFF_DURATION; } return undefined; diff --git a/src/error.ts b/src/error.ts index 6bb46ad6..8305c267 100644 --- a/src/error.ts +++ b/src/error.ts @@ -51,13 +51,15 @@ export function isFailoverableError(error: any): boolean { } export function isRetriableError(error: any): boolean { - if (isArgumentError(error) || - error instanceof RangeError) { + if (isArgumentError(error)) { return false; } return true; } +/** + * Check if the error is an instance of ArgumentError, TypeError, or RangeError. + */ export function isArgumentError(error: any): boolean { if (error instanceof ArgumentError || error instanceof TypeError || From aa037e55cc70514c0f3d8faf6f1f8d230551d7d9 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 25 Apr 2025 12:02:03 +0800 Subject: [PATCH 30/37] update --- src/AzureAppConfigurationImpl.ts | 10 +++++----- src/ConfigurationClientWrapper.ts | 4 ++-- src/{backoffDuration.ts => common/backoffUtils.ts} | 14 +++++++------- src/error.ts | 13 +++---------- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 2 +- test/keyvault.test.ts | 2 +- 6 files changed, 19 insertions(+), 26 deletions(-) rename src/{backoffDuration.ts => common/backoffUtils.ts} (69%) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index eddb9635..930f1627 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -34,8 +34,8 @@ import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOp import { AIConfigurationTracingOptions } from "./requestTracing/AIConfigurationTracingOptions.js"; import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; -import { getFixedBackoffDuration, calculateBackoffDuration } from "./backoffDuration.js"; -import { InvalidOperationError, ArgumentError, isFailoverableError, isRetriableError, isArgumentError } from "./error.js"; +import { getFixedBackoffDuration, getExponentialBackoffDuration } from "./common/backoffUtils.js"; +import { InvalidOperationError, ArgumentError, isFailoverableError, isInputError } from "./error.js"; const MIN_DELAY_FOR_UNHANDLED_FAILURE = 5_000; // 5 seconds @@ -251,7 +251,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { }) ]); } catch (error) { - if (!isArgumentError(error)) { + if (!isInputError(error)) { const timeElapsed = Date.now() - startTimestamp; if (timeElapsed < MIN_DELAY_FOR_UNHANDLED_FAILURE) { // load() method is called in the application's startup code path. @@ -365,7 +365,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#isInitialLoadCompleted = true; break; } catch (error) { - if (!isRetriableError(error)) { + if (isInputError(error)) { throw error; } if (abortSignal.aborted) { @@ -375,7 +375,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { let backoffDuration = getFixedBackoffDuration(timeElapsed); if (backoffDuration === undefined) { postAttempts += 1; - backoffDuration = calculateBackoffDuration(postAttempts); + backoffDuration = getExponentialBackoffDuration(postAttempts); } console.warn(`Failed to load. Error message: ${error.message}. Retrying in ${backoffDuration} ms.`); await new Promise(resolve => setTimeout(resolve, backoffDuration)); diff --git a/src/ConfigurationClientWrapper.ts b/src/ConfigurationClientWrapper.ts index 1f6ba2e1..137d1c38 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 { calculateBackoffDuration } from "./backoffDuration.js"; +import { getExponentialBackoffDuration } from "./common/backoffUtils.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() + calculateBackoffDuration(this.#failedAttempts); + this.backoffEndTime = Date.now() + getExponentialBackoffDuration(this.#failedAttempts); } } } diff --git a/src/backoffDuration.ts b/src/common/backoffUtils.ts similarity index 69% rename from src/backoffDuration.ts rename to src/common/backoffUtils.ts index 3fa421ff..2bebf5c4 100644 --- a/src/backoffDuration.ts +++ b/src/common/backoffUtils.ts @@ -5,20 +5,20 @@ 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; -export function getFixedBackoffDuration(timeElapsed: number): number | undefined { - if (timeElapsed < 100_000) { // 100 seconds in milliseconds - return 5_000; // 5 seconds in milliseconds +export function getFixedBackoffDuration(timeElapsedInMs: number): number | undefined { + if (timeElapsedInMs < 100_000) { + return 5_000; } - if (timeElapsed < 200_000) { // 200 seconds in milliseconds - return 10_000; // 10 seconds in milliseconds + if (timeElapsedInMs < 200_000) { + return 10_000; } - if (timeElapsed < 10 * 60 * 1000) { // 10 minutes in milliseconds + if (timeElapsedInMs < 10 * 60 * 1000) { return MIN_BACKOFF_DURATION; } return undefined; } -export function calculateBackoffDuration(failedAttempts: number) { +export function getExponentialBackoffDuration(failedAttempts: number): number { if (failedAttempts <= 1) { return MIN_BACKOFF_DURATION; } diff --git a/src/error.ts b/src/error.ts index 8305c267..f9aa0aaa 100644 --- a/src/error.ts +++ b/src/error.ts @@ -14,7 +14,7 @@ export class InvalidOperationError extends Error { } /** - * Error thrown when an argument or configuration is invalid. + * Error thrown when an input argument is invalid. */ export class ArgumentError extends Error { constructor(message: string) { @@ -24,7 +24,7 @@ export class ArgumentError extends Error { } /** - * Error thrown when it fails to get the secret from the Key Vault. + * Error thrown when a Key Vault reference cannot be resolved. */ export class KeyVaultReferenceError extends Error { constructor(message: string) { @@ -50,17 +50,10 @@ export function isFailoverableError(error: any): boolean { return false; } -export function isRetriableError(error: any): boolean { - if (isArgumentError(error)) { - return false; - } - return true; -} - /** * Check if the error is an instance of ArgumentError, TypeError, or RangeError. */ -export function isArgumentError(error: any): boolean { +export function isInputError(error: any): boolean { if (error instanceof ArgumentError || error instanceof TypeError || error instanceof RangeError) { diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 5135f166..50530f82 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -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 ArgumentError("Failed to process the key vault reference. The keyVaultOptions is not configured."); + throw new ArgumentError("Failed to process the Key Vault reference because Key Vault options are not configured."); } const { name: secretName, vaultUrl, sourceId, version } = parseKeyVaultSecretIdentifier( diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 4f5946f0..35a48102 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("Failed to process the key vault reference. The keyVaultOptions is not configured."); + return expect(load(createMockedConnectionString())).eventually.rejectedWith("Failed to process the Key Vault reference because Key Vault options are not configured."); }); it("should resolve key vault reference with credential", async () => { From 3afb300fdc762978ca4b4ab3752117ac8f947f5f Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 25 Apr 2025 13:34:19 +0800 Subject: [PATCH 31/37] update --- src/error.ts | 9 +++++++-- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 15 ++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/error.ts b/src/error.ts index f9aa0aaa..ae6ff001 100644 --- a/src/error.ts +++ b/src/error.ts @@ -2,6 +2,8 @@ // 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. @@ -27,13 +29,16 @@ export class ArgumentError extends Error { * Error thrown when a Key Vault reference cannot be resolved. */ export class KeyVaultReferenceError extends Error { - constructor(message: string) { - super(message); + constructor(message: string, options?: ErrorOptions) { + super(message, options); this.name = "KeyVaultReferenceError"; } } export function isFailoverableError(error: any): boolean { + if (error instanceof AuthenticationError) { + return true; + } if (!isRestError(error)) { return false; } diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 50530f82..3a702b67 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -27,11 +27,12 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { if (!this.#keyVaultOptions) { throw new ArgumentError("Failed to process the Key Vault reference because Key Vault options are not configured."); } - - const { name: secretName, vaultUrl, sourceId, version } = parseKeyVaultSecretIdentifier( - parseSecretReference(setting).value.secretId - ); + let sourceId; try { + const { name: secretName, vaultUrl, sourceId: parsedSourceId, version } = parseKeyVaultSecretIdentifier( + parseSecretReference(setting).value.secretId + ); + sourceId = parsedSourceId; // precedence: secret clients > credential > secret resolver const client = this.#getSecretClient(new URL(vaultUrl)); if (client) { @@ -42,7 +43,7 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; } } catch (error) { - throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage(error.message, setting, sourceId)); + throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage(setting, sourceId), { cause: error }); } // When code reaches here, it means that the key vault reference cannot be resolved in all possible ways. @@ -79,6 +80,6 @@ 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 ?? ""}'`; +function buildKeyVaultReferenceErrorMessage(setting: ConfigurationSetting, secretIdentifier?: string ): string { + return `Failed to resolve Key Vault reference. Key: '${setting.key}' Label: '${setting.label ?? ""}' ETag: '${setting.etag ?? ""}' SecretIdentifier: '${secretIdentifier ?? ""}'`; } From f69be0dc8fc8989f9c8bba9cf4ac9df977a5ddd1 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 25 Apr 2025 13:36:18 +0800 Subject: [PATCH 32/37] move error.ts to common folder --- src/AzureAppConfigurationImpl.ts | 2 +- src/ConfigurationClientManager.ts | 2 +- src/{ => common}/error.ts | 0 src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename src/{ => common}/error.ts (100%) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 930f1627..753db341 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -35,7 +35,7 @@ import { AIConfigurationTracingOptions } from "./requestTracing/AIConfigurationT import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; import { getFixedBackoffDuration, getExponentialBackoffDuration } from "./common/backoffUtils.js"; -import { InvalidOperationError, ArgumentError, isFailoverableError, isInputError } from "./error.js"; +import { InvalidOperationError, ArgumentError, isFailoverableError, isInputError } from "./common/error.js"; const MIN_DELAY_FOR_UNHANDLED_FAILURE = 5_000; // 5 seconds diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index eedbb570..72a3bfeb 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -8,7 +8,7 @@ import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js" import { isBrowser, isWebWorker } from "./requestTracing/utils.js"; import * as RequestTracing from "./requestTracing/constants.js"; import { shuffleList, instanceOfTokenCredential } from "./common/utils.js"; -import { ArgumentError } from "./error.js"; +import { ArgumentError } from "./common/error.js"; // Configuration client retry options const CLIENT_MAX_RETRIES = 2; diff --git a/src/error.ts b/src/common/error.ts similarity index 100% rename from src/error.ts rename to src/common/error.ts diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 3a702b67..9fd7f9a1 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -4,7 +4,7 @@ import { ConfigurationSetting, isSecretReference, parseSecretReference } from "@azure/app-configuration"; import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; -import { ArgumentError, KeyVaultReferenceError } from "../error.js"; +import { ArgumentError, KeyVaultReferenceError } from "../common/error.js"; import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { From 2f6585cac22070f14b04f20771b16c272b0d934a Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 25 Apr 2025 14:07:50 +0800 Subject: [PATCH 33/37] handle transient network error --- src/common/error.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/common/error.ts b/src/common/error.ts index ae6ff001..179d26c7 100644 --- a/src/common/error.ts +++ b/src/common/error.ts @@ -2,8 +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. @@ -36,14 +34,12 @@ export class KeyVaultReferenceError extends Error { } export function isFailoverableError(error: any): boolean { - if (error instanceof AuthenticationError) { - return true; - } if (!isRestError(error)) { return false; } - // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory - if (error.code === "ENOTFOUND" || error.code === "ENOENT") { + // https://nodejs.org/api/errors.html#common-system-errors + // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory, ECONNREFUSED: connection refused, ECONNRESET: connection reset by peer, ETIMEDOUT: connection timed out + if (error.code === "ENOTFOUND" || error.code === "ENOENT" || error.code === "ECONNREFUSED" || error.code === "ECONNRESET" || error.code === "ETIMEDOUT") { return true; } // 401 Unauthorized, 403 Forbidden, 408 Request Timeout, 429 Too Many Requests, 5xx Server Errors From 4b22c86284b7ef1d416f8c70a77fe0f80996d028 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 25 Apr 2025 15:25:05 +0800 Subject: [PATCH 34/37] 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 9fd7f9a1..6dec4bf1 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -81,5 +81,5 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } function buildKeyVaultReferenceErrorMessage(setting: ConfigurationSetting, secretIdentifier?: string ): string { - return `Failed to resolve Key Vault reference. Key: '${setting.key}' Label: '${setting.label ?? ""}' ETag: '${setting.etag ?? ""}' SecretIdentifier: '${secretIdentifier ?? ""}'`; + return `Failed to resolve Key Vault reference. Key: '${setting.key}' Label: '${setting.label ?? ""}' ETag: '${setting.etag ?? ""}' ${secretIdentifier ? ` SecretIdentifier: '${secretIdentifier}'` : ""}`; } From a0f5f1e683253f4a080991cb83167dbbc21c36ab Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Sun, 27 Apr 2025 17:10:10 +0800 Subject: [PATCH 35/37] update --- src/common/error.ts | 10 ++++----- src/keyvault/AzureKeyVaultKeyValueAdapter.ts | 23 +++++++++++++++----- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/common/error.ts b/src/common/error.ts index 179d26c7..bd4f5adf 100644 --- a/src/common/error.ts +++ b/src/common/error.ts @@ -39,7 +39,8 @@ export function isFailoverableError(error: any): boolean { } // https://nodejs.org/api/errors.html#common-system-errors // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory, ECONNREFUSED: connection refused, ECONNRESET: connection reset by peer, ETIMEDOUT: connection timed out - if (error.code === "ENOTFOUND" || error.code === "ENOENT" || error.code === "ECONNREFUSED" || error.code === "ECONNRESET" || error.code === "ETIMEDOUT") { + if (error.code !== undefined && + (error.code === "ENOTFOUND" || error.code === "ENOENT" || error.code === "ECONNREFUSED" || error.code === "ECONNRESET" || error.code === "ETIMEDOUT")) { return true; } // 401 Unauthorized, 403 Forbidden, 408 Request Timeout, 429 Too Many Requests, 5xx Server Errors @@ -55,10 +56,7 @@ export function isFailoverableError(error: any): boolean { * Check if the error is an instance of ArgumentError, TypeError, or RangeError. */ export function isInputError(error: any): boolean { - if (error instanceof ArgumentError || + return error instanceof ArgumentError || error instanceof TypeError || - error instanceof RangeError) { - return true; - } - return false; + error instanceof RangeError; } diff --git a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts index 6dec4bf1..1b8c5977 100644 --- a/src/keyvault/AzureKeyVaultKeyValueAdapter.ts +++ b/src/keyvault/AzureKeyVaultKeyValueAdapter.ts @@ -6,6 +6,8 @@ import { IKeyValueAdapter } from "../IKeyValueAdapter.js"; import { KeyVaultOptions } from "./KeyVaultOptions.js"; import { ArgumentError, KeyVaultReferenceError } from "../common/error.js"; import { SecretClient, parseKeyVaultSecretIdentifier } from "@azure/keyvault-secrets"; +import { isRestError } from "@azure/core-rest-pipeline"; +import { AuthenticationError } from "@azure/identity"; export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { /** @@ -27,12 +29,20 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { if (!this.#keyVaultOptions) { throw new ArgumentError("Failed to process the Key Vault reference because Key Vault options are not configured."); } - let sourceId; + let secretName, vaultUrl, sourceId, version; try { - const { name: secretName, vaultUrl, sourceId: parsedSourceId, version } = parseKeyVaultSecretIdentifier( + const { name: parsedName, vaultUrl: parsedVaultUrl, sourceId: parsedSourceId, version: parsedVersion } = parseKeyVaultSecretIdentifier( parseSecretReference(setting).value.secretId ); + secretName = parsedName; + vaultUrl = parsedVaultUrl; sourceId = parsedSourceId; + version = parsedVersion; + } catch (error) { + throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage("Invalid Key Vault reference.", setting), { cause: error }); + } + + try { // precedence: secret clients > credential > secret resolver const client = this.#getSecretClient(new URL(vaultUrl)); if (client) { @@ -43,7 +53,10 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { return [setting.key, await this.#keyVaultOptions.secretResolver(new URL(sourceId))]; } } catch (error) { - throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage(setting, sourceId), { cause: error }); + if (isRestError(error) || error instanceof AuthenticationError) { + throw new KeyVaultReferenceError(buildKeyVaultReferenceErrorMessage("Failed to resolve Key Vault reference.", setting, sourceId), { cause: error }); + } + throw error; } // When code reaches here, it means that the key vault reference cannot be resolved in all possible ways. @@ -80,6 +93,6 @@ export class AzureKeyVaultKeyValueAdapter implements IKeyValueAdapter { } } -function buildKeyVaultReferenceErrorMessage(setting: ConfigurationSetting, secretIdentifier?: string ): string { - return `Failed to resolve Key Vault reference. Key: '${setting.key}' Label: '${setting.label ?? ""}' ETag: '${setting.etag ?? ""}' ${secretIdentifier ? ` SecretIdentifier: '${secretIdentifier}'` : ""}`; +function buildKeyVaultReferenceErrorMessage(message: string, setting: ConfigurationSetting, secretIdentifier?: string ): string { + return `${message} Key: '${setting.key}' Label: '${setting.label ?? ""}' ETag: '${setting.etag ?? ""}' ${secretIdentifier ? ` SecretIdentifier: '${secretIdentifier}'` : ""}`; } From f1e683e6d15cc34a344b372db5de8f31792262e7 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Mon, 28 Apr 2025 11:43:20 +0800 Subject: [PATCH 36/37] keep error stack when fail to load --- src/AzureAppConfigurationImpl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 753db341..fb523ab0 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -260,7 +260,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { await new Promise(resolve => setTimeout(resolve, MIN_DELAY_FOR_UNHANDLED_FAILURE - timeElapsed)); } } - throw new Error(`Failed to load: ${error.message}`); + throw new Error("Failed to load.", { cause: error }); } finally { clearTimeout(timeoutId); // cancel the timeout promise } From 4a316598d6ae4f821523a3d85d9ef475c697085d Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Mon, 28 Apr 2025 13:42:42 +0800 Subject: [PATCH 37/37] update testcase --- test/keyvault.test.ts | 33 +++++++++++++++++------- test/startup.test.ts | 60 ++++++++++++++++++++++++++++++------------- 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 35a48102..219a0bda 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -40,7 +40,15 @@ describe("key vault reference", function () { }); it("require key vault options to resolve reference", async () => { - return expect(load(createMockedConnectionString())).eventually.rejectedWith("Failed to process the Key Vault reference because Key Vault options are not configured."); + try { + await load(createMockedConnectionString()); + } catch (error) { + expect(error.message).eq("Failed to load."); + expect(error.cause.message).eq("Failed to process the Key Vault reference because Key Vault options are not configured."); + return; + } + // we should never reach here, load should throw an error + throw new Error("Expected load to throw."); }); it("should resolve key vault reference with credential", async () => { @@ -89,14 +97,21 @@ describe("key vault reference", function () { }); it("should throw error when secret clients not provided for all key vault references", async () => { - const loadKeyVaultPromise = load(createMockedConnectionString(), { - keyVaultOptions: { - secretClients: [ - new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()), - ] - } - }); - 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."); + try { + await load(createMockedConnectionString(), { + keyVaultOptions: { + secretClients: [ + new SecretClient("https://fake-vault-name.vault.azure.net", createMockedTokenCredential()), + ] + } + }); + } catch (error) { + expect(error.message).eq("Failed to load."); + expect(error.cause.message).eq("Failed to process the key vault reference. No key vault secret client, credential or secret resolver callback is available to resolve the secret."); + return; + } + // we should never reach here, load should throw an error + throw new Error("Expected load to throw."); }); 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 8c2797d0..51b46a3a 100644 --- a/test/startup.test.ts +++ b/test/startup.test.ts @@ -43,12 +43,20 @@ describe("startup", function () { [[{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); + try { + await load(createMockedConnectionString(), { + startupOptions: { + timeoutInMs: 5_000 + } + }); + } catch (error) { + expect(error.message).eq("Failed to load."); + expect(error.cause.message).eq("Load operation timed out."); + expect(attempt).eq(1); + return; + } + // we should never reach here, load should throw an error + throw new Error("Expected load to throw."); }); it("should not retry on non-retriable TypeError", async () => { @@ -61,12 +69,20 @@ describe("startup", function () { [[{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); + try { + await load(createMockedConnectionString(), { + startupOptions: { + timeoutInMs: 10_000 + } + }); + } catch (error) { + expect(error.message).eq("Failed to load."); + expect(error.cause.message).eq("Non-retriable Test Error"); + expect(attempt).eq(1); + return; + } + // we should never reach here, load should throw an error + throw new Error("Expected load to throw."); }); it("should not retry on non-retriable RangeError", async () => { @@ -79,11 +95,19 @@ describe("startup", function () { [[{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); + try { + await load(createMockedConnectionString(), { + startupOptions: { + timeoutInMs: 10_000 + } + }); + } catch (error) { + expect(error.message).eq("Failed to load."); + expect(error.cause.message).eq("Non-retriable Test Error"); + expect(attempt).eq(1); + return; + } + // we should never reach here, load should throw an error + throw new Error("Expected load to throw."); }); });