Merge remote-tracking branch 'upstream/master' into pr416

.scripts/tsconfig.json

@@ -2,7 +2,16 @@
   "extends": "../tsconfig.json",
   "compilerOptions": {
     "noEmit": true,
-    "strict": true
+    "strict": true,
+    "lib": [
+      "dom",
+      "es5",
+      "es6",
+      "es7",
+      "esnext",
+      "esnext.asynciterable",
+      "es2015.iterable"
+    ]
   },
   "include": [
     "*.ts",

Changelog.md

@@ -3,6 +3,11 @@
 ## 2.3.0 - UNRELEASED
 - Moving @types dependencies into devdependencies
 
+## 2.2.2 - 2021-02-09
+- Port fix for nextLink issue from core-http (PR [#426](https://github.com/Azure/ms-rest-js/pull/426))
+- Fix abort signal event handler memory leak (PR [#425](https://github.com/Azure/ms-rest-js/pull/425))
+- Rework the use of `lib: ["dom"]` so consumers of this package don't need it in their tsconfig. Fixes (Issue [#367](https://github.com/Azure/ms-rest-js/issues/367))
+
 ## 2.2.1 - 2021-02-05
 - Fix issue of `SystemErrorRetryPolicy` didn't retry on errors (Issue [#412](https://github.com/Azure/ms-rest-js/issues/412))
 - `ThrottlingRetryPolicy` now keep retrying on 429 responses up to a limit. Fixes (Issue [#394](https://github.com/Azure/ms-rest-js/issues/394))

dom-shim.d.ts

@@ -0,0 +1,10 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+// d.ts shims provide types for things we use internally but are not part
+// of this package's surface area.
+
+interface Request {}
+interface RequestInit {}
+interface Response {}
+interface Headers {}

lib/dom.d.ts

@@ -0,0 +1,4 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+/// <reference lib="dom" />

lib/fetchHttpClient.ts

@@ -40,6 +40,7 @@ export abstract class FetchHttpClient implements HttpClient {
     }
 
     const abortController = new AbortController();
+    let abortListener: ((event: any) => void) | undefined;
     if (httpRequest.abortSignal) {
       if (httpRequest.abortSignal.aborted) {
         throw new RestError(
@@ -50,11 +51,12 @@ export abstract class FetchHttpClient implements HttpClient {
         );
       }
 
-      httpRequest.abortSignal.addEventListener("abort", (event: Event) => {
+      abortListener = (event: Event) => {
         if (event.type === "abort") {
           abortController.abort();
         }
-      });
+      };
+      httpRequest.abortSignal.addEventListener("abort", abortListener);
     }
 
     if (httpRequest.timeout) {
@@ -140,11 +142,12 @@ export abstract class FetchHttpClient implements HttpClient {
       ...platformSpecificRequestInit,
     };
 
+    let operationResponse: HttpOperationResponse | undefined;
     try {
-      const response: Response = await this.fetch(httpRequest.url, requestInit);
+      const response: CommonResponse = await this.fetch(httpRequest.url, requestInit);
 
       const headers = parseHeaders(response.headers);
-      const operationResponse: HttpOperationResponse = {
+      operationResponse = {
         headers: headers,
         request: httpRequest,
         status: response.status,
@@ -201,6 +204,24 @@ export abstract class FetchHttpClient implements HttpClient {
 
       throw fetchError;
     } finally {
+      // clean up event listener
+      if (httpRequest.abortSignal && abortListener) {
+        let uploadStreamDone = Promise.resolve();
+        if (isReadableStream(body)) {
+          uploadStreamDone = isStreamComplete(body);
+        }
+        let downloadStreamDone = Promise.resolve();
+        if (isReadableStream(operationResponse?.readableStreamBody)) {
+          downloadStreamDone = isStreamComplete(operationResponse!.readableStreamBody);
+        }
+
+        Promise.all([uploadStreamDone, downloadStreamDone])
+          .then(() => {
+            httpRequest.abortSignal?.removeEventListener("abort", abortListener!);
+            return;
+          })
+          .catch((_e) => {});
+      }
     }
   }
 
@@ -213,6 +234,14 @@ function isReadableStream(body: any): body is Readable {
   return body && typeof body.pipe === "function";
 }
 
+function isStreamComplete(stream: Readable): Promise<void> {
+  return new Promise((resolve) => {
+    stream.on("close", resolve);
+    stream.on("end", resolve);
+    stream.on("error", resolve);
+  });
+}
+
 export function parseHeaders(headers: Headers): HttpHeadersLike {
   const httpHeaders = new HttpHeaders();
 

lib/msRest.ts

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
+/// <reference path="../dom-shim.d.ts" />
+
 export {
   WebResource,
   WebResourceLike,

lib/url.ts

@@ -203,8 +203,12 @@ export class URLBuilder {
     if (!path) {
       this._path = undefined;
     } else {
-      if (path.indexOf("://") !== -1) {
-        this.set(path, "SCHEME");
+      const schemeIndex = path.indexOf("://");
+      if (schemeIndex !== -1) {
+        const schemeStart = path.lastIndexOf("/", schemeIndex);
+        // Make sure to only grab the URL part of the path before setting the state back to SCHEME
+        // this will handle cases such as "/a/b/c/https://microsoft.com" => "https://microsoft.com"
+        this.set(schemeStart === -1 ? path : path.substr(schemeStart + 1), "SCHEME");
       } else {
         this.set(path, "PATH");
       }

lib/util/constants.ts

@@ -7,7 +7,7 @@ export const Constants = {
    * @const
    * @type {string}
    */
-  msRestVersion: "2.2.1",
+  msRestVersion: "2.3.0",
 
   /**
    * Specifies HTTP.

package.json

@@ -35,7 +35,8 @@
     "es/lib/**/*.js.map",
     "es/lib/**/*.d.ts",
     "es/lib/**/*.d.ts.map",
-    "lib/**/*.ts",
+    "lib/**/!(dom.d).ts",
+    "dom-shim.d.ts",
     "LICENSE",
     "README.md",
     "ThirdPartyNotices.txt"
@@ -82,6 +83,7 @@
     "@types/xml2js": "^0.4.4",
     "abortcontroller-polyfill": "^1.3.0",
     "chai": "^4.2.0",
+    "cross-env": "^7.0.3",
     "express": "^4.17.1",
     "fetch-mock": "^7.3.3",
     "glob": "^7.1.4",
@@ -143,7 +145,7 @@
     "format": "prettier --write \"./**/*.ts\"",
     "test": "run-p test:tslint test:unit test:karma",
     "test:tslint": "tslint -p .",
-    "test:unit": "nyc mocha",
+    "test:unit": "cross-env TS_NODE_FILES=true nyc mocha",
     "test:karma": "npm run build:test-browser && node ./node_modules/karma/bin/karma start karma.conf.ts --browsers ChromeNoSecurity --single-run ",
     "test:karma:debug": "npm run build:test-browser && node ./node_modules/karma/bin/karma start karma.conf.ts  --log-level debug --browsers ChromeDebugging --debug --auto-watch",
     "test:karma:debugff": "npm run build:test-browser && node ./node_modules/karma/bin/karma start karma.conf.ts  --log-level debug --browsers FirefoxDebugging --debug --auto-watch",

test/urlTests.ts

@@ -998,6 +998,27 @@ describe("URLBuilder", () => {
   });
 
   describe("replaceAll()", () => {
+    it("should handle parametrized path containing a full url", () => {
+      const url = URLBuilder.parse("http://localhost");
+      url.appendPath("{nextLink}");
+      url.replaceAll("{nextLink}", "http://localhost:80/paging/multiple/page/2");
+
+      assert.strictEqual(url.toString(), "http://localhost:80/paging/multiple/page/2");
+    });
+
+    it("should handle parametrized path containing a full url, when path contains parts", () => {
+      const url = URLBuilder.parse("https://localhost/formrecognizer/v2.0-preview");
+      url.appendPath("{nextLink}");
+      url.replaceAll(
+        "{nextLink}",
+        "https://localhost/formrecognizer/v2.0-preview/custom/models?nextLink=1"
+      );
+
+      assert.strictEqual(
+        url.toString(),
+        "https://localhost/formrecognizer/v2.0-preview/custom/models?nextLink=1"
+      );
+    });
     it(`with undefined path, "{arg}" searchValue, and "cats" replaceValue`, () => {
       const urlBuilder = new URLBuilder();
       urlBuilder.setPath(undefined);

tsconfig.json

@@ -16,8 +16,6 @@
     "esModuleInterop": true,
     "allowSyntheticDefaultImports": true,
     "lib": [
-      "dom",
-      "dom.iterable",
       "es5",
       "es6",
       "es7",