From 505fcc7f7a9976504dd755743d6ab883382eea53 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Fri, 15 May 2026 10:01:26 +0200 Subject: [PATCH] [client/ui] Move profile-switch suppression from tray to Peers service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The optimistic Connecting paint and the Idle/stale-Connected suppression lived in the tray's applyStatus, so only the tray got the smoothed-out transition during a profile switch — the React Status page (useStatus hook in frontend) subscribes to the same netbird:status event and was seeing the raw daemon stream, complete with the Disconnected blink. Move the policy one layer up into the Peers service, between SubscribeStatus and the Wails event bus, so every consumer downstream sees the same filtered stream: * Peers gains BeginProfileSwitch / CancelProfileSwitch / shouldSuppress. BeginProfileSwitch sets the in-progress flag and emits a synthetic Connecting status so both the tray and React paint Connecting immediately. shouldSuppress swallows the daemon's stale Connected (peer-count teardown) and transient Idle (Down between flows) until Connecting / NeedsLogin / LoginFailed / SessionExpired / DaemonUnavailable indicates the new profile's flow has started, or a 30s safety timeout fires. * ProfileSwitcher.SwitchActive calls peers.BeginProfileSwitch when wasActive (prevStatus was Connected or Connecting) — the only cases where the daemon emits the blink-inducing sequence. Other prevStatuses already terminate cleanly on Idle. * Tray loses its switchInProgress fields, applyOptimisticConnecting helper, applyStatus suppression switch, and switchProfile's optimistic-paint call. handleDisconnect now calls Peers.CancelProfileSwitch alongside cancelling switchCancel, so the abort path bypasses the suppression filter and the daemon's Idle paints through immediately. The full prevStatus -> action / optimistic label / suppressed events matrix now lives in the ProfileSwitcher struct godoc, with the suppression-rule-per-incoming-status table on the Peers struct godoc — together they describe the click-time policy and the stream-filter behaviour without duplication. Wails bindings need regenerating to pick up Peers.BeginProfileSwitch and Peers.CancelProfileSwitch. --- client/ui/services/peers.go | 82 +++++++++++++++ client/ui/services/profileswitcher.go | 59 ++++++++--- client/ui/tray.go | 139 +++----------------------- 3 files changed, 143 insertions(+), 137 deletions(-) diff --git a/client/ui/services/peers.go b/client/ui/services/peers.go index 0adeeba25..bba67502b 100644 --- a/client/ui/services/peers.go +++ b/client/ui/services/peers.go @@ -130,6 +130,26 @@ type Status struct { // Peers serves the dashboard data: one polled Status RPC and a long-running // SubscribeEvents stream that re-emits every event over the Wails event bus. +// +// Profile-switch suppression: ProfileSwitcher calls BeginProfileSwitch +// before tearing down the old profile when it would otherwise be followed +// by an Up on the new profile (i.e. previous status was Connected or +// Connecting). statusStreamLoop then swallows the transient stale +// Connected and Idle pushes the daemon emits during Down so the tray +// and the React Status page both see Connecting → new-profile-state +// instead of Connected → Connected → Idle → Connecting → new-state. +// +// Suppression transition (applied by shouldSuppress before each emit): +// +// ┌────────────────────────────────────────────┬──────────────────────────────────┐ +// │ Incoming daemon status │ Action │ +// ├────────────────────────────────────────────┼──────────────────────────────────┤ +// │ Connected, Idle │ Suppress (the blink we hide) │ +// │ Connecting │ Emit, clear flag (new Up began) │ +// │ NeedsLogin, LoginFailed, SessionExpired, │ Emit, clear flag (new profile's │ +// │ DaemonUnavailable │ "Up won't run" terminal state) │ +// │ (timeout elapsed) │ Clear flag, emit normally │ +// └────────────────────────────────────────────┴──────────────────────────────────┘ type Peers struct { conn DaemonConn emitter Emitter @@ -137,12 +157,70 @@ type Peers struct { mu sync.Mutex cancel context.CancelFunc streamWg sync.WaitGroup + + switchMu sync.Mutex + switchInProgress bool + switchInProgressUntil time.Time } func NewPeers(conn DaemonConn, emitter Emitter) *Peers { return &Peers{conn: conn, emitter: emitter} } +// BeginProfileSwitch is called by ProfileSwitcher at the start of a switch +// when the previous status was Connected/Connecting — i.e. the daemon is +// about to emit Connected updates during Down's peer-count teardown and +// then an Idle before the new profile's Up resumes the stream. The flag +// makes statusStreamLoop drop those transient events. A synthetic +// Connecting snapshot is emitted right away so both consumers (tray and +// React) paint the optimistic state immediately. A 30s safety timeout +// clears the flag if the daemon never emits a follow-up status. +func (s *Peers) BeginProfileSwitch() { + s.switchMu.Lock() + s.switchInProgress = true + s.switchInProgressUntil = time.Now().Add(30 * time.Second) + s.switchMu.Unlock() + s.emitter.Emit(EventStatus, Status{Status: StatusConnecting}) +} + +// CancelProfileSwitch is called by callers that abort the switch midway +// (the tray's Disconnect click while Connecting). Clears the suppression +// flag so the next daemon Idle paints through immediately instead of +// being swallowed. +func (s *Peers) CancelProfileSwitch() { + s.switchMu.Lock() + s.switchInProgress = false + s.switchMu.Unlock() +} + +func (s *Peers) shouldSuppress(st Status) bool { + s.switchMu.Lock() + defer s.switchMu.Unlock() + if !s.switchInProgress { + return false + } + if time.Now().After(s.switchInProgressUntil) { + s.switchInProgress = false + return false + } + switch { + case strings.EqualFold(st.Status, StatusConnecting), + strings.EqualFold(st.Status, StatusNeedsLogin), + strings.EqualFold(st.Status, StatusLoginFailed), + strings.EqualFold(st.Status, StatusSessionExpired), + strings.EqualFold(st.Status, StatusDaemonUnavailable): + // New profile's flow has officially begun (Up started, or daemon + // refused to start it). Clear the guard and let it through. + s.switchInProgress = false + return false + default: + // Connected (stale carryover from old profile's teardown) or Idle + // (transient between Down and Up). Suppress so the optimistic + // Connecting from BeginProfileSwitch stays painted. + return true + } +} + // Watch starts the background loops that feed the frontend: // - statusStreamLoop: push-driven snapshots on connection-state change // (Connected/Disconnected/Connecting, peer list, address). Drives the @@ -272,6 +350,10 @@ func (s *Peers) statusStreamLoop(ctx context.Context) { unavailable = false st := statusFromProto(resp) log.Infof("backend event: status status=%q peers=%d", st.Status, len(st.Peers)) + if s.shouldSuppress(st) { + log.Debugf("suppressing status=%q during profile switch", st.Status) + continue + } s.emitter.Emit(EventStatus, st) } } diff --git a/client/ui/services/profileswitcher.go b/client/ui/services/profileswitcher.go index 5fd8ae1ca..e2cb492df 100644 --- a/client/ui/services/profileswitcher.go +++ b/client/ui/services/profileswitcher.go @@ -12,21 +12,44 @@ import ( "github.com/netbirdio/netbird/client/internal/profilemanager" ) -// ProfileSwitcher encapsulates the full profile-switching reconnect policy so -// both the tray and the React frontend use identical logic. +// ProfileSwitcher encapsulates the full profile-switching reconnect policy +// so both the tray and the React frontend use identical logic. // -// Reconnect policy: +// Reconnect policy + optimistic-feedback table (driven by prevStatus +// captured from Peers.Get at SwitchActive entry): // -// ┌─────────────────┬──────────────────────┬────────────────────────────────────┐ -// │ Previous status │ Action │ Rationale │ -// ├─────────────────┼──────────────────────┼────────────────────────────────────┤ -// │ Connected │ Switch + Down + Up │ Reconnect with the new profile. │ -// │ Connecting │ Switch + Down + Up │ Stop old retry loop, restart. │ -// │ NeedsLogin │ Switch + Down │ Clear stale error; user logs in. │ -// │ LoginFailed │ Switch + Down │ Clear stale error; user logs in. │ -// │ SessionExpired │ Switch + Down │ Clear stale error; user logs in. │ -// │ Idle │ Switch only │ User chose offline; don't connect. │ -// └─────────────────┴──────────────────────┴────────────────────────────────────┘ +// ┌─────────────────┬──────────────────────┬──────────────────────────┬────────────────────┐ +// │ Previous status │ Action │ Optimistic UI label │ Suppressed events │ +// │ │ │ shown immediately │ until new flow │ +// ├─────────────────┼──────────────────────┼──────────────────────────┼────────────────────┤ +// │ Connected │ Switch + Down + Up │ Connecting (synthetic) │ Connected, Idle │ +// │ Connecting │ Switch + Down + Up │ Connecting (unchanged) │ Connected, Idle │ +// │ NeedsLogin │ Switch + Down │ (no change) │ — │ +// │ LoginFailed │ Switch + Down │ (no change) │ — │ +// │ SessionExpired │ Switch + Down │ (no change) │ — │ +// │ Idle │ Switch only │ (no change) │ — │ +// └─────────────────┴──────────────────────┴──────────────────────────┴────────────────────┘ +// +// Only Connected/Connecting trigger the optimistic Connecting paint +// (via Peers.BeginProfileSwitch): they're the only prevStatuses where +// the daemon emits stale Connected updates (peer count drops as the +// engine tears down) and then Idle, before the new profile's Up +// resumes the stream. Both are swallowed by Peers.shouldSuppress +// until a status that signals the new flow has begun (Connecting, or +// any of the "Up won't run" terminal states: NeedsLogin / LoginFailed / +// SessionExpired / DaemonUnavailable). The other prevStatuses either +// don't drive Down/Up at all (Idle) or stop after Down (NeedsLogin / +// LoginFailed / SessionExpired) — the resulting Idle is the correct +// terminal state, so no suppression is needed. +// +// Rationale for each Action choice: +// +// Connected → Reconnect with the new profile. +// Connecting → Stop old retry loop, restart. +// NeedsLogin → Clear stale error; user logs in. +// LoginFailed → Clear stale error; user logs in. +// SessionExpired → Clear stale error; user logs in. +// Idle → User chose offline; don't connect. type ProfileSwitcher struct { profiles *Profiles connection *Connection @@ -60,6 +83,16 @@ func (s *ProfileSwitcher) SwitchActive(ctx context.Context, p ProfileRef) error log.Infof("profileswitcher: switch profile=%q prevStatus=%q wasActive=%v needsDown=%v", p.ProfileName, prevStatus, wasActive, needsDown) + // Optimistic Connecting feedback for tray + React Status page: only + // when wasActive — those are the prevStatuses where the daemon will + // emit stale Connected + transient Idle pushes during Down before + // the new profile's Up resumes the stream (see Peers godoc for the + // suppression table). Other prevStatuses already terminate cleanly + // on Idle, no suppression needed. + if wasActive { + s.peers.BeginProfileSwitch() + } + if err := s.profiles.Switch(ctx, p); err != nil { return fmt.Errorf("switch profile %q: %w", p.ProfileName, err) } diff --git a/client/ui/tray.go b/client/ui/tray.go index 3a182db5d..cc947c919 100644 --- a/client/ui/tray.go +++ b/client/ui/tray.go @@ -134,15 +134,6 @@ type Tray struct { activeUsername string switchCancel context.CancelFunc - // switchInProgress / switchInProgressUntil drive the optimistic - // Connecting feedback when the user clicks a profile while we are - // Connected or Connecting. See switchProfile and applyStatus — - // applyStatus suppresses the transient daemon Idle that the Down - // step emits, keeping the tray on Connecting until the new - // profile's Up actually starts. - switchInProgress bool - switchInProgressUntil time.Time - // profileLoadMu serializes loadProfiles so the daemon-status-driven // refresh in applyStatus cannot race with the ApplicationStarted seed // or the post-switchProfile reload — both manipulate profileSubmenu and @@ -320,8 +311,9 @@ func (t *Tray) handleConnect() { // handleDisconnect aborts any in-flight profile switch before sending // Down — otherwise the switcher's queued Up would re-establish the // connection right after the Disconnect, making the click look like a -// no-op. Also clears the optimistic-Connecting guard so subsequent -// daemon Idle pushes paint through to the tray immediately. +// no-op. Also clears Peers' optimistic-Connecting guard so the daemon's +// Idle push (and any subsequent updates) paint through immediately +// instead of being swallowed by the profile-switch suppression filter. func (t *Tray) handleDisconnect() { t.downItem.SetEnabled(false) t.mu.Lock() @@ -329,8 +321,8 @@ func (t *Tray) handleDisconnect() { t.switchCancel() t.switchCancel = nil } - t.switchInProgress = false t.mu.Unlock() + t.svc.Peers.CancelProfileSwitch() go func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -493,49 +485,12 @@ func (t *Tray) onUpdateProgress(ev *application.CustomEvent) { // rapid SubscribeStatus bursts during health probes that would // otherwise spam Shell_NotifyIcon and the log. // -// Optimistic profile-switch suppression: when switchProfile fired the -// optimistic Connecting paint (see its table), switchInProgress is set -// and the daemon-emitted Connected (stale, from old profile teardown -// where the daemon keeps pushing as peer count drops) and Idle (from -// the Down step) events are swallowed here so the user doesn't see a -// Disconnected/Connected blink before the new profile's flow begins. -// Clears the flag and lets reality take over on: -// - Connecting (the new profile's Up has officially started), -// - NeedsLogin / LoginFailed / SessionExpired / DaemonUnavailable -// (the daemon won't start Up — that's the new profile's terminal -// state, surface it immediately), or -// - the 30s safety timeout (Up wedged or never emitted anything). +// Profile-switch suppression lives one layer up in services/peers.go +// (Peers.BeginProfileSwitch / shouldSuppress) so the optimistic +// Connecting paint and the suppressed Idle/Connected events are shared +// with the React Status page rather than being a tray-only behaviour. func (t *Tray) applyStatus(st services.Status) { t.mu.Lock() - if t.switchInProgress { - switch { - case time.Now().After(t.switchInProgressUntil): - // Safety net: the daemon never emitted a follow-up status - // after Down (Up failed silently, RPC wedged, etc.). Drop - // the optimistic guard and render whatever arrives next. - t.switchInProgress = false - case strings.EqualFold(st.Status, services.StatusConnecting), - strings.EqualFold(st.Status, services.StatusNeedsLogin), - strings.EqualFold(st.Status, services.StatusLoginFailed), - strings.EqualFold(st.Status, services.StatusSessionExpired), - strings.EqualFold(st.Status, services.StatusDaemonUnavailable): - // New profile's flow has officially started: Up has begun - // (Connecting) or the daemon refused to start Up at all - // (NeedsLogin/LoginFailed/SessionExpired/DaemonUnavailable). - // Clear the guard and let applyStatus paint normally — the - // user should see the new state without further delay. - t.switchInProgress = false - default: - // Connected (stale leftover from the old profile's teardown - // — the daemon emits extra status updates while peer count - // drops during cleanup) or Idle (transient between Down and - // Up). Suppress so the tray stays on the optimistic - // Connecting until the new profile's flow actually emits a - // post-switch status. - t.mu.Unlock() - return - } - } connected := strings.EqualFold(st.Status, services.StatusConnected) iconChanged := connected != t.connected || st.Status != t.lastStatus // Detect the transition into SessionExpired: the daemon emits the @@ -888,38 +843,13 @@ func (t *Tray) loadProfiles() { // Cancelling the previous context aborts its in-flight gRPC calls (Down/Up) // so rapid clicks always converge to the last selected profile. // -// Optimistic-feedback transition table (driven by t.lastStatus at click time; -// see applyStatus for the suppression rules): -// -// ┌──────────────────────┬──────────────────────┬──────────────────────────┬────────────────────┐ -// │ prevStatus │ ProfileSwitcher call │ Optimistic tray label │ Suppressed events │ -// │ │ │ shown immediately │ until new flow │ -// ├──────────────────────┼──────────────────────┼──────────────────────────┼────────────────────┤ -// │ Connected │ Switch + Down + Up │ Connecting (spinner) │ Connected, Idle │ -// │ Connecting │ Switch + Down + Up │ Connecting (unchanged) │ Connected, Idle │ -// │ Idle │ Switch only │ (no change) │ — │ -// │ NeedsLogin │ Switch + Down │ (no change) │ — │ -// │ LoginFailed │ Switch + Down │ (no change) │ — │ -// │ SessionExpired │ Switch + Down │ (no change) │ — │ -// │ DaemonUnavailable │ (RPC fails) │ (no change; toast error) │ — │ -// └──────────────────────┴──────────────────────┴──────────────────────────┴────────────────────┘ -// -// Only Connected/Connecting need optimistic feedback because they're the -// only prevStatus values where the switcher follows up with Up — i.e. the -// only ones where the daemon emits stale Connected updates during Down -// (peer count drops as the engine tears down) and then Idle, before the -// new profile's Connecting/Connected. Both the stale Connected and the -// transient Idle are suppressed by applyStatus until a status that -// indicates the new flow has begun (Connecting, or any of the "Up won't -// run" states: NeedsLogin / LoginFailed / SessionExpired / -// DaemonUnavailable). The other branches either don't drive Down/Up at -// all (Idle) or stop after Down (NeedsLogin / LoginFailed / -// SessionExpired), and the resulting Idle is the correct terminal -// state, so no suppression is needed. -// -// A 30s safety timeout clears the in-progress flag if the daemon never -// emits a follow-up event — without it, a stuck Down/Up could leave the -// tray frozen on Connecting forever. +// The optimistic Connecting paint (and suppression of the transient +// Idle/stale Connected daemon events that follow Down) lives in +// services/peers.go — ProfileSwitcher calls Peers.BeginProfileSwitch +// when the previous status was Connected/Connecting, which emits a +// synthetic Connecting status to the event bus and starts filtering +// the daemon stream. That way both this tray and the React Status +// page see the same optimistic state without duplicating policy. func (t *Tray) switchProfile(name string) { t.mu.Lock() if t.switchCancel != nil { @@ -927,18 +857,8 @@ func (t *Tray) switchProfile(name string) { } ctx, cancel := context.WithCancel(context.Background()) t.switchCancel = cancel - needsOptimistic := strings.EqualFold(t.lastStatus, services.StatusConnected) || - strings.EqualFold(t.lastStatus, services.StatusConnecting) - if needsOptimistic { - t.switchInProgress = true - t.switchInProgressUntil = time.Now().Add(30 * time.Second) - } t.mu.Unlock() - if needsOptimistic { - t.applyOptimisticConnecting() - } - go func() { username, err := t.svc.Profiles.Username() if err != nil { @@ -960,35 +880,6 @@ func (t *Tray) switchProfile(name string) { }() } -// applyOptimisticConnecting paints the tray as if the daemon had just -// pushed status=Connecting, without waiting for the daemon to actually -// get there. Touches the status cache (so the iconChanged comparison in -// the next real applyStatus call lines up) and the same item bits that -// applyStatus would set for Connecting. Doesn't touch exitNodes, -// daemonVersion, or sessionExpired — none of those should change as a -// side effect of a profile switch. -func (t *Tray) applyOptimisticConnecting() { - t.mu.Lock() - t.connected = false - t.lastStatus = services.StatusConnecting - t.mu.Unlock() - - t.applyIcon() - if t.statusItem != nil { - t.statusItem.SetLabel(services.StatusConnecting) - t.statusItem.SetEnabled(false) - t.applyStatusIndicator(services.StatusConnecting) - } - if t.upItem != nil { - t.upItem.SetHidden(true) - t.upItem.SetEnabled(false) - } - if t.downItem != nil { - t.downItem.SetHidden(false) - t.downItem.SetEnabled(true) - } -} - // notify wraps the Wails notification service with the tray's standard // id-prefix scheme and swallows errors (notifications are best-effort). func (t *Tray) notify(title, body, id string) {