diff --git a/packages/testcontainers/src/wait-strategies/http-wait-strategy.agent-lifecycle.test.ts b/packages/testcontainers/src/wait-strategies/http-wait-strategy.agent-lifecycle.test.ts new file mode 100644 index 000000000..4e25c8be0 --- /dev/null +++ b/packages/testcontainers/src/wait-strategies/http-wait-strategy.agent-lifecycle.test.ts @@ -0,0 +1,106 @@ +import { Readable } from "stream"; +import { Agent, request } from "undici"; +import { HttpWaitStrategy } from "./http-wait-strategy"; + +// Tracks every undici Agent the strategy constructs. Hoisted so it is initialised +// before the mock factories below run. +const { agentInstances } = vi.hoisted(() => ({ agentInstances: [] as import("undici").Agent[] })); + +// Spy on the Agent constructor and stub `request` so these tests exercise the wait +// strategy without any real HTTP or container runtime. Mocking at the top level +// (hoisted) ensures HttpWaitStrategy binds these mocks when it is imported. +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + Agent: vi.fn(function (...args: ConstructorParameters) { + const agent = new actual.Agent(...args); + agentInstances.push(agent); + return agent; + }), + request: vi.fn(), + }; +}); + +vi.mock("../container-runtime", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getContainerRuntimeClient: async () => ({ + info: { containerRuntime: { host: "localhost" } }, + container: { inspect: vi.fn() }, + }), + }; +}); + +const boundPorts = { getBinding: () => 12345 } as never; +const container = { id: "container-id" } as never; + +const passingResponse = () => + ({ statusCode: 200, headers: {}, body: Readable.from(["ok"]) }) as unknown as Awaited>; + +// Sequential: the tests share the module-level Agent spy and instance list. +describe.sequential("HttpWaitStrategy insecure agent lifecycle", () => { + beforeEach(() => { + agentInstances.length = 0; + vi.mocked(Agent).mockClear(); + vi.mocked(request).mockReset(); + }); + + it("constructs a single insecure agent across retries and disposes it on completion", async () => { + let attempts = 0; + vi.mocked(request).mockImplementation(async () => { + attempts++; + // Fail the first couple of attempts so the retry loop runs more than once. + if (attempts < 3) { + throw new Error("connection refused"); + } + return passingResponse(); + }); + + const strategy = new HttpWaitStrategy("/health", 8443, {}) + .usingTls() + .allowInsecure() + .withReadTimeout(10) + .withStartupTimeout(5000); + + await strategy.waitUntilReady(container, boundPorts); + + expect(attempts).toBeGreaterThan(1); + // A single Agent is constructed and reused across every retry attempt... + expect(vi.mocked(Agent)).toHaveBeenCalledTimes(1); + expect(agentInstances).toHaveLength(1); + // ...and it is disposed once the wait completes. + expect(agentInstances[0].destroyed).toBe(true); + }); + + it("creates a separate insecure agent per wait so concurrent waits stay isolated", async () => { + vi.mocked(request).mockImplementation(async () => passingResponse()); + + // A single strategy instance can drive multiple concurrent waits (e.g. a compose + // default wait strategy passed to every service). Each wait must get its own agent + // so one finishing wait cannot destroy a dispatcher another wait is still using. + const strategy = new HttpWaitStrategy("/health", 8443, {}) + .usingTls() + .allowInsecure() + .withReadTimeout(10) + .withStartupTimeout(5000); + + await Promise.all([strategy.waitUntilReady(container, boundPorts), strategy.waitUntilReady(container, boundPorts)]); + + expect(vi.mocked(Agent)).toHaveBeenCalledTimes(2); + expect(agentInstances).toHaveLength(2); + expect(agentInstances.every((agent) => agent.destroyed)).toBe(true); + }); + + it("never constructs an agent when allowInsecure is not set", async () => { + vi.mocked(request).mockImplementation(async () => passingResponse()); + + 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); + }); +}); diff --git a/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts index c3f12ecba..16f40039d 100644 --- a/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/http-wait-strategy.ts @@ -57,8 +57,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; } @@ -79,61 +79,73 @@ export class HttpWaitStrategy extends AbstractWaitStrategy { let containerExited = false; const client = await getContainerRuntimeClient(); const { abortOnContainerExit } = this.options; + // Scope the insecure agent to this invocation rather than the strategy instance: a single + // strategy object can drive multiple concurrent waits (e.g. a compose default wait strategy + // passed to every service), and a shared dispatcher would let one finished wait destroy the + // agent another wait is still using. + const agent = this.createInsecureAgent(); - await new IntervalRetry(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(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: agent, + }) + ); + } 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 { + // Force-close rather than graceful close(): status-only predicates never consume the + // response body, so close() could hang waiting for those connections to be released. + // The wait has finished by this point, so there is nothing left worth draining. + await agent?.destroy(); + } if (abortOnContainerExit && containerExited) { return this.handleContainerExit(container); @@ -165,13 +177,15 @@ export class HttpWaitStrategy extends AbstractWaitStrategy { throw new Error(message); } - private getAgent(): Agent | undefined { - if (this._allowInsecure) { - return new Agent({ - connect: { - rejectUnauthorized: false, - }, - }); + private createInsecureAgent(): Agent | undefined { + if (!this._allowInsecure) { + return undefined; } + + return new Agent({ + connect: { + rejectUnauthorized: false, + }, + }); } }