fix: add timeout to network connectivity checks in healthcheck - #7625
fix: add timeout to network connectivity checks in healthcheck#7625qianeric-backup wants to merge 1 commit into
Conversation
- Added 5 second timeout to TCP/UDP network dial operations - Prevents healthcheck from hanging indefinitely on network issues - Uses context.WithTimeout for proper cancellation support
WalkthroughHealth checks now apply a five-second timeout to IPv4 TCP, IPv6 TCP, and IPv4 UDP dialing. Existing connectivity reporting and connection cleanup remain unchanged. ChangesHealth-check timeout handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/runner/healthcheck.go`:
- Around line 53-57: Update DoHealthCheck to create a single five-second timeout
context before the first network probe and reuse that ctx for the TCP4, TCP6,
and UDP4 dial calls. Remove the per-probe ctx2 and ctx3 contexts while
preserving the existing cancellation and probe behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e627068-88ac-4ca8-9ef9-c7db29f4c20a
📒 Files selected for processing (1)
internal/runner/healthcheck.go
| ctx, cancel := context.WithTimeout(context.Background(), defaultNetworkTimeout) | ||
| defer cancel() | ||
|
|
||
| var d net.Dialer | ||
| c4, err := d.DialContext(ctx, "tcp4", "scanme.sh:80") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reuse one five-second context for all probes.
The current code resets the deadline for each probe. If all three dials block, DoHealthCheck can take almost fifteen seconds instead of five seconds.
Create one timeout context before the first probe. Pass the same ctx to the TCP6 and UDP4 calls. Remove ctx2 and ctx3.
Proposed fix
ctx, cancel := context.WithTimeout(context.Background(), defaultNetworkTimeout)
defer cancel()
var d net.Dialer
c4, err := d.DialContext(ctx, "tcp4", "scanme.sh:80")
...
-ctx2, cancel2 := context.WithTimeout(context.Background(), defaultNetworkTimeout)
-defer cancel2()
-c6, err := d.DialContext(ctx2, "tcp6", "scanme.sh:80")
+c6, err := d.DialContext(ctx, "tcp6", "scanme.sh:80")
...
-ctx3, cancel3 := context.WithTimeout(context.Background(), defaultNetworkTimeout)
-defer cancel3()
-u4, err := d.DialContext(ctx3, "udp4", "scanme.sh:53")
+u4, err := d.DialContext(ctx, "udp4", "scanme.sh:53")Also applies to: 67-69, 79-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/runner/healthcheck.go` around lines 53 - 57, Update DoHealthCheck to
create a single five-second timeout context before the first network probe and
reuse that ctx for the TCP4, TCP6, and UDP4 dial calls. Remove the per-probe
ctx2 and ctx3 contexts while preserving the existing cancellation and probe
behavior.
1 participant
Description
This PR adds a 5-second timeout to the network connectivity checks in the healthcheck functionality to prevent the healthcheck from hanging indefinitely when network issues occur.
Changes
Testing
This change improves the reliability of the healthcheck feature, especially in environments with intermittent network connectivity.
Summary by CodeRabbit