[core-rest-pipeline] Bug Fix - Retry logic to honor abort signal

sdk/core/core-rest-pipeline/CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### Bugs Fixed
 
+- [Bug #20778](https://github.com/Azure/azure-sdk-for-js/pull/20778) Customers can provide abort signals in the options bags for the client libraries but they were not being checked when requests were being retried. The issue is fixed in [#20781](https://github.com/Azure/azure-sdk-for-js/pull/20781).
+
 ### Other Changes
 
 - Changed the default number of retries from 10 to 3.

sdk/core/core-rest-pipeline/package.json

@@ -101,11 +101,13 @@
     "@microsoft/api-extractor": "^7.18.11",
     "@opentelemetry/api": "^1.0.1",
     "@types/chai": "^4.1.6",
+    "@types/chai-as-promised": "^7.1.0",
     "@types/mocha": "^7.0.2",
     "@types/node": "^12.0.0",
     "@types/sinon": "^9.0.4",
     "@types/uuid": "^8.0.0",
     "chai": "^4.2.0",
+    "chai-as-promised": "^7.1.1",
     "downlevel-dts": "^0.8.0",
     "cross-env": "^7.0.2",
     "eslint": "^7.15.0",
@@ -124,6 +126,7 @@
     "mocha-junit-reporter": "^2.0.0",
     "mocha": "^7.1.1",
     "prettier": "^2.5.1",
+    "puppeteer": "^13.5.1",
     "rimraf": "^3.0.0",
     "sinon": "^9.0.2",
     "source-map-support": "^0.5.9",

sdk/core/core-rest-pipeline/src/createPipelineFromOptions.ts

@@ -6,12 +6,12 @@ import { PipelineRetryOptions } from "./interfaces";
 import { Pipeline, createEmptyPipeline } from "./pipeline";
 import { decompressResponsePolicy } from "./policies/decompressResponsePolicy";
 import { formDataPolicy } from "./policies/formDataPolicy";
-import { logPolicy, LogPolicyOptions } from "./policies/logPolicy";
+import { LogPolicyOptions, logPolicy } from "./policies/logPolicy";
 import { proxyPolicy } from "./policies/proxyPolicy";
-import { redirectPolicy, RedirectPolicyOptions } from "./policies/redirectPolicy";
+import { RedirectPolicyOptions, redirectPolicy } from "./policies/redirectPolicy";
 import { setClientRequestIdPolicy } from "./policies/setClientRequestIdPolicy";
 import { tracingPolicy } from "./policies/tracingPolicy";
-import { userAgentPolicy, UserAgentPolicyOptions } from "./policies/userAgentPolicy";
+import { UserAgentPolicyOptions, userAgentPolicy } from "./policies/userAgentPolicy";
 import { defaultRetryPolicy } from "./policies/defaultRetryPolicy";
 import { isNode } from "./util/helpers";
 

sdk/core/core-rest-pipeline/src/nodeHttpClient.ts

@@ -8,11 +8,11 @@ import { Transform } from "stream";
 import { AbortController, AbortError } from "@azure/abort-controller";
 import {
   HttpClient,
+  HttpHeaders,
   PipelineRequest,
   PipelineResponse,
-  TransferProgressEvent,
-  HttpHeaders,
   RequestBodyType,
+  TransferProgressEvent,
 } from "./interfaces";
 import { createHttpHeaders } from "./httpHeaders";
 import { RestError } from "./restError";

sdk/core/core-rest-pipeline/src/pipeline.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineRequest, PipelineResponse, HttpClient, SendRequest } from "./interfaces";
+import { HttpClient, PipelineRequest, PipelineResponse, SendRequest } from "./interfaces";
 
 /**
  * Policies are executed in phases.

sdk/core/core-rest-pipeline/src/pipelineRequest.ts

@@ -2,13 +2,13 @@
 // Licensed under the MIT license.
 
 import {
-  PipelineRequest,
-  TransferProgressEvent,
-  RequestBodyType,
-  HttpMethods,
-  HttpHeaders,
   FormDataMap,
+  HttpHeaders,
+  HttpMethods,
+  PipelineRequest,
   ProxySettings,
+  RequestBodyType,
+  TransferProgressEvent,
 } from "./interfaces";
 import { createHttpHeaders } from "./httpHeaders";
 import { AbortSignalLike } from "@azure/abort-controller";

sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts

@@ -1,9 +1,9 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-auth";
+import { AccessToken, GetTokenOptions, TokenCredential } from "@azure/core-auth";
 import { AzureLogger } from "@azure/logger";
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 import { createTokenCycler } from "../util/tokenCycler";
 

sdk/core/core-rest-pipeline/src/policies/decompressResponsePolicy.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 
 /**

sdk/core/core-rest-pipeline/src/policies/formDataPolicy.browser.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 
 /**

sdk/core/core-rest-pipeline/src/policies/formDataPolicy.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT license.
 
 import FormData from "form-data";
-import { PipelineResponse, PipelineRequest, SendRequest, FormDataMap } from "../interfaces";
+import { FormDataMap, PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 
 /**

sdk/core/core-rest-pipeline/src/policies/logPolicy.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT license.
 
 import { Debugger } from "@azure/logger";
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 import { logger as coreLogger } from "../log";
 import { Sanitizer } from "../util/sanitizer";

sdk/core/core-rest-pipeline/src/policies/ndJsonPolicy.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 
 /**

sdk/core/core-rest-pipeline/src/policies/proxyPolicy.ts

@@ -6,11 +6,11 @@ import * as https from "https";
 import { HttpsProxyAgent, HttpsProxyAgentOptions } from "https-proxy-agent";
 import { HttpProxyAgent, HttpProxyAgentOptions } from "http-proxy-agent";
 import {
-  PipelineResponse,
+  HttpHeaders,
   PipelineRequest,
-  SendRequest,
+  PipelineResponse,
   ProxySettings,
-  HttpHeaders,
+  SendRequest,
 } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 import { URL } from "../util/url";

sdk/core/core-rest-pipeline/src/policies/redirectPolicy.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 import { URL } from "../util/url";
 

sdk/core/core-rest-pipeline/src/policies/retryPolicy.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 import { delay } from "../util/helpers";
 import { createClientLogger } from "@azure/logger";
@@ -113,7 +113,7 @@ export function retryPolicy(
             strategyLogger.info(
               `Retry ${retryCount}: Retry strategy ${strategy.name} retries after ${retryAfterInMs}`
             );
-            await delay(retryAfterInMs);
+            await delay(retryAfterInMs, undefined, { abortSignal: request.abortSignal });
             continue retryRequest;
           }
 

sdk/core/core-rest-pipeline/src/policies/setClientRequestIdPolicy.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 
 /**

sdk/core/core-rest-pipeline/src/policies/tracingPolicy.ts

@@ -2,15 +2,15 @@
 // Licensed under the MIT license.
 
 import {
-  getTraceParentHeader,
-  createSpanFunction,
-  SpanStatusCode,
-  isSpanContextValid,
   Span,
   SpanOptions,
+  SpanStatusCode,
+  createSpanFunction,
+  getTraceParentHeader,
+  isSpanContextValid,
 } from "@azure/core-tracing";
 import { SpanKind } from "@azure/core-tracing";
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
 import { getUserAgentValue } from "../util/userAgent";
 import { logger } from "../log";

sdk/core/core-rest-pipeline/src/policies/userAgentPolicy.ts

@@ -1,9 +1,9 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
+import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces";
 import { PipelinePolicy } from "../pipeline";
-import { getUserAgentValue, getUserAgentHeaderName } from "../util/userAgent";
+import { getUserAgentHeaderName, getUserAgentValue } from "../util/userAgent";
 
 const UserAgentHeaderName = getUserAgentHeaderName();
 

sdk/core/core-rest-pipeline/src/restError.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineResponse, PipelineRequest } from "./interfaces";
+import { PipelineRequest, PipelineResponse } from "./interfaces";
 import { custom } from "./util/inspect";
 import { Sanitizer } from "./util/sanitizer";
 

sdk/core/core-rest-pipeline/src/util/helpers.ts

@@ -1,22 +1,71 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
+import { AbortError, AbortSignalLike } from "@azure/abort-controller";
+
 /**
  * A constant that indicates whether the environment the code is running is Node.JS.
  * @internal
  */
 export const isNode =
   typeof process !== "undefined" && Boolean(process.version) && Boolean(process.versions?.node);
 
+const StandardAbortMessage = "The operation was aborted.";
+
 /**
- * A wrapper for setTimeout that resolves a promise after t milliseconds.
- * @internal
- * @param t - The number of milliseconds to be delayed.
+ * A wrapper for setTimeout that resolves a promise after delayInMs milliseconds.
+ * @param delayInMs - The number of milliseconds to be delayed.
  * @param value - The value to be resolved with after a timeout of t milliseconds.
+ * @param options - The options for delay - currently abort options
+ *                  - abortSignal - The abortSignal associated with containing operation.
+ *                  - abortErrorMsg - The abort error message associated with containing operation.
  * @returns Resolved promise
  */
-export function delay<T>(t: number, value?: T): Promise<T | void> {
-  return new Promise((resolve) => setTimeout(() => resolve(value), t));
+export function delay<T>(
+  delayInMs: number,
+  value?: T,
+  options?: {
+    abortSignal?: AbortSignalLike;
+    abortErrorMsg?: string;
+  }
+): Promise<T | void> {
+  return new Promise((resolve, reject) => {
+    let timer: ReturnType<typeof setTimeout> | undefined = undefined;
+    let onAborted: (() => void) | undefined = undefined;
+
+    const rejectOnAbort = (): void => {
+      return reject(
+        new AbortError(options?.abortErrorMsg ? options?.abortErrorMsg : StandardAbortMessage)
+      );
+    };
+
+    const removeListeners = (): void => {
+      if (options?.abortSignal && onAborted) {
+        options.abortSignal.removeEventListener("abort", onAborted);
+      }
+    };
+
+    onAborted = (): void => {
+      if (timer) {
+        clearTimeout(timer);
+      }
+      removeListeners();
+      return rejectOnAbort();
+    };
+
+    if (options?.abortSignal && options.abortSignal.aborted) {
+      return rejectOnAbort();
+    }
+
+    timer = setTimeout(() => {
+      removeListeners();
+      resolve(value);
+    }, delayInMs);
+
+    if (options?.abortSignal) {
+      options.abortSignal.addEventListener("abort", onAborted);
+    }
+  });
 }
 
 /**

sdk/core/core-rest-pipeline/src/util/sanitizer.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { isObject, UnknownObject } from "./helpers";
+import { UnknownObject, isObject } from "./helpers";
 import { URL } from "./url";
 
 /**

sdk/core/core-rest-pipeline/src/util/userAgent.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { setPlatformSpecificData, getHeaderName } from "./userAgentPlatform";
+import { getHeaderName, setPlatformSpecificData } from "./userAgentPlatform";
 import { SDK_VERSION } from "../constants";
 
 function getUserAgentString(telemetryInfo: Map<string, string>): string {

sdk/core/core-rest-pipeline/test/bearerTokenAuthenticationPolicy.spec.ts

@@ -3,14 +3,14 @@
 
 import { assert } from "chai";
 import * as sinon from "sinon";
-import { TokenCredential, AccessToken } from "@azure/core-auth";
+import { AccessToken, TokenCredential } from "@azure/core-auth";
 import {
   PipelinePolicy,
-  createPipelineRequest,
-  createHttpHeaders,
   PipelineResponse,
-  bearerTokenAuthenticationPolicy,
   SendRequest,
+  bearerTokenAuthenticationPolicy,
+  createHttpHeaders,
+  createPipelineRequest,
 } from "../src";
 import { DEFAULT_CYCLER_OPTIONS } from "../src/util/tokenCycler";