fix: notify Go core on seamless network handover (cellular → WiFi) while VPN is active - #243
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Thanks for the fix! One question: this also fires in the reverse case, e.g. cellular comes up while the device stays on WiFi. That's still a new type with size() > 1, so peers get reset even though nothing changed for the tunnel. Should we only notify when the new transport actually displaces the current one? |
This comment was marked as outdated.
This comment was marked as outdated.
Track the highest-priority validated transport (WiFi > Cellular) and only fire NotifyNetworkChange when it truly changes, instead of whenever a secondary transport validates. This prevents false notifications when enabling cellular data while WiFi is already the active network.
Prove the engine survives every WiFi/cellular/no-network combination without a restart, and that it recovers via the network-change fast path: the budgets are deliberately tight, so a recovery that only happens after ICE disconnect detection and backoff retries fails the test. All assertions are data-plane checks (ping to a live peer, or HTTPS egress through the exit node), not just the Connected status. The cellular->WiFi handover speed case is expected to fail until the community fix (#243) merges; it is ordered last so the fail-fast listener does not skip the rest of the suite.
PR #243 landed, so the handover notification is prompt: the sweep runs ~0.5s after WiFi validates and there is no ICE timeout in the log. The outage is unchanged at 13s, but it now sits in the peer rebuild behind the notification — mostly endpoint.go's 5s fallbackDelay on the responder path, plus an offer sent while the signal stream was still reconnecting and retried. Message and doc only; the budget and the measurement are untouched.
Network changes no longer restart the engine; they are handled as events. No network: the reconnect loops suspend instead of retrying against an unreachable network, and the UI and the notification show "No network available". The loops resume with a fresh backoff when connectivity returns. Network switch: stale management, signal and relay connections are swept and re-dialed immediately on the new network (cross-transport handover fast path: fix: notify Go core on seamless network handover (cellular → WiFi) while VPN is active #243). WiFi-to-WiFi handovers are detected by network identity, so a same-transport switch triggers the same reconnect path. Availability is seeded from non-VPN internet networks, so an up tunnel cannot mask a missing underlying network. New instrumented e2e suites (NetworkTransitionTest, ExitNodeNetworkTransitionTest) cover blackout, transport-switch and handover scenarios with outage budgets. --------- Co-authored-by: hchengting <74168694+hchengting@users.noreply.github.com>
2 participants
When
onNetworkAvailabledetects a new transport type appearing alongside an already-present one (e.g. WiFi coming up while cellular is still connected), it now callsnotifyListener()to forward the network-change event to the Go core.Before: Switching from cellular to WiFi with the VPN active caused a 10–20 second outage — peer connections stayed bound to the old transport, DNS queries to peer nameservers timed out (4s each), and recovery only happened once ICE eventually detected the connection as disconnected/failed (~6s timeout × multiple retries).
After: The Go core is notified immediately, triggering the sweeper (cuts stale management/signal/relay connections) and resetting peer reconnect backoff, so peers reconnect on the new transport within ~1–2 seconds — matching the WiFi→cellular recovery time.
Related to: #146