fix: critical state machine and process timeout bugs - #26
Merged
Conversation
Fix three critical bugs identified in the code audit: 1. State machine counter bug: aggregate failure/success counters were incremented once per target per round instead of once per round. With N targets, this caused premature state transitions (e.g., ONLINE->OFFLINE in a single round with 3 targets). Now tracks round completion and only evaluates state after all targets report. 2. Traceroute process timeout: traceroute subprocess had no hard timeout and could hang indefinitely on broken networks. Added tokio::select! with configurable process_timeout_ms, matching the pattern already used in ping.rs. 3. Gateway ping process timeout: ping_host() had no process timeout and used hardcoded 2000ms instead of config values. Now uses configured ping_timeout_ms and process_timeout_ms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three critical bugs from Feature 017 (Code Audit):
degraded_threshold=3, a single round of failures jumped straight to OFFLINE. Now tracks round completion and evaluates state only after all targets report.traceroutesubprocess had no hard timeout; could hang indefinitely on broken networks (exactly when it's invoked). Addedtokio::select!with configurableprocess_timeout_ms.ping_host()had no process timeout and used hardcoded 2000ms. Now uses config values and the same kill-on-timeout pattern asping.rs.Test plan
test_multi_target_round_counting— verifies fix: 2-target rounds don't skip statestest_process_round_batch— verifies newprocess_round()batch APIcargo run -- --dev start -fwith default 2 targets, verify DEGRADED/OFFLINE transitions take expected number of rounds