From 632d227ed8c30bd31e312d4c4fb3cf329a509455 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Thu, 10 Apr 2025 09:23:03 +0100 Subject: [PATCH 1/3] Abort timers when log wait strategy completes --- .../wait-strategies/log-wait-strategy.test.ts | 7 ++++-- .../src/wait-strategies/log-wait-strategy.ts | 23 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts index 57961c1ea..6bc6c175d 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts @@ -55,13 +55,13 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { expect(await getRunningContainerNames()).not.toContain(containerName); }); - it("should throw an error if the message is never received", async () => { + it("should throw an error if the log stream ends and the message is not received", async () => { const containerName = `container-${new RandomUuid().nextUuid()}`; await expect( new GenericContainer("cristianrgreco/testcontainer:1.1.14") .withName(containerName) - .withCommand("/bin/sh", "-c", 'echo "Ready"') + .withCommand(["/bin/sh", "-c", 'echo "Ready"']) .withWaitStrategy(Wait.forLogMessage("unexpected")) .start() ).rejects.toThrowError(`Log stream ended and message "unexpected" was not received`); @@ -69,6 +69,9 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { expect(await getRunningContainerNames()).not.toContain(containerName); }); + // TODO + it.skip("should throw an error if the log stream is open and the strategy times out", async () => {}); + it("does not matter if container does not send all content in a single line", async () => { const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") .withCommand([ diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index a54c3f928..caaed0c24 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -9,6 +9,8 @@ import { AbstractWaitStrategy } from "./wait-strategy"; export type Log = string; export class LogWaitStrategy extends AbstractWaitStrategy { + private abortController!: AbortController; + constructor( private readonly message: Log | RegExp, private readonly times: number @@ -17,12 +19,20 @@ export class LogWaitStrategy extends AbstractWaitStrategy { } public async waitUntilReady(container: Dockerode.Container, boundPorts: BoundPorts, startTime?: Date): Promise { + this.abortController = new AbortController(); await Promise.race([this.handleTimeout(container.id), this.handleLogs(container, startTime)]); } async handleTimeout(containerId: string): Promise { - await setTimeout(this.startupTimeout); + try { + await setTimeout(this.startupTimeout, undefined, { signal: this.abortController.signal }); + } catch (err) { + if (!(err instanceof Error && err.name === "AbortError")) { + throw err; + } + } this.throwError(containerId, `Log message "${this.message}" not received after ${this.startupTimeout}ms`); + this.abortController.abort(); } async handleLogs(container: Dockerode.Container, startTime?: Date): Promise { @@ -32,9 +42,14 @@ export class LogWaitStrategy extends AbstractWaitStrategy { let matches = 0; for await (const line of byline(stream)) { + if (this.abortController.signal.aborted) { + break; + } if (this.matches(line)) { if (++matches === this.times) { - return log.debug(`Log wait strategy complete`, { containerId: container.id }); + log.debug(`Log wait strategy complete`, { containerId: container.id }); + this.abortController.abort(); + return; } } } @@ -42,11 +57,11 @@ export class LogWaitStrategy extends AbstractWaitStrategy { this.throwError(container.id, `Log stream ended and message "${this.message}" was not received`); } - matches(line: string): boolean { + private matches(line: string): boolean { return this.message instanceof RegExp ? this.message.test(line) : line.includes(this.message); } - throwError(containerId: string, message: string): void { + private throwError(containerId: string, message: string): void { log.error(message, { containerId }); throw new Error(message); } From a7f352550bccbe497e206e3a7fdf7b1f1ff2d488 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Thu, 10 Apr 2025 10:45:40 +0100 Subject: [PATCH 2/3] Remove TODO --- .../src/wait-strategies/log-wait-strategy.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts index 6bc6c175d..3f9d36142 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts @@ -69,9 +69,6 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { expect(await getRunningContainerNames()).not.toContain(containerName); }); - // TODO - it.skip("should throw an error if the log stream is open and the strategy times out", async () => {}); - it("does not matter if container does not send all content in a single line", async () => { const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") .withCommand([ From da4ed3f943a2c32a1513f4cfee924b51c5d4132a Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Thu, 10 Apr 2025 11:05:49 +0100 Subject: [PATCH 3/3] Refactor --- .../src/wait-strategies/log-wait-strategy.ts | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index caaed0c24..39f183461 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -9,8 +9,6 @@ import { AbstractWaitStrategy } from "./wait-strategy"; export type Log = string; export class LogWaitStrategy extends AbstractWaitStrategy { - private abortController!: AbortController; - constructor( private readonly message: Log | RegExp, private readonly times: number @@ -18,37 +16,45 @@ export class LogWaitStrategy extends AbstractWaitStrategy { super(); } - public async waitUntilReady(container: Dockerode.Container, boundPorts: BoundPorts, startTime?: Date): Promise { - this.abortController = new AbortController(); - await Promise.race([this.handleTimeout(container.id), this.handleLogs(container, startTime)]); + async waitUntilReady(container: Dockerode.Container, boundPorts: BoundPorts, startTime?: Date): Promise { + const abortController = new AbortController(); + + await Promise.race([ + this.handleTimeout(container.id, abortController), + this.handleLogs(container, startTime ? startTime.getTime() / 1000 : 0, abortController), + ]); } - async handleTimeout(containerId: string): Promise { + private async handleTimeout(containerId: string, abortController: AbortController): Promise { try { - await setTimeout(this.startupTimeout, undefined, { signal: this.abortController.signal }); + await setTimeout(this.startupTimeout, undefined, { signal: abortController.signal }); } catch (err) { if (!(err instanceof Error && err.name === "AbortError")) { throw err; } } this.throwError(containerId, `Log message "${this.message}" not received after ${this.startupTimeout}ms`); - this.abortController.abort(); + abortController.abort(); } - async handleLogs(container: Dockerode.Container, startTime?: Date): Promise { + private async handleLogs( + container: Dockerode.Container, + startTime: number, + abortController: AbortController + ): Promise { log.debug(`Waiting for log message "${this.message}"...`, { containerId: container.id }); const client = await getContainerRuntimeClient(); - const stream = await client.container.logs(container, { since: startTime ? startTime.getTime() / 1000 : 0 }); + const stream = await client.container.logs(container, { since: startTime }); let matches = 0; for await (const line of byline(stream)) { - if (this.abortController.signal.aborted) { + if (abortController.signal.aborted) { break; } if (this.matches(line)) { if (++matches === this.times) { log.debug(`Log wait strategy complete`, { containerId: container.id }); - this.abortController.abort(); + abortController.abort(); return; } }