fix(node-http-handler): resolve config provider only once per NodeHttpHandler instance

.gitignore

@@ -20,6 +20,7 @@ jspm_packages
 .DS_Store
 .vscode/launch.json
 
+Makefile
 lerna-debug.log
 package-lock.json
 

packages/node-http-handler/src/node-http-handler.spec.ts

@@ -1,9 +1,7 @@
 import { AbortController } from "@aws-sdk/abort-controller";
 import { HttpRequest } from "@aws-sdk/protocol-http";
-import { Server as HttpServer } from "http";
-import http from "http";
-import { Server as HttpsServer } from "https";
-import https from "https";
+import http, { Server as HttpServer } from "http";
+import https, { Server as HttpsServer } from "https";
 import { AddressInfo } from "net";
 
 import { NodeHttpHandler } from "./node-http-handler";
@@ -16,7 +14,7 @@ import {
 } from "./server.mock";
 
 describe("NodeHttpHandler", () => {
-  describe("constructor", () => {
+  describe("constructor and #handle", () => {
     let hRequestSpy: jest.SpyInstance;
     let hsRequestSpy: jest.SpyInstance;
     const randomMaxSocket = Math.round(Math.random() * 50) + 1;
@@ -38,55 +36,87 @@ describe("NodeHttpHandler", () => {
       hRequestSpy.mockRestore();
       hsRequestSpy.mockRestore();
     });
+    describe("constructor", () => {
+      it.each([
+        ["empty", undefined],
+        ["a provider", async () => {}],
+      ])("sets keepAlive=true by default when input is %s", async (_, option) => {
+        const nodeHttpHandler = new NodeHttpHandler(option);
+        await nodeHttpHandler.handle({} as any);
+        expect(hRequestSpy.mock.calls[0][0]?.agent.keepAlive).toEqual(true);
+      });
 
-    it.each([
-      ["empty", undefined],
-      ["a provider", async () => {}],
-    ])("sets keepAlive=true by default when input is %s", async (_, option) => {
-      const nodeHttpHandler = new NodeHttpHandler(option);
-      await nodeHttpHandler.handle({} as any);
-      expect(hRequestSpy.mock.calls[0][0]?.agent.keepAlive).toEqual(true);
-    });
+      it.each([
+        ["empty", undefined],
+        ["a provider", async () => {}],
+      ])("sets maxSockets=50 by default when input is %s", async (_, option) => {
+        const nodeHttpHandler = new NodeHttpHandler(option);
+        await nodeHttpHandler.handle({} as any);
+        expect(hRequestSpy.mock.calls[0][0]?.agent.maxSockets).toEqual(50);
+      });
 
-    it.each([
-      ["empty", undefined],
-      ["a provider", async () => {}],
-    ])("sets maxSockets=50 by default when input is %s", async (_, option) => {
-      const nodeHttpHandler = new NodeHttpHandler(option);
-      await nodeHttpHandler.handle({} as any);
-      expect(hRequestSpy.mock.calls[0][0]?.agent.maxSockets).toEqual(50);
-    });
+      it.each([
+        ["an options hash", { httpAgent: new http.Agent({ keepAlive: false, maxSockets: randomMaxSocket }) }],
+        [
+          "a provider",
+          async () => ({
+            httpAgent: new http.Agent({ keepAlive: false, maxSockets: randomMaxSocket }),
+          }),
+        ],
+      ])("sets httpAgent when input is %s", async (_, option) => {
+        const nodeHttpHandler = new NodeHttpHandler(option);
+        await nodeHttpHandler.handle({ protocol: "http:", headers: {}, method: "GET", hostname: "localhost" } as any);
+        expect(hRequestSpy.mock.calls[0][0]?.agent.keepAlive).toEqual(false);
+        expect(hRequestSpy.mock.calls[0][0]?.agent.maxSockets).toEqual(randomMaxSocket);
+      });
 
-    it.each([
-      ["an options hash", { httpAgent: new http.Agent({ keepAlive: false, maxSockets: randomMaxSocket }) }],
-      [
-        "a provider",
-        async () => ({
-          httpAgent: new http.Agent({ keepAlive: false, maxSockets: randomMaxSocket }),
-        }),
-      ],
-    ])("sets httpAgent when input is %s", async (_, option) => {
-      const nodeHttpHandler = new NodeHttpHandler(option);
-      await nodeHttpHandler.handle({ protocol: "http:", headers: {}, method: "GET", hostname: "localhost" } as any);
-      expect(hRequestSpy.mock.calls[0][0]?.agent.keepAlive).toEqual(false);
-      expect(hRequestSpy.mock.calls[0][0]?.agent.maxSockets).toEqual(randomMaxSocket);
+      it.each([
+        ["an option hash", { httpsAgent: new https.Agent({ keepAlive: true, maxSockets: randomMaxSocket }) }],
+        [
+          "a provider",
+          async () => ({
+            httpsAgent: new https.Agent({ keepAlive: true, maxSockets: randomMaxSocket }),
+          }),
+        ],
+      ])("sets httpsAgent when input is %s", async (_, option) => {
+        const nodeHttpHandler = new NodeHttpHandler(option);
+        await nodeHttpHandler.handle({ protocol: "https:" } as any);
+        expect(hsRequestSpy.mock.calls[0][0]?.agent.keepAlive).toEqual(true);
+        expect(hsRequestSpy.mock.calls[0][0]?.agent.maxSockets).toEqual(randomMaxSocket);
+      });
     });
 
-    it.each([
-      ["an option hash", { httpsAgent: new https.Agent({ keepAlive: true, maxSockets: randomMaxSocket }) }],
-      [
-        "a provider",
-        async () => ({
-          httpsAgent: new https.Agent({ keepAlive: true, maxSockets: randomMaxSocket }),
-        }),
-      ],
-    ])("sets httpsAgent when input is %s", async (_, option) => {
-      const nodeHttpHandler = new NodeHttpHandler(option);
-      await nodeHttpHandler.handle({ protocol: "https:" } as any);
-      expect(hsRequestSpy.mock.calls[0][0]?.agent.keepAlive).toEqual(true);
-      expect(hsRequestSpy.mock.calls[0][0]?.agent.maxSockets).toEqual(randomMaxSocket);
+    describe("#handle", () => {
+      it("should only generate a single config when the config provider is async and it is not ready yet", async () => {
+        let providerInvokedCount = 0;
+        let providerResolvedCount = 0;
+        const slowConfigProvider = async () => {
+          providerInvokedCount += 1;
+          await new Promise((r) => setTimeout(r, 15));
+          providerResolvedCount += 1;
+          return {
+            connectionTimeout: 12345,
+            socketTimeout: 12345,
+            httpAgent: null,
+            httpsAgent: null,
+          };
+        };
+
+        const nodeHttpHandler = new NodeHttpHandler(slowConfigProvider);
+
+        const promises = Promise.all(
+          Array.from({ length: 20 }).map(() => nodeHttpHandler.handle({} as unknown as HttpRequest))
+        );
+
+        expect(providerInvokedCount).toBe(1);
+        expect(providerResolvedCount).toBe(0);
+        await promises;
+        expect(providerInvokedCount).toBe(1);
+        expect(providerResolvedCount).toBe(1);
+      });
     });
   });
+
   describe("http", () => {
     const mockHttpServer: HttpServer = createMockHttpServer().listen(54321);
 

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

@@ -39,18 +39,23 @@ interface ResolvedNodeHttpHandlerConfig {
 
 export class NodeHttpHandler implements HttpHandler {
   private config?: ResolvedNodeHttpHandlerConfig;
-  private readonly configProvider?: Provider<ResolvedNodeHttpHandlerConfig>;
+  private readonly configProvider: Promise<ResolvedNodeHttpHandlerConfig>;
+
   // Node http handler is hard-coded to http/1.1: https://github.com/nodejs/node/blob/ff5664b83b89c55e4ab5d5f60068fb457f1f5872/lib/_http_server.js#L286
   public readonly metadata = { handlerProtocol: "http/1.1" };
 
   constructor(options?: NodeHttpHandlerOptions | Provider<NodeHttpHandlerOptions | void>) {
-    if (typeof options === "function") {
-      this.configProvider = async () => {
-        return this.resolveDefaultConfig(await options());
-      };
-    } else {
-      this.config = this.resolveDefaultConfig(options);
-    }
+    this.configProvider = new Promise((resolve, reject) => {
+      if (typeof options === "function") {
+        options()
+          .then((_options) => {
+            resolve(this.resolveDefaultConfig(_options));
+          })
+          .catch(reject);
+      } else {
+        resolve(this.resolveDefaultConfig(options));
+      }
+    });
   }
 
   private resolveDefaultConfig(options?: NodeHttpHandlerOptions | void): ResolvedNodeHttpHandlerConfig {
@@ -71,9 +76,8 @@ export class NodeHttpHandler implements HttpHandler {
   }
 
   async handle(request: HttpRequest, { abortSignal }: HttpHandlerOptions = {}): Promise<{ response: HttpResponse }> {
-    if (!this.config && this.configProvider) {
-      // TODO: make resolving provide only resolve once at concurrent execution
-      this.config = await this.configProvider();
+    if (!this.config) {
+      this.config = await this.configProvider;
     }
     return new Promise((resolve, reject) => {
       if (!this.config) {