Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Startup Retry & Configurable Startup Time-out #166

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand Down
3 changes: 3 additions & 0 deletions src/AzureAppConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

import { Disposable } from "./common/disposable.js";

/**
* Azure App Configuration provider.
*/
export type AzureAppConfiguration = {
/**
* API to trigger refresh operation.
Expand Down
107 changes: 79 additions & 28 deletions src/AzureAppConfigurationImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import { AzureAppConfiguration, ConfigurationObjectConstructionOptions } from ".
import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js";
import { IKeyValueAdapter } from "./IKeyValueAdapter.js";
import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter.js";
import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./RefreshOptions.js";
import { DEFAULT_STARTUP_TIMEOUT_IN_MS } from "./StartupOptions.js";
import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./refresh/refreshOptions.js";
import { Disposable } from "./common/disposable.js";
import { base64Helper, jsonSorter } from "./common/utils.js";
import {
Expand Down Expand Up @@ -40,6 +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, calculateBackoffDuration } from "./failover.js";
import { OperationError, ArgumentError, isFailoverableError, isRetriableError } from "./error.js";

type PagedSettingSelector = SettingSelector & {
/**
Expand Down Expand Up @@ -123,10 +126,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 ArgumentError("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 ArgumentError("The characters '*' and ',' are not supported in label of watched settings.");
}
this.#sentinels.push(setting);
}
Expand All @@ -135,7 +138,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;
}
Expand All @@ -153,7 +156,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;
}
Expand Down Expand Up @@ -229,13 +232,30 @@ 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: number = this.#options?.startupOptions?.timeoutInMs ?? DEFAULT_STARTUP_TIMEOUT_IN_MS;
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 must ensure other promises will be eventually terminated.
await Promise.race([
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(timeoutId); // cancel the timeout promise
}
// Mark all settings have loaded at startup.
this.#isInitialLoadCompleted = true;
}

/**
Expand All @@ -245,7 +265,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 ArgumentError(`Invalid separator '${separator}'. Supported values: ${validSeparators.map(s => `'${s}'`).join(", ")}.`);
}

// construct hierarchical data object from map
Expand All @@ -258,22 +278,22 @@ 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) {
current[segment] = {};
}
// 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;
Expand All @@ -286,7 +306,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
*/
async refresh(): Promise<void> {
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) {
Expand All @@ -305,7 +325,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);
Expand All @@ -320,6 +340,42 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
return new Disposable(remove);
}

/**
* Initializes the configuration provider.
*/
async #initializeWithRetryPolicy(abortSignal: AbortSignal): Promise<void> {
if (!this.#isInitialLoadCompleted) {
await this.#inspectFmPackage();
const startTimestamp = Date.now();
do { // at least try to load once
try {
await this.#loadSelectedAndWatchedKeyValues();
if (this.#featureFlagEnabled) {
await this.#loadFeatureFlags();
}
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);
if (backoffDuration === undefined) {
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));
}
} while (!abortSignal.aborted);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the check on abortSignal is at the end of each try operation, so there may be a scenario that when it's still in loading process but timeout happens, program still need to run a little while before abort. May be you can setup timeout for signal like this https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did think about this option, it is an alternative for using Promise.race for timeout.

If we take this option, the code will be

async function load() {
	try {
		await initializeWithRetryPolicy(AbortSignal.timeout(5200)); // 200ms longer than default backoff duration
	} catch (error) {}
}

async function initializeWithRetryPolicy(signal) {
	do {
		try {
			//--> Position A
			// load keyvalue from App Config
		} catch (error) {
		 	//--> Position B
			// wait for 5000ms (default backoff duration)
		}
	} while (signal.abort)

	if (signal.abort) {
		throw new Error("Timeout")
	}
}

Let's say one failure of load kv from App Config (position A) costs 50ms
At T0, we set timeout for the signal (At T0+5200, it should be aborted)
At T0 + 50ms, the first attempt failed and it went to position B and started to wait for 5000ms
At T0+5050ms, it started to retry and it took 50m to fail again
At T0+5100ms, failed at position A again and started to wait for 5000ms or maybe even longer due to exponential backoff duration increase
At T0+5200ms, the signal timeout, but the code is still stuck at await initialize.

program still need to run a little while before abort.

This is half-true. Unless the customer write code like this:

try {
	const setting = await load();
} catch (err) {
	// do not throw here
}

The initializeWithRetryPolicy promise will not be resolved immediately after timeout. Otherwise, error is thrown, the node process is terminated. All promises running no longer exist.

}
}

/**
* Inspects the feature management package version.
*/
Expand Down Expand Up @@ -425,7 +481,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]);
Expand Down Expand Up @@ -600,6 +656,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
return response;
}

// Only operations related to Azure App Configuration should be executed with failover policy.
async #executeWithFailoverPolicy(funcToExecute: (client: AppConfigurationClient) => Promise<any>): Promise<any> {
let clientWrappers = await this.#clientManager.getClients();
if (this.#options?.loadBalancingEnabled && this.#lastSuccessfulEndpoint !== "" && clientWrappers.length > 1) {
Expand Down Expand Up @@ -639,7 +696,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
}

this.#clientManager.refreshClients();
throw new Error("All clients failed to get configuration settings.");
throw new Error("All fallback clients failed to get configuration settings.");
}

async #processKeyValues(setting: ConfigurationSetting<string>): Promise<[string, unknown]> {
Expand Down Expand Up @@ -671,7 +728,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration {
async #parseFeatureFlag(setting: ConfigurationSetting<string>): Promise<any> {
const rawFlag = setting.value;
if (rawFlag === undefined) {
throw new Error("The value of configuration setting cannot be undefined.");
throw new ArgumentError("The value of configuration setting cannot be undefined.");
}
const featureFlag = JSON.parse(rawFlag);

Expand Down Expand Up @@ -895,13 +952,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 ArgumentError("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 ArgumentError("The characters '*' and ',' are not supported in label filters.");
}
return selector;
});
Expand All @@ -925,9 +982,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)));
}
11 changes: 7 additions & 4 deletions src/AzureAppConfigurationOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@

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";

export const MaxRetries = 2;
export const MaxRetryDelayInMs = 60000;
import { StartupOptions } from "./StartupOptions.js";

export interface AzureAppConfigurationOptions {
/**
Expand Down Expand Up @@ -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.
*
Expand Down
Loading