Avoid mutating global fetch

Porting fix from https://github.com/Azure/azure-sdk-for-js/pull/9880.
Use import node_fetch from "node-fetch" instead of import "node-fetch".

Resolves #383

Changelog.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 2.2.1 - (Unreleased)
+
+- The global `fetch()` is no longer overridden by node-fetch. Fixes (Issue [#383](https://github.com/Azure/ms-rest-js/issues/383))
+
 ## 2.2.0 - 2021-01-26
 - Add support for @azure/core-auth's TokenCredential (PR [#410](https://github.com/Azure/ms-rest-js/pull/410))
 - Allow = character in parameter value (PR [#408](https://github.com/Azure/ms-rest-js/pull/408))

lib/browserFetchHttpClient.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
-import { FetchHttpClient } from "./fetchHttpClient";
+import { CommonRequestInfo, CommonRequestInit, CommonResponse, FetchHttpClient } from "./fetchHttpClient";
 import { HttpOperationResponse } from "./httpOperationResponse";
 import { WebResourceLike } from "./webResource";
 
@@ -14,7 +14,7 @@ export class BrowserFetchHttpClient extends FetchHttpClient {
     return Promise.resolve();
   }
 
-  fetch(input: RequestInfo, init?: RequestInit): Promise<Response> {
+  fetch(input: CommonRequestInfo, init?: CommonRequestInit): Promise<CommonResponse> {
     return fetch(input, init);
   }
 }

lib/fetchHttpClient.ts

@@ -17,6 +17,20 @@ interface FetchError extends Error {
   type?: string;
 }
 
+export type CommonRequestInfo = string; // we only call fetch() on string urls.
+
+export type CommonRequestInit = Omit<RequestInit, "body" | "headers" | "signal"> & {
+  body?: any;
+  headers?: any;
+  signal?: any;
+};
+
+export type CommonResponse = Omit<Response, "body" | "trailer" | "formData"> & {
+  body: any;
+  trailer: any;
+  formData: any;
+};
+
 export abstract class FetchHttpClient implements HttpClient {
   async sendRequest(httpRequest: WebResourceLike): Promise<HttpOperationResponse> {
     if (!httpRequest && typeof httpRequest !== "object") {
@@ -166,7 +180,7 @@ export abstract class FetchHttpClient implements HttpClient {
 
   abstract async prepareRequest(httpRequest: WebResourceLike): Promise<Partial<RequestInit>>;
   abstract async processRequest(operationResponse: HttpOperationResponse): Promise<void>;
-  abstract async fetch(input: RequestInfo, init?: RequestInit): Promise<Response>;
+  abstract async fetch(input: CommonRequestInfo, init?: CommonRequestInit): Promise<CommonResponse>;
 }
 
 function isReadableStream(body: any): body is Readable {

lib/nodeFetchHttpClient.ts

@@ -4,29 +4,18 @@
 import * as tough from "tough-cookie";
 import * as http from "http";
 import * as https from "https";
-import "node-fetch";
+import node_fetch from "node-fetch";
 
-import { FetchHttpClient } from "./fetchHttpClient";
+import { CommonRequestInfo, CommonRequestInit, CommonResponse, FetchHttpClient } from "./fetchHttpClient";
 import { HttpOperationResponse } from "./httpOperationResponse";
 import { WebResourceLike } from "./webResource";
 import { createProxyAgent, ProxyAgent } from "./proxyAgent";
 
-interface GlobalWithFetch extends NodeJS.Global {
-  fetch: (input: RequestInfo, init?: RequestInit) => Promise<Response>;
-}
-
-const globalWithFetch = global as GlobalWithFetch;
-if (typeof globalWithFetch.fetch !== "function") {
-  const fetch = require("node-fetch").default;
-  globalWithFetch.fetch = fetch;
-}
-
-
 export class NodeFetchHttpClient extends FetchHttpClient {
   private readonly cookieJar = new tough.CookieJar(undefined, { looseMode: true });
 
-  async fetch(input: RequestInfo, init?: RequestInit): Promise<Response> {
-    return fetch(input, init);
+  async fetch(input: CommonRequestInfo, init?: CommonRequestInit): Promise<CommonResponse> {
+    return node_fetch(input, init) as unknown as Promise<CommonResponse>;
   }
 
   async prepareRequest(httpRequest: WebResourceLike): Promise<Partial<RequestInit>> {

test/defaultHttpClientTests.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 import { assert, AssertionError } from "chai";
+import * as sinon from "sinon";
 import "chai/register-should";
 import { createReadStream } from "fs";
 
@@ -11,6 +12,7 @@ import { isNode } from "../lib/util/utils";
 import { WebResource, HttpRequestBody, TransferProgressEvent } from "../lib/webResource";
 import { getHttpMock, HttpMockFacade } from "./mockHttp";
 import { TestFunction } from "mocha";
+import { CommonResponse } from "../lib/fetchHttpClient";
 
 const nodeIt = (isNode ? it : it.skip) as TestFunction;
 
@@ -38,6 +40,20 @@ describe("defaultHttpClient", function () {
   afterEach(() => httpMock.teardown());
   after(() => httpMock.teardown());
 
+  function getMockedHttpClient(): DefaultHttpClient {
+    const httpClient = new DefaultHttpClient();
+    const fetchMock = httpMock.getFetch();
+    if (fetchMock) {
+      sinon.stub(httpClient, "fetch").callsFake(async (input, init) => {
+        const response = await fetchMock(input, init);
+        return (response as unknown) as CommonResponse;
+      });
+    }
+
+    return httpClient;
+  }
+
+
   it("should return a response instead of throwing for awaited 404", async function () {
     const resourceUrl = "/nonexistent";
 
@@ -46,7 +62,7 @@ describe("defaultHttpClient", function () {
     });
 
     const request = new WebResource(resourceUrl, "GET");
-    const httpClient = new DefaultHttpClient();
+    const httpClient = getMockedHttpClient();
 
     const response = await httpClient.sendRequest(request);
     response.status.should.equal(404);
@@ -62,7 +78,7 @@ describe("defaultHttpClient", function () {
     const controller = getAbortController();
     const veryBigPayload = "very long string";
     const request = new WebResource(resourceUrl, "POST", veryBigPayload, undefined, undefined, true, undefined, controller.signal);
-    const client = new DefaultHttpClient();
+    const client = getMockedHttpClient();
     const promise = client.sendRequest(request);
     controller.abort();
     try {
@@ -89,7 +105,7 @@ describe("defaultHttpClient", function () {
       };
     });
 
-    const client = new DefaultHttpClient();
+    const client = getMockedHttpClient();
 
     const request1 = new WebResource("http://my.fake.domain/set-cookie");
     const response1 = await client.sendRequest(request1);
@@ -117,7 +133,7 @@ describe("defaultHttpClient", function () {
       new WebResource("/fileupload", "POST", buf, undefined, undefined, true, undefined, controller.signal),
       new WebResource("/fileupload", "POST", buf, undefined, undefined, true, undefined, controller.signal)
     ];
-    const client = new DefaultHttpClient();
+    const client = getMockedHttpClient();
     const promises = requests.map(r => client.sendRequest(r));
     controller.abort();
     // Ensure each promise is individually rejected
@@ -154,7 +170,7 @@ describe("defaultHttpClient", function () {
         ev => listener(upload, ev),
         ev => listener(download, ev));
 
-      const client = new DefaultHttpClient();
+      const client = getMockedHttpClient();
       const response = await client.sendRequest(request);
       response.should.exist;
       response.status.should.equal(251);
@@ -183,7 +199,7 @@ describe("defaultHttpClient", function () {
         ev => listener(upload, ev),
         ev => listener(download, ev));
 
-      const client = new DefaultHttpClient();
+      const client = getMockedHttpClient();
       const response = await client.sendRequest(request);
       response.status.should.equal(250);
       if (response.blobBody) {
@@ -206,7 +222,7 @@ describe("defaultHttpClient", function () {
     httpMock.timeout("GET", "/slow");
 
     const request = new WebResource("/slow", "GET", undefined, undefined, undefined, false, false, undefined, 100);
-    const client = new DefaultHttpClient();
+    const client = getMockedHttpClient();
     try {
       await client.sendRequest(request);
       throw new Error("request did not fail as expected");
@@ -217,8 +233,9 @@ describe("defaultHttpClient", function () {
 
   it("should give a graceful error for nonexistent hosts", async function () {
     const requestUrl = "http://fake.domain";
-    httpMock.passThrough();
     const request = new WebResource(requestUrl, "GET");
+    httpMock.passThrough();
+    // testing the unstubbed behavior so not using local mock
     const client = new DefaultHttpClient();
     try {
       await client.sendRequest(request);
@@ -245,17 +262,18 @@ describe("defaultHttpClient", function () {
     });
 
     const request = new WebResource(requestUrl, "PUT");
-    const client = new DefaultHttpClient();
+    const client = getMockedHttpClient();
     const response = await client.sendRequest(request);
     response.status.should.equal(200, response.bodyAsText!);
   });
 
   it("should send HTTP requests", async function () {
-    httpMock.passThrough();
     const request = new WebResource("https://example.com", "GET");
     request.headers.set("Access-Control-Allow-Headers", "Content-Type");
     request.headers.set("Access-Control-Allow-Methods", "GET");
     request.headers.set("Access-Control-Allow-Origin", "https://example.com");
+    httpMock.passThrough();
+    // testing the unstubbed behavior so not using local mock
     const httpClient = new DefaultHttpClient();
 
     const response = await httpClient.sendRequest(request);

test/mockHttp.ts

@@ -5,6 +5,7 @@ import xhrMock, { proxy } from "xhr-mock";
 import { isNode, HttpMethods } from "../lib/msRest";
 import fetchMock, * as fetch from "fetch-mock";
 import { Readable } from "stream";
+import node_fetch from "node-fetch";
 
 export type UrlFilter = string | RegExp;
 
@@ -27,6 +28,7 @@ export interface HttpMockFacade {
   get(url: UrlFilter, response: MockResponse): void;
   post(url: UrlFilter, response: MockResponse): void;
   put(url: UrlFilter, response: MockResponse): void;
+  getFetch(): typeof node_fetch | undefined;
 }
 
 export function getHttpMock(): HttpMockFacade {
@@ -34,16 +36,26 @@ export function getHttpMock(): HttpMockFacade {
 }
 
 class FetchHttpMock implements HttpMockFacade {
+  private _fetch: fetchMock.FetchMockSandbox;
+
+  constructor() {
+    this._fetch = fetchMock.sandbox();
+  }
+
+  getFetch(): typeof node_fetch {
+    return this._fetch as unknown as typeof node_fetch;
+  }
+
   setup(): void {
-    fetchMock.resetHistory();
+    this._fetch.resetHistory();
   }
 
   teardown(): void {
-    fetchMock.resetHistory();
+    this._fetch.resetHistory();
   }
 
   passThrough(_url?: string | RegExp | undefined): void {
-    fetchMock.reset();
+    this._fetch.reset();
   }
 
   timeout(_method: HttpMethods, url: UrlFilter): void {
@@ -51,7 +63,7 @@ class FetchHttpMock implements HttpMockFacade {
       setTimeout(() => resolve({$uri: url, delay: 500}), 2500);
     });
 
-    fetchMock.mock(url, delay);
+    this._fetch.mock(url, delay);
   }
 
   convertStreamToBuffer(stream: Readable): Promise<any> {
@@ -83,7 +95,7 @@ class FetchHttpMock implements HttpMockFacade {
     }
 
     const matcher = (_url: string, opts: fetch.MockRequest) => (url === _url) && (opts.method === method);
-    fetchMock.mock(matcher, mockResponse);
+    this._fetch.mock(matcher, mockResponse);
   }
 
   get(url: UrlFilter, response: MockResponse): void {
@@ -145,5 +157,9 @@ export class BrowserHttpMock implements HttpMockFacade {
   timeout(method: HttpMethods, url: UrlFilter): void {
     return this.mockHttpMethod(method, url, () => new Promise(() => { }));
   }
+
+  getFetch(): undefined {
+    return undefined;
+  }
 }