1.12.4#342
Merged
Merged
Conversation
The trigger condition that decides whether to fire the data-plane recovery flow in startPingCheck was AND-ed with `currentInterval < maxInterval`. That clause was meant to throttle the *backoff ramp* (don't widen the interval past 6s), but it also gated the recovery trigger itself — a conflation that became invisibly load-bearing once commit 8161fa6 (March 2026) bumped the default pingInterval from 3s to 15s while leaving maxInterval at 6s. Under the new defaults `currentInterval` starts at 15s and `15 < 6` is permanently false, so the recovery branch never executed. Pings just kept failing and the failure counter climbed forever, with no "Connection to server lost" log line and no newt/ping/request emitted on the websocket. Real-world recovery only happened when the underlying network came back fast enough that a periodic ping naturally succeeded again — which doesn't happen if the WireGuard state on either end has rotated, so users were left stuck until they restarted newt. This is the proximate cause of the user reports in #284 (and dups #310, fosrl/pangolin#1004). Logs in those issues all show ping-failure counters growing without ever emitting "Connection to server lost", which is exactly the fingerprint of this gate being false. The fix is to extract the trigger decision into shouldFireRecovery and remove currentInterval from it. Backoff is now computed in a separate `if` in the caller, still gated by `currentInterval < maxInterval` so the ramp is a no-op under default settings (which is the existing behaviour, just no longer entangled with the recovery trigger). Fixing the backoff ramp itself — making it useful when pingInterval >= maxInterval — is a follow-up: the priority is restoring recovery, not improving the dampening schedule. The new shouldFireRecovery helper is unit-tested. Its signature intentionally omits currentInterval, so a future refactor that re-introduces the interval-dependent gate would need to change the function signature, which makes the historical bug harder to reintroduce silently.
miloschwartz
approved these changes
May 8, 2026
mrkhachaturov
pushed a commit
to mrkhachaturov/newt
that referenced
this pull request
Jun 15, 2026
1.12.4 Former-commit-id: 542c70b
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
4 participants
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.
Fixes #284