Reuse insecure HTTP wait agent across retries

packages/testcontainers/src/wait-strategies/http-wait-strategy.test.ts

@@ -1,9 +1,122 @@
+import { Readable } from "stream";
+import { Agent } from "undici";
 import { RandomUuid } from "../common";
 import { GenericContainer } from "../generic-container/generic-container";
 import { checkContainerIsHealthy, checkContainerIsHealthyTls, stopStartingContainer } from "../utils/test-helper";
 import { Wait } from "./wait";
 
+// The Agent constructor is spied so we can assert how many insecure agents the
+// HTTP wait strategy creates across retries. `request` falls through to the real
+// implementation by default (used by the Docker-backed tests below) and is only
+// overridden inside the agent-lifecycle tests.
+vi.mock("undici", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("undici")>();
+  return {
+    ...actual,
+    Agent: vi.fn(function (...args: ConstructorParameters<typeof actual.Agent>) {
+      const agent = new actual.Agent(...args);
+      agentInstances.push(agent);
+      return agent;
+    }),
+    request: vi.fn(actual.request),
+  };
+});
+
+let agentInstances: Agent[] = [];
+
 describe("HttpWaitStrategy", { timeout: 180_000 }, () => {
+  describe.sequential("agent lifecycle", () => {
+    beforeEach(async () => {
+      agentInstances = [];
+      const { request } = await import("undici");
+      const actual = await vi.importActual<typeof import("undici")>("undici");
+      vi.mocked(Agent).mockClear();
+      vi.mocked(request).mockClear();
+      vi.mocked(request).mockImplementation(actual.request);
+    });
+
+    afterEach(() => {
+      vi.doUnmock("../container-runtime");
+    });
+
+    function mockContainerRuntime() {
+      const client = {
+        info: { containerRuntime: { host: "localhost" } },
+        container: { inspect: vi.fn() },
+      };
+      vi.doMock("../container-runtime", async (importOriginal) => {
+        const actual = await importOriginal<typeof import("../container-runtime")>();
+        return { ...actual, getContainerRuntimeClient: async () => client };
+      });
+    }
+
+    it("should construct a single insecure agent across retries and dispose it on completion", async () => {
+      const { request } = await import("undici");
+
+      let attempts = 0;
+      vi.mocked(request).mockImplementation(async () => {
+        attempts++;
+        // Fail the first few attempts so the retry loop runs multiple times,
+        // then return a passing response.
+        if (attempts < 3) {
+          throw new Error("connection refused");
+        }
+        return {
+          statusCode: 200,
+          headers: {},
+          body: Readable.from(["ok"]),
+        } as unknown as Awaited<ReturnType<typeof request>>;
+      });
+
+      mockContainerRuntime();
+      const { HttpWaitStrategy } = await import("./http-wait-strategy.js");
+
+      const boundPorts = { getBinding: () => 12345 } as never;
+      const container = { id: "container-id" } as never;
+
+      const strategy = new HttpWaitStrategy("/health", 8443, {})
+        .usingTls()
+        .allowInsecure()
+        .withReadTimeout(10)
+        .withStartupTimeout(5000);
+
+      await strategy.waitUntilReady(container, boundPorts);
+
+      expect(attempts).toBeGreaterThan(1);
+      // Only one Agent is constructed despite multiple retry attempts.
+      expect(vi.mocked(Agent)).toHaveBeenCalledTimes(1);
+      expect(agentInstances).toHaveLength(1);
+      // The agent is disposed once the wait strategy finishes.
+      expect(agentInstances[0].closed).toBe(true);
+    });
+
+    it("should never construct an agent when allowInsecure is not set", async () => {
+      const { request } = await import("undici");
+
+      vi.mocked(request).mockImplementation(
+        async () =>
+          ({
+            statusCode: 200,
+            headers: {},
+            body: Readable.from(["ok"]),
+          }) as unknown as Awaited<ReturnType<typeof request>>
+      );
+
+      mockContainerRuntime();
+      const { HttpWaitStrategy } = await import("./http-wait-strategy.js");
+
+      const boundPorts = { getBinding: () => 12345 } as never;
+      const container = { id: "container-id" } as never;
+
+      const strategy = new HttpWaitStrategy("/health", 8080, {}).withReadTimeout(10).withStartupTimeout(5000);
+
+      await strategy.waitUntilReady(container, boundPorts);
+
+      expect(vi.mocked(Agent)).not.toHaveBeenCalled();
+      expect(agentInstances).toHaveLength(0);
+    });
+  });
+
   it("should wait for 200", async () => {
     await using container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
       .withExposedPorts(8080)

packages/testcontainers/src/wait-strategies/http-wait-strategy.ts

@@ -17,6 +17,7 @@ export class HttpWaitStrategy extends AbstractWaitStrategy {
   private readonly predicates: Array<(response: Response) => Promise<boolean>> = [];
   private _allowInsecure = false;
   private readTimeoutMs = 1000;
+  private insecureAgent?: Agent;
 
   constructor(
     private readonly path: string,
@@ -57,8 +58,8 @@ export class HttpWaitStrategy extends AbstractWaitStrategy {
     return this;
   }
 
-  public withReadTimeout(startupTimeoutMs: number): this {
-    this.readTimeoutMs = startupTimeoutMs;
+  public withReadTimeout(readTimeoutMs: number): this {
+    this.readTimeoutMs = readTimeoutMs;
     return this;
   }
 
@@ -80,60 +81,64 @@ export class HttpWaitStrategy extends AbstractWaitStrategy {
     const client = await getContainerRuntimeClient();
     const { abortOnContainerExit } = this.options;
 
-    await new IntervalRetry<Response | undefined, Error>(this.readTimeoutMs).retryUntil(
-      async () => {
-        try {
-          const url = `${this.protocol}://${client.info.containerRuntime.host}:${boundPorts.getBinding(this.port)}${
-            this.path
-          }`;
-
-          if (abortOnContainerExit) {
-            const containerStatus = (await client.container.inspect(container)).State.Status;
-
-            if (containerStatus === exitStatus) {
-              containerExited = true;
-              return;
+    try {
+      await new IntervalRetry<Response | undefined, Error>(this.readTimeoutMs).retryUntil(
+        async () => {
+          try {
+            const url = `${this.protocol}://${client.info.containerRuntime.host}:${boundPorts.getBinding(this.port)}${
+              this.path
+            }`;
+
+            if (abortOnContainerExit) {
+              const containerStatus = (await client.container.inspect(container)).State.Status;
+
+              if (containerStatus === exitStatus) {
+                containerExited = true;
+                return;
+              }
             }
+
+            return undiciResponseToFetchResponse(
+              await request(url, {
+                method: this.method,
+                signal: AbortSignal.timeout(this.readTimeoutMs),
+                headers: this.headers,
+                dispatcher: this.getAgent(),
+              })
+            );
+          } catch {
+            return undefined;
+          }
+        },
+        async (response) => {
+          if (abortOnContainerExit && containerExited) {
+            return true;
           }
 
-          return undiciResponseToFetchResponse(
-            await request(url, {
-              method: this.method,
-              signal: AbortSignal.timeout(this.readTimeoutMs),
-              headers: this.headers,
-              dispatcher: this.getAgent(),
-            })
-          );
-        } catch {
-          return undefined;
-        }
-      },
-      async (response) => {
-        if (abortOnContainerExit && containerExited) {
-          return true;
-        }
-
-        if (response === undefined) {
-          return false;
-        } else if (!this.predicates.length) {
-          return response.ok;
-        } else {
-          for (const predicate of this.predicates) {
-            const result = await predicate(response);
-            if (!result) {
-              return false;
+          if (response === undefined) {
+            return false;
+          } else if (!this.predicates.length) {
+            return response.ok;
+          } else {
+            for (const predicate of this.predicates) {
+              const result = await predicate(response);
+              if (!result) {
+                return false;
+              }
             }
+            return true;
           }
-          return true;
-        }
-      },
-      () => {
-        const message = `URL ${this.path} not accessible after ${this.startupTimeoutMs}ms`;
-        log.error(message, { containerId: container.id });
-        throw new Error(message);
-      },
-      this.startupTimeoutMs
-    );
+        },
+        () => {
+          const message = `URL ${this.path} not accessible after ${this.startupTimeoutMs}ms`;
+          log.error(message, { containerId: container.id });
+          throw new Error(message);
+        },
+        this.startupTimeoutMs
+      );
+    } finally {
+      await this.closeAgent();
+    }
 
     if (abortOnContainerExit && containerExited) {
       return this.handleContainerExit(container);
@@ -166,12 +171,26 @@ export class HttpWaitStrategy extends AbstractWaitStrategy {
   }
 
   private getAgent(): Agent | undefined {
-    if (this._allowInsecure) {
-      return new Agent({
+    if (!this._allowInsecure) {
+      return undefined;
+    }
+
+    if (!this.insecureAgent) {
+      this.insecureAgent = new Agent({
         connect: {
           rejectUnauthorized: false,
         },
       });
     }
+
+    return this.insecureAgent;
+  }
+
+  private async closeAgent(): Promise<void> {
+    if (this.insecureAgent) {
+      const agent = this.insecureAgent;
+      this.insecureAgent = undefined;
+      await agent.close();
+    }
   }
 }