Skip to content

Remove dead code and redundant ping timeout in health check #4790

@gkatz2

Description

@gkatz2

Bug description

PR #4108 made health check parameters configurable but left behind two cleanup items identified during review:

  1. Two dead zero-value guards in transparent_proxy.go that can never trigger, since the constructor and functional options guarantee positive values for healthCheckInterval and healthCheckPingTimeout.

  2. A hardcoded context.WithTimeout(ctx, 5*time.Second) in healthcheck.go:checkMCPStatus that redundantly wraps the ping call. The MCPPinger already enforces its own timeout via http.Client.Timeout (set to the configured value through TOOLHIVE_HEALTH_CHECK_PING_TIMEOUT). The redundant context timeout silently caps the configured ping timeout at 5s — if a user sets TOOLHIVE_HEALTH_CHECK_PING_TIMEOUT=10s for a slow backend, the context cancels first and the configured value is ineffective.

Steps to reproduce

  1. Set TOOLHIVE_HEALTH_CHECK_PING_TIMEOUT=10s
  2. The transparent proxy's MCPPinger is correctly created with a 10s HTTP client timeout
  3. But checkMCPStatus wraps the ping in context.WithTimeout(ctx, 5*time.Second), which fires first

Expected behavior

The configured ping timeout should be the effective timeout for health check pings.

Actual behavior

The hardcoded 5s context timeout in checkMCPStatus overrides any configured value above 5s.

Additional context

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggoPull requests that update go codeproxy

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions