Set same default retry options with other providers (#7)

Author: Eskibear

src/AzureAppConfigurationOptions.ts

@@ -4,6 +4,9 @@
 import { AppConfigurationClientOptions } from "@azure/app-configuration";
 import { AzureAppConfigurationKeyVaultOptions } from "./keyvault/AzureAppConfigurationKeyVaultOptions";
 
+export const MaxRetries = 2;
+export const MaxRetryDelayInMs = 60000;
+
 export interface AzureAppConfigurationOptions {
     selectors?: { keyFilter: string, labelFilter: string }[];
     trimKeyPrefixes?: string[];

src/Load.ts

@@ -5,7 +5,7 @@ import { AppConfigurationClient, AppConfigurationClientOptions } from "@azure/ap
 import { TokenCredential } from "@azure/identity";
 import { AzureAppConfiguration } from "./AzureAppConfiguration";
 import { AzureAppConfigurationImpl } from "./AzureAppConfigurationImpl";
-import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions";
+import { AzureAppConfigurationOptions, MaxRetries, MaxRetryDelayInMs } from "./AzureAppConfigurationOptions";
 
 export async function load(connectionString: string, options?: AzureAppConfigurationOptions): Promise<AzureAppConfiguration>;
 export async function load(endpoint: URL | string, credential: TokenCredential, options?: AzureAppConfigurationOptions): Promise<AzureAppConfiguration>;
@@ -52,9 +52,16 @@ function instanceOfTokenCredential(obj: unknown) {
     return obj && typeof obj === "object" && "getToken" in obj && typeof obj.getToken === "function";
 }
 
-function getClientOptions(options?: AzureAppConfigurationOptions): AppConfigurationClientOptions | undefined{
+function getClientOptions(options?: AzureAppConfigurationOptions): AppConfigurationClientOptions | undefined {
     // TODO: user-agent
     // TODO: set correlation context using additional policies
-    // TODO: allow override default retry options
-    return options?.clientOptions;
+    // retry options
+    const defaultRetryOptions = {
+        maxRetries: MaxRetries,
+        maxRetryDelayInMs: MaxRetryDelayInMs,
+    }
+    const retryOptions = Object.assign({}, defaultRetryOptions, options?.clientOptions?.retryOptions);
+    return Object.assign({}, options?.clientOptions, {
+        retryOptions
+    });
 }

test/clientOptions.test.js

@@ -0,0 +1,101 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+const chai = require("chai");
+const chaiAsPromised = require("chai-as-promised");
+chai.use(chaiAsPromised);
+const expect = chai.expect;
+const { load } = require("../dist/index");
+const { createMockedConnectionString } = require("./utils/testHelper");
+const nock = require('nock');
+
+class HttpRequestCountPolicy {
+    constructor() {
+        this.count = 0;
+        this.name = "HttpRequestCountPolicy";
+    }
+    sendRequest(req, next) {
+        this.count++;
+        return next(req).then(resp => { resp.status = 500; return resp; });
+    }
+    resetCount() {
+        this.count = 0;
+    }
+};
+
+describe("custom client options", function () {
+    const fakeEndpoint = "https://azure.azconfig.io";
+    before(() => {
+        // Thus here mock it to reply 500, in which case the retry mechanism works.
+        nock(fakeEndpoint).persist().get(() => true).reply(500);
+    });
+
+    after(() => {
+        nock.restore();
+    })
+
+    it("should retry 2 times by default", async () => {
+        const countPolicy = new HttpRequestCountPolicy();
+        const loadPromise = () => {
+            return load(createMockedConnectionString(fakeEndpoint), {
+                clientOptions: {
+                    additionalPolicies: [{
+                        policy: countPolicy,
+                        position: "perRetry"
+                    }]
+                }
+            })
+        };
+        let error;
+        try {
+            await loadPromise();
+        } catch (e) {
+            error = e;
+        }
+        expect(error).not.undefined;
+        expect(countPolicy.count).eq(3);
+    });
+
+    it("should override default retry options", async () => {
+        const countPolicy = new HttpRequestCountPolicy();
+        const loadWithMaxRetries = (maxRetries) => {
+            return load(createMockedConnectionString(fakeEndpoint), {
+                clientOptions: {
+                    additionalPolicies: [{
+                        policy: countPolicy,
+                        position: "perRetry"
+                    }],
+                    retryOptions: {
+                        maxRetries
+                    }
+                }
+            })
+        };
+
+        let error;
+        try {
+            error = undefined;
+            await loadWithMaxRetries(0);
+        } catch (e) {
+            error = e;
+        }
+        expect(error).not.undefined;
+        expect(countPolicy.count).eq(1);
+
+        countPolicy.resetCount();
+        try {
+            error = undefined;
+            await loadWithMaxRetries(1);
+        } catch (e) {
+            error = e;
+        }
+        expect(error).not.undefined;
+        expect(countPolicy.count).eq(2);
+    });
+
+    // Note:
+    // core-rest-pipeline skips the retry throwing `RestError: getaddrinfo ENOTFOUND azure.azconfig.io`
+    // Probably would be fixed in upstream libs.
+    // See https://github.com/Azure/azure-sdk-for-js/issues/27037
+    it("should retry on DNS failure");
+})