Apply withStartupTimeout() to the port-binding pre-wait

[kenzox]

Fixes #1446.

GenericContainer.start() waits for host port bindings (inspectContainerUntilPortsExposed) before the wait strategy runs, but neither call site passes this.startupTimeoutMs, so that pre-wait is always capped at the util's 10 s default and withStartupTimeout() does not apply to it.

This passes this.startupTimeoutMs at both call sites (reuseContainer and startContainer). When no startup timeout was configured the argument is undefined and the existing 10 s default still applies, so behaviour only changes for callers who explicitly asked for a longer budget.

Measured (12.1.0, six postgres:18-alpine containers starting concurrently on a 2-vCPU GitHub runner): port binding took up to 16.7 s and failed at 10 s despite withStartupTimeout(120_000); with this change the same suite passes.

Verified locally on the branch: npm ci → 0, tsc -p packages/testcontainers/tsconfig.json --noEmit → 0, eslint → 0, prettier → 0. The Docker-backed integration suite was not run here.

[netlify]

✅ Deploy Preview for testcontainers-node ready!

Name	Link
🔨 Latest commit	c1a809f
🔍 Latest deploy log	https://app.netlify.com/projects/testcontainers-node/deploys/6ab689d5f848b500085da8ef
😎 Deploy Preview	https://deploy-preview-1447--testcontainers-node.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

GenericContainer now passes startupTimeoutMs to port-exposure inspection for reused and newly started containers.

Changes

Container startup timeout

Layer / File(s)	Summary
Apply startup timeout to port inspection packages/testcontainers/src/generic-container/generic-container.ts	Both container startup paths pass this.startupTimeoutMs to inspectContainerUntilPortsExposed.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🟡 Moderate · up to c1a80

Containers configured with a longer wait-strategy timeout can still fail during slow port binding. Propagate the effective timeout before merging unless this limitation is explicitly accepted.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c1a80

The configured timeout now applies to the port-binding wait as well as the later readiness wait. The change does not add permissions or network exposure. Docker-backed integration behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed wait applies to containers started through these existing GenericContainer paths when callers set a startup timeout; it does not itself broaden which containers or ports can be reached.

Trust Boundaries and Controls

• inferred — No new attacker-controlled input or authority transition is evident in the changed calls: they pass an existing caller-set value to the existing inspection operation, leaving its port-binding check intact.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description directly explains the timeout propagation change, affected call sites, intended behavior, validation results, and the unrun Docker integration suite.
Title check	✅ Passed	The title clearly and concisely describes the main change: applying withStartupTimeout() to the port-binding pre-wait.
Linked Issues check	✅ Passed	Issue #1446 requires the configured startup timeout to apply to the port-binding pre-wait. The whole-PR diff passes this.startupTimeoutMs to inspectContainerUntilPortsExposed() in both `reuseConta…
Out of Scope Changes check	✅ Passed	The whole-PR diff contains only the two timeout arguments in packages/testcontainers/src/generic-container/generic-container.ts. Both changes directly implement issue #1446. The diff contains no dem…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b917d69b3c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add a regression test for the timeout path

This bug fix changes GenericContainer.start() behavior, but there is no test proving that a custom withStartupTimeout() lets the port-binding pre-wait run past the utility's 10s default; a future refactor could silently reintroduce the cap. Please add a focused regression test around this startup path that fails before the change and passes after it.

AGENTS.md reference: AGENTS.md:L29-L34

Useful? React with 👍 / 👎.

[cristianrgreco]

Thanks for raising the PR, apologies for the delay in reviewing it. Looks good but a few gaps:

1. restart() has the same bug. StartedGenericContainer.restart() (started-generic-container.ts:95) also calls inspectContainerUntilPortsExposed without a timeout, so withStartupTimeout() is not applied to the port-binding pre-wait (hardcoded 10 s) — measured under CPU contention #1446 still reproduces on restart. The wait strategy is available there, so you can pass this.waitStrategy.isStartupTimeoutSet() ? this.waitStrategy.getStartupTimeout() : undefined.

2. Timeouts set on the wait strategy itself are still ignored. withWaitStrategy(Wait.forLogMessage(...).withStartupTimeout(120_000)) is a documented pattern and doesn't touch this.startupTimeoutMs, so the pre-wait would still be capped at 10 s. Deriving the value from the effective strategy rather than the container field would cover both.

3. This also shortens the pre-wait for anyone with a timeout under 10 s. The description says only longer budgets change behaviour, but withStartupTimeout(3_000) now caps port binding at 3 s where it previously had 10 s (Increase timeout for waiting for host port bindings #1038 raised it from 5 s because 5 s was flaky). Either Math.max(timeout, 10_000) or call it out explicitly.

4. Consider making the util's timeout parameter required. The silent default is what let the three call sites drift in the first place, and it's how restart() got missed here.

5. A small unit test asserting the third argument is passed through (mocking ./inspect-container-util-ports-exposed) would lock this in.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Use the configured wait-strategy timeout for the port pre-wait. · generic-container.ts:173

packages/testcontainers/src/generic-container/generic-container.ts:173
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the configured wait-strategy timeout for the port pre-wait.

When withWaitStrategy() supplies a timeout without GenericContainer.withStartupTimeout(), both pre-wait calls use the helper's 10,000 ms default. Startup can fail before the wait strategy runs.

When the container timeout is undefined, pass the configured wait-strategy timeout to inspectContainerUntilPortsExposed() in both startup paths.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f2256eb5-fa0b-4bdb-b61a-ec0e6967091e

📥 Commits

Reviewing files that changed from the base of the PR and between b917d69 and c1a809f.

📒 Files selected for processing (1)

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

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

[behzadardehei]

For reference, we're running a patch on 11.14.0 that addresses all five review points. Details, measurements and a deterministic fault-injection reproduction are in #1446 (comment):

1. restart() passes resolvePortsExposedTimeout(undefined, this.waitStrategy).
2. Wait-strategy-level timeouts are honoured: the container-level value first, else waitStrategy.isStartupTimeoutSet() ? waitStrategy.getStartupTimeout() : undefined.
3. 10 s floor: Math.max(10_000, configured).
4. timeout is required in the helper, and a missing value throws.
5. Unit tests cover the resolver, the required timeout, and that every call site passes the resolved value.

Unpatched with a simulated 15 s bind delay, the start fails at 10.6 s; patched, it succeeds at 15.2 s. Glad to contribute these changes or tests here if useful.

[cristianrgreco]

Glad to contribute these changes or tests here if useful.

Sure @behzadardehei , thank you.

[cristianrgreco]

Thanks @kenzox and @behzadardehei for the report, measurements and reproduction. Tying this wait to the startup timeout means the two phases can each consume the full budget, so a 10 min timeout could wait close to 20 min. I've gone with a fixed, larger limit instead in #1494, which covers the measured 16.7 s case without changing what withStartupTimeout means. Closing in favour of that.