From 5d628d2197a187bc2a4f440a0e6fed89d2255cdc Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 1 Jun 2026 16:53:14 +0100 Subject: [PATCH 1/4] Reject failed Docker image builds --- .../clients/image/docker-image-client.test.ts | 47 +++++++++++++++++++ .../clients/image/docker-image-client.ts | 26 +++++----- .../generic-container.test.ts | 6 +-- 3 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts diff --git a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts new file mode 100644 index 000000000..0ebc6f8af --- /dev/null +++ b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts @@ -0,0 +1,47 @@ +import Dockerode from "dockerode"; +import { Readable } from "stream"; +import { DockerImageClient } from "./docker-image-client"; + +describe("DockerImageClient", () => { + it("rejects when the Docker image build cannot start", async () => { + const dockerode = { + buildImage: vi.fn((stream: Readable) => { + stream.destroy(); + return Promise.reject(new Error("build failed")); + }), + } as unknown as Dockerode; + const imageClient = new DockerImageClient(dockerode, ""); + + const result = await Promise.race([ + imageClient.build(__dirname, { t: "image" }).then( + () => "resolved", + () => "rejected" + ), + new Promise((resolve) => setTimeout(() => resolve("timeout"), 100)), + ]); + + expect(result).toBe("rejected"); + }); + + it("rejects when the Docker image build stream errors", async () => { + const buildStream = new Readable({ read() {} }); + const dockerode = { + buildImage: vi.fn((stream: Readable) => { + stream.destroy(); + setTimeout(() => buildStream.destroy(new Error("build failed")), 0); + return Promise.resolve(buildStream); + }), + } as unknown as Dockerode; + const imageClient = new DockerImageClient(dockerode, ""); + + const result = await Promise.race([ + imageClient.build(__dirname, { t: "image" }).then( + () => "resolved", + () => "rejected" + ), + new Promise((resolve) => setTimeout(() => resolve("timeout"), 100)), + ]); + + expect(result).toBe("rejected"); + }); +}); diff --git a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts index 4c6ab884c..11bbcd3a8 100644 --- a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts +++ b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts @@ -24,19 +24,19 @@ export class DockerImageClient implements ImageClient { log.debug(`Building image "${opts.t}" with context "${context}"...`); const tarPackOptions = await this.createTarPackOptions(context, opts.dockerfile ?? "Dockerfile"); const tarStream = tar.pack(context, tarPackOptions); - await new Promise((resolve) => { - this.dockerode - .buildImage(tarStream, opts) - .then((stream) => byline(stream)) - .then((stream) => { - stream.setEncoding("utf-8"); - stream.on("data", (line) => { - if (buildLog.enabled()) { - buildLog.trace(line, { imageName: opts.t }); - } - }); - stream.on("end", () => resolve()); - }); + const buildStream = await this.dockerode.buildImage(tarStream, opts); + await new Promise((resolve, reject) => { + buildStream.on("error", reject); + + const lineStream = byline(buildStream); + lineStream.setEncoding("utf-8"); + lineStream.on("data", (line) => { + if (buildLog.enabled()) { + buildLog.trace(line, { imageName: opts.t }); + } + }); + lineStream.on("error", reject); + lineStream.on("end", resolve); }); log.debug(`Built image "${opts.t}" with context "${context}"`); } catch (err) { diff --git a/packages/testcontainers/src/generic-container/generic-container.test.ts b/packages/testcontainers/src/generic-container/generic-container.test.ts index 2e7f999a0..89227390f 100644 --- a/packages/testcontainers/src/generic-container/generic-container.test.ts +++ b/packages/testcontainers/src/generic-container/generic-container.test.ts @@ -665,10 +665,8 @@ describe("GenericContainer", { timeout: 180_000 }, () => { expect(container.getHostname()).toEqual("hostname"); }); - // failing to build an image hangs within the DockerImageClient.build method, - // that change might be larger so leave it out of this commit but skip the failing test - it.skip("should throw an error for a target stage that does not exist", async () => { + it("should throw an error for a target stage that does not exist", async () => { const context = path.resolve(fixtures, "docker-multi-stage"); - await GenericContainer.fromDockerfile(context).withTarget("invalid").build(); + await expect(GenericContainer.fromDockerfile(context).withTarget("invalid").build()).rejects.toThrow(); }); }); From 1ac1925bf33539cca37cbd27c1be409444c6bd22 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 1 Jun 2026 17:20:12 +0100 Subject: [PATCH 2/4] Reject Docker build progress errors --- AGENTS.md | 2 ++ .../clients/image/docker-image-client.test.ts | 34 +++++++++++++++---- .../clients/image/docker-image-client.ts | 23 +++++++------ 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3e36dfe94..b9d38d498 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,6 +28,8 @@ It captures practical rules that prevent avoidable CI and PR churn. - In docs Markdown, keep `` blocks tight with no blank lines between the markers and the include line, or the rendered snippet will contain blank lines between code lines. - Tests should verify observable behavior changes, not only internal/config state. - Example: for a security option, assert a real secure/insecure behavior difference. +- Docker build failures may arrive as JSON progress entries before the stream ends normally. + - When handling build streams, reject `errorDetail` or `error` entries explicitly; `followProgress` alone only reports transport errors. - Test-only helper files under `src` (for example `*-test-utils.ts`) must be explicitly excluded from package `tsconfig.build.json` so they are not emitted into `build` and accidentally published. - For substantial changes to GitHub Actions, runner images, Node/npm versions, or release/publish automation, consider running the manual `Node.js Package` workflow as a dry-run publish sanity check. - Select the PR branch as the workflow ref to test publish workflow changes before merging. diff --git a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts index 0ebc6f8af..551e8baa6 100644 --- a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts +++ b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts @@ -3,6 +3,23 @@ import { Readable } from "stream"; import { DockerImageClient } from "./docker-image-client"; describe("DockerImageClient", () => { + const createDockerode = (buildStream: Readable): Dockerode => + ({ + buildImage: vi.fn((stream: Readable) => { + stream.destroy(); + return Promise.resolve(buildStream); + }), + modem: { + followProgress: vi.fn( + (stream: Readable, onFinished: (error: Error | null) => void, onProgress: (event: unknown) => void) => { + stream.on("data", (line) => onProgress(JSON.parse(line.toString()))); + stream.on("error", onFinished); + stream.on("end", () => onFinished(null)); + } + ), + }, + }) as unknown as Dockerode; + it("rejects when the Docker image build cannot start", async () => { const dockerode = { buildImage: vi.fn((stream: Readable) => { @@ -25,14 +42,9 @@ describe("DockerImageClient", () => { it("rejects when the Docker image build stream errors", async () => { const buildStream = new Readable({ read() {} }); - const dockerode = { - buildImage: vi.fn((stream: Readable) => { - stream.destroy(); - setTimeout(() => buildStream.destroy(new Error("build failed")), 0); - return Promise.resolve(buildStream); - }), - } as unknown as Dockerode; + const dockerode = createDockerode(buildStream); const imageClient = new DockerImageClient(dockerode, ""); + setTimeout(() => buildStream.destroy(new Error("build failed")), 0); const result = await Promise.race([ imageClient.build(__dirname, { t: "image" }).then( @@ -44,4 +56,12 @@ describe("DockerImageClient", () => { expect(result).toBe("rejected"); }); + + it("rejects when the Docker image build progress reports an error", async () => { + const buildStream = Readable.from([`${JSON.stringify({ errorDetail: { message: "build failed" } })}\n`]); + const dockerode = createDockerode(buildStream); + const imageClient = new DockerImageClient(dockerode, ""); + + await expect(imageClient.build(__dirname, { t: "image" })).rejects.toThrow("build failed"); + }); }); diff --git a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts index 11bbcd3a8..1a42ac5c6 100644 --- a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts +++ b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts @@ -26,17 +26,20 @@ export class DockerImageClient implements ImageClient { const tarStream = tar.pack(context, tarPackOptions); const buildStream = await this.dockerode.buildImage(tarStream, opts); await new Promise((resolve, reject) => { - buildStream.on("error", reject); - - const lineStream = byline(buildStream); - lineStream.setEncoding("utf-8"); - lineStream.on("data", (line) => { - if (buildLog.enabled()) { - buildLog.trace(line, { imageName: opts.t }); + this.dockerode.modem.followProgress( + buildStream, + (err) => (err ? reject(err) : resolve()), + (event) => { + if (buildLog.enabled()) { + buildLog.trace(JSON.stringify(event), { imageName: opts.t }); + } + + const error = event.errorDetail?.message ?? event.error; + if (error !== undefined) { + reject(new Error(error)); + } } - }); - lineStream.on("error", reject); - lineStream.on("end", resolve); + ); }); log.debug(`Built image "${opts.t}" with context "${context}"`); } catch (err) { From a38973cc7c5eb7c9df7b7ba329d227811f044cca Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 1 Jun 2026 17:23:09 +0100 Subject: [PATCH 3/4] Remove implementation detail from AGENTS.md --- AGENTS.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b9d38d498..3e36dfe94 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,8 +28,6 @@ It captures practical rules that prevent avoidable CI and PR churn. - In docs Markdown, keep `` blocks tight with no blank lines between the markers and the include line, or the rendered snippet will contain blank lines between code lines. - Tests should verify observable behavior changes, not only internal/config state. - Example: for a security option, assert a real secure/insecure behavior difference. -- Docker build failures may arrive as JSON progress entries before the stream ends normally. - - When handling build streams, reject `errorDetail` or `error` entries explicitly; `followProgress` alone only reports transport errors. - Test-only helper files under `src` (for example `*-test-utils.ts`) must be explicitly excluded from package `tsconfig.build.json` so they are not emitted into `build` and accidentally published. - For substantial changes to GitHub Actions, runner images, Node/npm versions, or release/publish automation, consider running the manual `Node.js Package` workflow as a dry-run publish sanity check. - Select the PR branch as the workflow ref to test publish workflow changes before merging. From 17b0bec9d3a12c6408e7e6dc16e47b09ae8dfad3 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Sun, 7 Jun 2026 14:29:08 +0100 Subject: [PATCH 4/4] Simplify Docker build progress error handling --- .../clients/image/docker-image-client.test.ts | 17 ++++++++++---- .../clients/image/docker-image-client.ts | 22 +++++++++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts index 551e8baa6..bf910e36e 100644 --- a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts +++ b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.test.ts @@ -11,10 +11,19 @@ describe("DockerImageClient", () => { }), modem: { followProgress: vi.fn( - (stream: Readable, onFinished: (error: Error | null) => void, onProgress: (event: unknown) => void) => { - stream.on("data", (line) => onProgress(JSON.parse(line.toString()))); - stream.on("error", onFinished); - stream.on("end", () => onFinished(null)); + ( + stream: Readable, + onFinished: (error: Error | null, output: unknown[]) => void, + onProgress: (event: unknown) => void + ) => { + const output: unknown[] = []; + stream.on("data", (line) => { + const event = JSON.parse(line.toString()); + output.push(event); + onProgress(event); + }); + stream.on("error", (error) => onFinished(error, output)); + stream.on("end", () => onFinished(null, output)); } ), }, diff --git a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts index 1a42ac5c6..0987e72bf 100644 --- a/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts +++ b/packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts @@ -10,6 +10,15 @@ import { getAuthConfig } from "../../auth/get-auth-config"; import { ImageName } from "../../image-name"; import { ImageClient } from "./image-client"; +type DockerBuildEvent = { + error?: string; + errorDetail?: { + message?: string; + }; +}; + +const getBuildError = (event: DockerBuildEvent): string | undefined => event.errorDetail?.message ?? event.error; + export class DockerImageClient implements ImageClient { private readonly existingImages = new Set(); private readonly imageExistsLock = new AsyncLock(); @@ -25,22 +34,21 @@ export class DockerImageClient implements ImageClient { const tarPackOptions = await this.createTarPackOptions(context, opts.dockerfile ?? "Dockerfile"); const tarStream = tar.pack(context, tarPackOptions); const buildStream = await this.dockerode.buildImage(tarStream, opts); - await new Promise((resolve, reject) => { + const buildEvents = await new Promise((resolve, reject) => { this.dockerode.modem.followProgress( buildStream, - (err) => (err ? reject(err) : resolve()), + (err, output) => (err ? reject(err) : resolve(output)), (event) => { if (buildLog.enabled()) { buildLog.trace(JSON.stringify(event), { imageName: opts.t }); } - - const error = event.errorDetail?.message ?? event.error; - if (error !== undefined) { - reject(new Error(error)); - } } ); }); + const error = buildEvents.map(getBuildError).find((error) => error !== undefined); + if (error !== undefined) { + throw new Error(error); + } log.debug(`Built image "${opts.t}" with context "${context}"`); } catch (err) { log.error(`Failed to build image: ${err}`);