fetch browser unit tests

Author: kuhe

packages/fetch-http-handler/src/create-request.ts

@@ -0,0 +1,9 @@
+import { AdditionalRequestParameters } from "./fetch-http-handler";
+
+/**
+ * @internal
+ * For mocking/interception.
+ */
+export function createRequest(url: string, requestOptions?: RequestInit & AdditionalRequestParameters) {
+  return new Request(url, requestOptions);
+}

packages/fetch-http-handler/src/fetch-http-handler.browser.spec.ts

@@ -1,11 +1,18 @@
 import { HttpRequest } from "@smithy/protocol-http";
 import { QueryParameterBag } from "@smithy/types";
-import { afterEach, describe, expect, test as it, vi } from "vitest";
+import { afterEach, beforeAll, describe, expect, test as it, vi } from "vitest";
 
-import { FetchHttpHandler } from "./fetch-http-handler";
+import { createRequest } from "./create-request";
+import { FetchHttpHandler, keepAliveSupport } from "./fetch-http-handler";
 
-// TODO(vitest): fix this test.
-describe.skip(FetchHttpHandler.name, () => {
+vi.mock("./create-request", async () => {
+  const actual: any = await vi.importActual("./create-request");
+  return {
+    createRequest: vi.fn().mockImplementation(actual.createRequest),
+  };
+});
+
+describe(FetchHttpHandler.name, () => {
   interface MockHttpRequestOptions {
     method?: string;
     body?: any;
@@ -19,52 +26,53 @@ describe.skip(FetchHttpHandler.name, () => {
     new HttpRequest({ hostname: "example.com", ...options });
 
   describe("fetch", () => {
+    beforeAll(() => {
+      keepAliveSupport.supported = true;
+    });
+
     afterEach(() => {
       vi.clearAllMocks();
     });
 
     it("sends basic fetch request", async () => {
       const fetchHttpHandler = new FetchHttpHandler();
-      const winReqSpy = vi.spyOn(window, "Request");
 
       const mockHttpRequest = getMockHttpRequest({});
       await fetchHttpHandler.handle(mockHttpRequest);
 
       const expectedUrl = `${mockHttpRequest.protocol}//${mockHttpRequest.hostname}/`;
-      const requestArgs = winReqSpy.mock.calls[0][0];
+      const requestArgs = vi.mocked(createRequest).mock.calls[0];
+
       expect(requestArgs[0]).toEqual(expectedUrl);
-      expect(requestArgs[1].method).toEqual(mockHttpRequest.method);
-      expect(requestArgs[1].keepalive).toEqual(false);
+      expect(requestArgs[1]!.method).toEqual(mockHttpRequest.method);
+      expect(requestArgs[1]!.keepalive).toEqual(false);
     });
 
     for (const method of ["GET", "HEAD"]) {
       it(`sets body to undefined when method: '${method}'`, async () => {
         const fetchHttpHandler = new FetchHttpHandler();
-        const winReqSpy = vi.spyOn(window, "Request");
 
         const mockHttpRequest = getMockHttpRequest({ method, body: "test" });
         await fetchHttpHandler.handle(mockHttpRequest);
 
-        const requestArgs = winReqSpy.mock.calls[0][0];
-        expect(requestArgs[1].method).toEqual(mockHttpRequest.method);
-        expect(requestArgs[1].body).toEqual(undefined);
+        const requestArgs = vi.mocked(createRequest).mock.calls[0];
+        expect(requestArgs[1]!.method).toEqual(mockHttpRequest.method);
+        expect(requestArgs[1]!.body).toEqual(undefined);
       });
     }
 
     it(`sets keepalive to true if explicitly requested`, async () => {
       const fetchHttpHandler = new FetchHttpHandler({ keepAlive: true });
-      const winReqSpy = vi.spyOn(window, "Request");
 
       const mockHttpRequest = getMockHttpRequest({});
       await fetchHttpHandler.handle(mockHttpRequest);
 
-      const requestArgs = winReqSpy.mock.calls[0][0];
-      expect(requestArgs[1].keepalive).toEqual(true);
+      const requestArgs = vi.mocked(createRequest).mock.calls[0];
+      expect(requestArgs[1]!.keepalive).toEqual(true);
     });
 
     it(`builds querystring if provided`, async () => {
       const fetchHttpHandler = new FetchHttpHandler();
-      const winReqSpy = vi.spyOn(window, "Request");
 
       const query = { foo: "bar" };
       const fragment = "test";
@@ -74,22 +82,23 @@ describe.skip(FetchHttpHandler.name, () => {
       const expectedUrl = `${mockHttpRequest.protocol}//${mockHttpRequest.hostname}/?${Object.entries(query)
         .map(([key, val]) => `${key}=${val}`)
         .join("&")}#${fragment}`;
-      const requestArgs = winReqSpy.mock.calls[0][0];
+      const requestArgs = vi.mocked(createRequest).mock.calls[0];
       expect(requestArgs[0]).toEqual(expectedUrl);
     });
 
     it(`sets auth if username/password are provided`, async () => {
       const fetchHttpHandler = new FetchHttpHandler();
-      const winReqSpy = vi.spyOn(window, "Request");
 
       const username = "foo";
       const password = "bar";
       const mockHttpRequest = getMockHttpRequest({ username, password });
-      await fetchHttpHandler.handle(mockHttpRequest);
+      await fetchHttpHandler.handle(mockHttpRequest).catch(error => {
+        expect(String(error)).toContain("TypeError: Request cannot be constructed from a URL that includes credentials");
+      })
 
       const mockAuth = `${mockHttpRequest.username}:${mockHttpRequest.password}`;
       const expectedUrl = `${mockHttpRequest.protocol}//${mockAuth}@${mockHttpRequest.hostname}/`;
-      const requestArgs = winReqSpy.mock.calls[0][0];
+      const requestArgs = vi.mocked(createRequest).mock.calls[0];
       expect(requestArgs[0]).toEqual(expectedUrl);
     });
   });

packages/fetch-http-handler/src/fetch-http-handler.ts

@@ -3,6 +3,7 @@ import { buildQueryString } from "@smithy/querystring-builder";
 import type { FetchHttpHandlerOptions } from "@smithy/types";
 import { HeaderBag, HttpHandlerOptions, Provider } from "@smithy/types";
 
+import { createRequest } from "./create-request";
 import { requestTimeout } from "./request-timeout";
 
 declare let AbortController: any;
@@ -22,7 +23,7 @@ export const keepAliveSupport = {
 /**
  * @internal
  */
-type AdditionalRequestParameters = {
+export type AdditionalRequestParameters = {
   // This is required in Node.js when Request has a body, and does nothing in the browser.
   // Duplex: half means the request is fully transmitted before attempting to process the response.
   // As of writing this is the only accepted value in https://fetch.spec.whatwg.org/.
@@ -62,7 +63,7 @@ export class FetchHttpHandler implements HttpHandler<FetchHttpHandlerConfig> {
     }
     if (keepAliveSupport.supported === undefined) {
       keepAliveSupport.supported = Boolean(
-        typeof Request !== "undefined" && "keepalive" in new Request("https://[::1]")
+        typeof Request !== "undefined" && "keepalive" in createRequest("https://[::1]")
       );
     }
   }
@@ -139,7 +140,7 @@ export class FetchHttpHandler implements HttpHandler<FetchHttpHandlerConfig> {
 
     let removeSignalEventListener = () => {};
 
-    const fetchRequest = new Request(url, requestOptions);
+    const fetchRequest = createRequest(url, requestOptions);
     const raceOfPromises = [
       fetch(fetchRequest).then((response) => {
         const fetchHeaders: any = response.headers;