[App Config] Handle throttling - do not hang - should honor abort signal

sdk/appconfiguration/app-configuration/CHANGELOG.md

@@ -10,6 +10,9 @@
 
 ### Fixed
 
+- High request rate would result in throttling. SDK would retry on the failed requests based on the service suggested time from the `retry-after-ms` header in the error response. If there are too many parallel requests, retries for all of them may also result in a high request rate entering into a state which might seem like the application is hanging forever.
+  - [#15721](https://github.com/Azure/azure-sdk-for-js/pull/15721) allows the user-provided abortSignal to be taken into account to abort the requests sooner.
+  - More resources - [App Configuration | Throttling](https://docs.microsoft.com/en-us/azure/azure-app-configuration/rest-api-throttling) and [App Configuration | Requests Quota](https://docs.microsoft.com/en-us/azure/azure-app-configuration/faq#which-app-configuration-tier-should-i-use)
 
 ## 1.2.0-beta.2 (2021-06-08)
 

sdk/appconfiguration/app-configuration/src/policies/throttlingRetryPolicy.ts

@@ -83,7 +83,7 @@ export async function waitForTimeoutOrAbortOrResolve<T>(args: {
  *
  * @internal
  */
-export function checkAndRegisterWithAbortSignal(
+function checkAndRegisterWithAbortSignal(
   onAbortFn: (abortError: AbortError) => void,
   abortSignal?: AbortSignalLike
 ): () => void {
@@ -131,10 +131,8 @@ export class ThrottlingRetryPolicy extends BaseRequestPolicy {
         return await waitForTimeoutOrAbortOrResolve({
           timeoutMs: delayInMs,
           abortSignal: httpRequest.abortSignal,
-          actionFn: async () => {
-            return await this.sendRequest(httpRequest.clone());
-          },
-          timeoutMessage: `ServiceBusy: Unable to fulfill the request in ${delayInMs}ms when retried.`
+          actionFn: () => this.sendRequest(httpRequest.clone()),
+          timeoutMessage: `Unable to fulfill the request in ${delayInMs}ms when retried.`
         });
       } else {
         throw err;

sdk/appconfiguration/app-configuration/test/internal/throttlingRetryPolicyTests.spec.ts

@@ -13,6 +13,10 @@ import {
 } from "@azure/core-http";
 import { ThrottlingRetryPolicy, getDelayInMs } from "../../src/policies/throttlingRetryPolicy";
 import { assertThrowsRestError } from "../public/utils/testHelpers";
+import { AppConfigurationClient } from "../../src";
+import { AbortController } from "@azure/abort-controller";
+import nock from "nock";
+import { generateUuid } from "@azure/core-http";
 
 describe("ThrottlingRetryPolicy", () => {
   class PassThroughPolicy {
@@ -188,3 +192,57 @@ describe("ThrottlingRetryPolicy", () => {
     });
   });
 });
+
+describe("Should not retry forever - honors the abort signal passed", () => {
+  let client: AppConfigurationClient;
+  const connectionString = "Endpoint=https://myappconfig.azconfig.io;Id=key:ai/u/fake;Secret=abcd=";
+
+  beforeEach(function() {
+    if (!nock.isActive()) {
+      nock.activate();
+    }
+    nock("https://myappconfig.azconfig.io:443")
+      .persist()
+      .put(/.*/g)
+      .reply(
+        429,
+        {
+          type: "https://azconfig.io/errors/too-many-requests",
+          title: "Resource utilization has surpassed the assigned quota",
+          policy: "Total Requests",
+          status: 429
+        },
+        ["retry-after-ms", "123456"]
+      );
+
+    client = new AppConfigurationClient(connectionString);
+  });
+
+  afterEach(async function() {
+    nock.restore();
+    nock.cleanAll();
+    nock.enableNetConnect();
+  });
+
+  it("simulate the service throttling", async () => {
+    const key = generateUuid();
+    const numberOfSettings = 200;
+    const promises = [];
+    try {
+      for (let index = 0; index < numberOfSettings; index++) {
+        promises.push(
+          client.addConfigurationSetting(
+            {
+              key: key + "-" + index,
+              value: "added"
+            },
+            {
+              abortSignal: AbortController.timeout(1000)
+            }
+          )
+        );
+      }
+      await Promise.all(promises);
+    } catch (_) {}
+  });
+});