Skip to content

Defer exit latency probes until after connect so listing stays non-bl… - #1398

Merged
abhishek9686 merged 3 commits into
developfrom
v1.8.0-NM-356
Oct 22, 2026
Merged

abhishek9686 merged 3 commits into
developfrom
v1.8.0-NM-356

Conversation

@abhishek9686

Copy link
Copy Markdown
Member

…ocking.

Require a connected mesh for exit list/select, skip overlay RTT during the post-connect grace and refresh rankings afterward, and keep IGW default routes repaired when they lose to the ISP.

Describe your changes

Provide Issue ticket number if applicable/not in title

Provide link to Netmaker PR if required

Provide testing steps

Checklist before requesting a review

  • My changes affect only 10 files or less.
  • I have performed a self-review of my code and tested it.
  • If it is a new feature, I have added thorough tests, my code is <= 1450 lines.
  • If it is a bugfix, my code is <= 200 lines.
  • My functions are <= 80 lines.
  • I have had my code reviewed by a peer.
  • My unit tests pass locally.
  • Netclient & Netmaker are awesome.

…ocking.

Require a connected mesh for exit list/select, skip overlay RTT during the post-connect grace and refresh rankings afterward, and keep IGW default routes repaired when they lose to the ISP.
@abhishek9686
abhishek9686 marked this pull request as ready for review October 21, 2026 20:02
@tenki-reviewer

tenki-reviewer Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review complete. 🟠 2 high · 🟡 1 medium

💬 Inline comments (3)

  • 🟠 Rejected nexthop blocks all reinstall paths — mqhandlers.go:300
  • 🟠 Probe-only health can black-hole a working exit — igw.go:315
  • 🟡 RejectedIGW races on unsynchronized isHealthy — igw.go:227
🧹 Nitpicks (2) — 🟢 2 low
  • 🟢 409 status coupled to error message substring (handlers.go:628) — writeExitNodeErr maps the new "network not connected" case to 409 by substring-matching the error text instead of referencing a shared constant (uiapi/handlers.go:628).
  • 🟢 Manual exit restore dropped when pre-connect lookup errors (desktop_state.go:289) — The new ErrNetworkNotConnected guard makes GetDeviceSelectedExitNode fail while the network is not locally connected (desktop/device_api.go:571-573), and the manual-exit restore branch reads it only once before the retry loop (desktop/desktop_state.go:289-294).

The PR gates all exit-node list/select/get operations behind a connected network, converting a previously silent no-op into ErrNetworkNotConnected mapped to HTTP 409 across the four desktop API endpoints, with Postman docs updated accordingly. Enforced auto-exit no longer selects a nearest exit before connect; instead the enforced flag is armed in desired state and a post-connect goroutine performs the selection after probing grace has elapsed.

Files Change
desktop/device_api.go, uiapi/handlers.go, docs/uiapi-postman.json Add connected-network gating with ErrNetworkNotConnected → 409 mapping and API documentation updates
desktop/auto_exit_failover.go, desktop/auto_exit_failover_test.go, desktop/auto_exit_test.go, desktop/desktop_state.go Defer auto-exit/nearest selection until after connect; restore path handles the new disconnected-state error
functions/connect.go, functions/exit_node_meta.go, functions/mqhandlers.go Move finishDesktopConnect to a goroutine, add overlay-probe grace waiting/tracking, and skip peer-update installs for rejected nexthops
wireguard/igw.go, wireguard/igw_test.go Keep the monitor alive when unhealthy so IsCurrentIGW/RejectedIGW can suppress reinstall; overlay probe result now drives health with a first-sample failure count and the passive fallback is skipped when probing is possible

Tests were added for the failover and probe-grace flows, but two high-severity recovery-path issues (rejected nexthops blocking reinstall, probe-only health black-holing a working exit) and a data race in RejectedIGW warrant changes before merge.

Reviewed commit: b899ad4

@tenki-reviewer tenki-reviewer Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR reworks exit-node selection and IGW health handling: device exit APIs now require the network to be connected (returning 409 otherwise), enforced auto-exit selection is deferred until after connect, and internet-gateway monitors track rejected nexthops with a probe-only health path.

Key findings

  • 🟠 Rejected nexthop blocks all reinstall paths — mqhandlers.go:300
  • 🟠 Probe-only health can black-hole a working exit — igw.go:315
  • 🟡 RejectedIGW races on unsynchronized isHealthy — igw.go:227

Comment thread wireguard/igw.go Outdated
Comment thread functions/mqhandlers.go Outdated
Comment thread wireguard/igw.go
…t flap exits.

Make isHealthy atomic for RejectedIGW, allow forced ReplacePeers/iface-repair reinstalls past a rejected nexthop, and skip overlay-probe failures when underlay handshake+rx still show the gateway alive.
On FORCE_LOGOUT, clear JWT and emit a logout event so Desktop Idle can
sign the user out without waiting for the next API call.
@abhishek9686
abhishek9686 merged commit 9e1a5d3 into develop Oct 22, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants