Fix container leak when start fails after the container is created

packages/testcontainers/src/container-runtime/utils/remove-failed-container.ts

@@ -0,0 +1,16 @@
+import type Dockerode from "dockerode";
+import { log } from "../../common";
+
+export const removeFailedContainer = async (container: Dockerode.Container): Promise<void> => {
+  try {
+    log.debug(`Removing container...`, { containerId: container.id });
+    await container.remove({ force: true, v: true });
+    log.debug(`Removed container`, { containerId: container.id });
+  } catch (err) {
+    if ((err as { statusCode?: number }).statusCode === 404) {
+      log.debug(`Container already removed`, { containerId: container.id });
+    } else {
+      log.error(`Failed to remove container: ${err}`, { containerId: container.id });
+    }
+  }
+};

packages/testcontainers/src/generic-container/generic-container-lifecycle.test.ts

@@ -1,5 +1,12 @@
 import type { Mock } from "vitest";
-import { AbstractStartedContainer, GenericContainer, type InspectResult, type StartedTestContainer } from "../index";
+import { RandomUuid } from "../common";
+import {
+  AbstractStartedContainer,
+  GenericContainer,
+  getContainerRuntimeClient,
+  type InspectResult,
+  type StartedTestContainer,
+} from "../index";
 
 describe("GenericContainer lifecycle", { concurrent: false, timeout: 180_000 }, () => {
   let beforeContainerCreated: Mock;
@@ -50,6 +57,44 @@ describe("GenericContainer lifecycle", { concurrent: false, timeout: 180_000 },
     expect(containerStarted).not.toHaveBeenCalled();
   });
 
+  it.each(["containerCreated", "containerStarting", "containerStarted"] as const)(
+    "should remove the container when the %s callback fails",
+    async (callback) => {
+      const containerName = `container-${new RandomUuid().nextUuid()}`;
+      ({ containerCreated, containerStarting, containerStarted })[callback].mockImplementation(() => {
+        throw new Error(`${callback} failed`);
+      });
+
+      await expect(
+        new CustomContainer("cristianrgreco/testcontainer:1.1.14")
+          .withName(containerName)
+          .withExposedPorts(8080)
+          .start()
+      ).rejects.toThrowError(`${callback} failed`);
+
+      const client = await getContainerRuntimeClient();
+      await expect(client.container.getById(containerName).inspect()).rejects.toThrow(/404/);
+    }
+  );
+
+  it("should not reuse a container when a lifecycle callback failed", async () => {
+    const labels = { "failed-start": new RandomUuid().nextUuid() };
+    const startContainer = () =>
+      new CustomContainer("cristianrgreco/testcontainer:1.1.14")
+        .withLabels(labels)
+        .withExposedPorts(8080)
+        .withReuse()
+        .start();
+    containerStarted.mockImplementationOnce(() => {
+      throw new Error("containerStarted failed");
+    });
+
+    await expect(startContainer()).rejects.toThrowError("containerStarted failed");
+    await using container = await startContainer();
+
+    expect(container.getId()).not.toEqual(containerCreated.mock.calls[0][0]);
+  });
+
   class CustomContainer extends GenericContainer {
     protected override async beforeContainerCreated(): Promise<void> {
       beforeContainerCreated();

packages/testcontainers/src/generic-container/generic-container.ts

@@ -6,6 +6,7 @@ import type { Container, ContainerCreateOptions, ContainerInspectInfo, HostConfi
 import { containerLog, hash, log, toNanos } from "../common";
 import { type ContainerRuntimeClient, getContainerRuntimeClient, ImageName } from "../container-runtime";
 import { CONTAINER_STATUSES } from "../container-runtime/clients/container/types";
+import { removeFailedContainer } from "../container-runtime/utils/remove-failed-container";
 import type { StartedNetwork } from "../network/network";
 // biome-ignore lint/suspicious/noImportCycles: Ryuk and sshd run as GenericContainers; only dereferenced at runtime
 import { PortForwarderInstance, SSHD_IMAGE } from "../port-forwarder/port-forwarder";
@@ -199,6 +200,18 @@ export class GenericContainer implements TestContainer {
   private async startContainer(client: ContainerRuntimeClient): Promise<StartedTestContainer> {
     const container = await client.container.create({ ...this.createOpts, HostConfig: this.hostConfig });
 
+    try {
+      return await this.startCreatedContainer(client, container);
+    } catch (err) {
+      await removeFailedContainer(container);
+      throw err;
+    }
+  }
+
+  private async startCreatedContainer(
+    client: ContainerRuntimeClient,
+    container: Container
+  ): Promise<StartedTestContainer> {
     if (!this.isHelperContainer() && PortForwarderInstance.isRunning()) {
       await this.connectContainerToPortForwarder(client, container);
     }

packages/testcontainers/src/wait-strategies/wait-for-container.ts

@@ -1,11 +1,12 @@
 import type { Container } from "dockerode";
 import { log } from "../common";
 import type { ContainerRuntimeClient } from "../container-runtime";
+import { removeFailedContainer } from "../container-runtime/utils/remove-failed-container";
 import type { BoundPorts } from "../utils/bound-ports";
 import type { WaitStrategy } from "./wait-strategy";
 
 export const waitForContainer = async (
-  client: ContainerRuntimeClient,
+  _client: ContainerRuntimeClient,
   container: Container,
   waitStrategy: WaitStrategy,
   boundPorts: BoundPorts,
@@ -18,12 +19,7 @@ export const waitForContainer = async (
     log.info(`Container is ready`, { containerId: container.id });
   } catch (err) {
     log.error(`Container failed to be ready: ${err}`, { containerId: container.id });
-    try {
-      await client.container.stop(container, { timeout: 0 });
-      await client.container.remove(container, { removeVolumes: true });
-    } catch (stopErr) {
-      log.error(`Failed to stop container after it failed to be ready: ${stopErr}`, { containerId: container.id });
-    }
+    await removeFailedContainer(container);
     throw err;
   }
 };