Skip to content

fix(nim): bound curl health probes with --max-time - #1813

Closed
WuKongAI-CMU wants to merge 1 commit into
NVIDIA:mainfrom
WuKongAI-CMU:fix/nim-curl-timeouts-v2
Closed

fix(nim): bound curl health probes with --max-time#1813
WuKongAI-CMU wants to merge 1 commit into
NVIDIA:mainfrom
WuKongAI-CMU:fix/nim-curl-timeouts-v2

Conversation

@WuKongAI-CMU

@WuKongAI-CMU WuKongAI-CMU commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Summary

The NIM health probe loop and nimStatusByName() use curl -sf without a timeout in src/lib/nim.ts. On a hung or silently-dropping NIM endpoint, the curl call can block indefinitely, stalling onboard/waitForNimHealth and CLI status.

Add --max-time 5 to both probe calls so each attempt times out cleanly and the outer retry loop can make progress.

Changes

  • src/lib/nim.ts:212waitForNimHealth loop probe now curl -sf --max-time 5 ...
  • src/lib/nim.ts:265nimStatusByName health probe now curl -sf --max-time 5 ...

Test plan

  • Code review — minimal diff, existing retry/interval behavior preserved
  • 5s timeout chosen to fit under the 5s inter-retry sleep interval in waitForNimHealth
  • No new dependencies, no test changes required (probes are shelled out)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved health check responsiveness by adding a timeout mechanism to ensure faster detection and recovery from unresponsive health checks.

The NIM health probe loop and nimStatusByName() use `curl -sf` without a
timeout. On a hung or silently-dropping NIM endpoint the curl call can
block indefinitely, stalling onboard/waitForNimHealth and CLI status.

Add `--max-time 5` to both probe calls so each attempt times out cleanly
and the outer retry loop can make progress.
@github-actions

Copy link
Copy Markdown
Contributor

This repository limits contributors to 10 open pull requests. Please close or merge existing PRs before opening new ones.

@github-actions github-actions Bot closed this Apr 13, 2026
@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 23bfc73d-19f6-4147-a094-52d87d87112c

📥 Commits

Reviewing files that changed from the base of the PR and between d4aac4c and fae358d.

📒 Files selected for processing (1)
  • src/lib/nim.ts

📝 Walkthrough

Walkthrough

Added a --max-time 5 timeout constraint to curl health-check commands in NIM probing functions (waitForNimHealth and nimStatusByName). This ensures health checks complete within 5 seconds, preventing indefinite hangs during container health verification.

Changes

Cohort / File(s) Summary
NIM Health Probe Timeout
src/lib/nim.ts
Added --max-time 5 flag to curl commands in waitForNimHealth and nimStatusByName functions to enforce a 5-second timeout on health-check requests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A timeout here, a timeout there,
Five seconds now—the health checks care!
No more infinite waiting games,
Our probes hop faster, earning their names!

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

cv added a commit that referenced this pull request Apr 21, 2026
## Summary
The NIM health probe loop and \`nimStatusByName()\` use \`curl -sf\`
without any timeout in \`src/lib/nim.ts\`. On a hung or
silently-dropping NIM endpoint, the curl call can block indefinitely,
stalling \`onboard\`/\`waitForNimHealth\` and CLI \`status\`.

Add \`--connect-timeout 5 --max-time 5\` to both probe calls so each
attempt fails fast on unresponsive endpoints and the outer retry loop
can make progress.

## Changes
- \`src/lib/nim.ts:212\` — \`waitForNimHealth\` loop probe
- \`src/lib/nim.ts:265\` — \`nimStatusByName\` health probe

## Test plan
- [x] Minimal diff, existing retry/interval behavior preserved
- [x] 5s timeout chosen to fit under the 5s inter-retry sleep interval
- [x] No new dependencies

(Resubmitting — prior PR #1813 was auto-closed by the 10-PR-cap check
when our open PR count briefly exceeded the limit; we're now under the
cap.)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Health readiness probes now enforce 5-second connection and overall
timeouts for local HTTP checks, preventing probes from hanging
indefinitely. This yields faster failure detection, quicker recovery,
and reduced resource waits during startup and health checks.

* **Tests**
* Added and updated tests to verify health probes include the expected
timeout behavior so regressions are caught early.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Intern Dev <dev@wukongai.io>

Signed-off-by: Intern Dev <dev@wukongai.io>
Co-authored-by: Intern Dev <dev@wukongai.io>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants