Skip to content

fix: add timeout to API health check to prevent UI startup hang#441

Merged
hsteude merged 3 commits into
mainfrom
fix/health-check-timeout
May 26, 2026
Merged

fix: add timeout to API health check to prevent UI startup hang#441
hsteude merged 3 commits into
mainfrom
fix/health-check-timeout

Conversation

@hsteude

@hsteude hsteude commented May 26, 2026

Copy link
Copy Markdown
Contributor

Problem

After merging the Debian base image (#440), the deployed notebook shows:

upstream connect error or disconnect/reset before headers. retried and the latest reset reason: remote connection failure, transport failure reason: delayed connect error: Connection refused

Root cause: The curl -s http://127.0.0.1:4096/health health check in start-server.sh has no --max-time timeout. When OpenCode accepts the TCP connection but is slow to respond during initialization, curl hangs indefinitely and the script never reaches the exec bun run serve-ui.ts line — so the UI never starts on port 8888.

Fix

  • Add --max-time 2 to curl so it times out and retries
  • Switch endpoint from /health (returns full SPA HTML, ~2.5KB) to /session/status (returns {}, fast JSON)
  • Add -f flag to fail on HTTP errors

Testing

Verified in the running pod that curl -sf --max-time 2 http://127.0.0.1:4096/session/status returns immediately with exit 0.

The curl health check had no --max-time, causing it to hang indefinitely
when OpenCode API accepts the TCP connection but is slow to respond during
initialization. This prevented the UI server from ever starting.

Also switch from /health (returns SPA HTML) to /session/status (returns
JSON, faster) and add -f flag to fail on HTTP errors.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents the Kubeflow notebook UI startup script from hanging indefinitely while waiting for the local OpenCode API to become responsive during initialization.

Changes:

  • Add a hard timeout (--max-time 2) to the API readiness curl check to avoid indefinite hangs.
  • Switch the readiness endpoint from /health to /session/status and add -f to treat HTTP errors as failures.
Comments suppressed due to low confidence (1)

docker/kubeflow/start-server.sh:55

  • The loop still says "failed to start within 30 seconds", but with curl --max-time 2 plus sleep 1 the worst-case total wait can exceed 30s. Consider adjusting attempts/sleep/max-time or updating the error message to match the actual max wait.
    if curl -sf --max-time 2 http://127.0.0.1:4096/session/status > /dev/null 2>&1; then
        echo "API server is ready!"
        break
    fi
    if [ $i -eq 30 ]; then
        echo "ERROR: API server failed to start within 30 seconds"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread docker/kubeflow/start-server.sh Outdated
Comment on lines +49 to +55
for i in {1..30}; do
if curl -s http://127.0.0.1:4096/health > /dev/null 2>&1; then
if curl -sf --max-time 2 http://127.0.0.1:4096/session/status > /dev/null 2>&1; then
echo "API server is ready!"
break
fi
if [ $i -eq 30 ]; then
echo "ERROR: API server failed to start within 30 seconds"
echo "ERROR: API server failed to start within the allotted time"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 3a2141f: the startup failure now reports the configured 30 attempts and 2s request timeout explicitly.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@hsteude hsteude merged commit 73a44cd into main May 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants