From 3b6d8845fb029d1b8787a874a7e9f294024c543d Mon Sep 17 00:00:00 2001 From: Alexis Johnson Date: Thu, 30 Jul 2026 12:42:19 +0200 Subject: [PATCH] [client] Add a Reconnect entry to the tray menu Collapses the Disconnect -> wait -> Connect sequence users repeat after a sleep/wake, a network roam, or peers reading connected while no traffic flows, into a single tray click. Connect and Disconnect are unchanged. Connection.Reconnect owns the ordering: Down is synchronous, so Up goes out only once the daemon reports the teardown finished, a failed Down aborts instead of stacking an Up on a session that is still up, and a context cancelled between the legs aborts as well. The tray keeps its own share - the in-flight guard, the transitional paint and the failure toast. reconnectCancel doubles as guard and abort handle: a second click cannot send a Down into the in-flight Up, and a Disconnect or quit is not undone by the Up queued behind it. The transitional paint reuses DaemonFeed's switch suppression, which already hides the stale Connected/Idle blink a Down emits and arms the SSO login-watch for the Up that follows. --- client/ui/i18n/locales/de/common.json | 6 + client/ui/i18n/locales/en/common.json | 8 ++ client/ui/i18n/locales/es/common.json | 6 + client/ui/i18n/locales/fr/common.json | 6 + client/ui/i18n/locales/hu/common.json | 6 + client/ui/i18n/locales/it/common.json | 6 + client/ui/i18n/locales/ja/common.json | 6 + client/ui/i18n/locales/pt/common.json | 6 + client/ui/i18n/locales/ru/common.json | 6 + client/ui/i18n/locales/zh-CN/common.json | 6 + client/ui/services/connection.go | 19 +++ client/ui/services/connection_test.go | 79 +++++++++++++ client/ui/tray.go | 144 +++++++++++++++++++++++ 13 files changed, 304 insertions(+) create mode 100644 client/ui/services/connection_test.go diff --git a/client/ui/i18n/locales/de/common.json b/client/ui/i18n/locales/de/common.json index 5e91e8d8880..d4c7320e0cc 100644 --- a/client/ui/i18n/locales/de/common.json +++ b/client/ui/i18n/locales/de/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "Trennen" }, + "tray.menu.reconnect": { + "message": "Neu verbinden" + }, "tray.menu.exitNode": { "message": "Exit Node" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "Trennen fehlgeschlagen" }, + "notify.error.reconnect": { + "message": "Neu verbinden fehlgeschlagen" + }, "notify.error.switchProfile": { "message": "Wechsel zu {profile} fehlgeschlagen" }, diff --git a/client/ui/i18n/locales/en/common.json b/client/ui/i18n/locales/en/common.json index b668146e89d..0310da6baba 100644 --- a/client/ui/i18n/locales/en/common.json +++ b/client/ui/i18n/locales/en/common.json @@ -79,6 +79,10 @@ "message": "Disconnect", "description": "Tray menu item that disconnects from the network. Keep short." }, + "tray.menu.reconnect": { + "message": "Reconnect", + "description": "Tray menu item that disconnects and connects again in one step. Keep short." + }, "tray.menu.exitNode": { "message": "Exit Node", "description": "Tray submenu title for choosing an exit node (route all traffic through another peer)." @@ -175,6 +179,10 @@ "message": "Failed to disconnect", "description": "Error notification body shown when disconnecting failed." }, + "notify.error.reconnect": { + "message": "Failed to reconnect", + "description": "Error notification body shown when reconnecting failed." + }, "notify.error.switchProfile": { "message": "Failed to switch to {profile}", "description": "Error notification shown when switching profiles failed. {profile} is the target profile name; keep it." diff --git a/client/ui/i18n/locales/es/common.json b/client/ui/i18n/locales/es/common.json index c036e4f754d..2a9c5082268 100644 --- a/client/ui/i18n/locales/es/common.json +++ b/client/ui/i18n/locales/es/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "Desconectar" }, + "tray.menu.reconnect": { + "message": "Reconectar" + }, "tray.menu.exitNode": { "message": "Nodo de salida" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "Error al desconectar" }, + "notify.error.reconnect": { + "message": "Error al reconectar" + }, "notify.error.switchProfile": { "message": "Error al cambiar a {profile}" }, diff --git a/client/ui/i18n/locales/fr/common.json b/client/ui/i18n/locales/fr/common.json index c6b91fb2530..dd8428199ed 100644 --- a/client/ui/i18n/locales/fr/common.json +++ b/client/ui/i18n/locales/fr/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "Se déconnecter" }, + "tray.menu.reconnect": { + "message": "Se reconnecter" + }, "tray.menu.exitNode": { "message": "Nœud de sortie" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "Échec de la déconnexion" }, + "notify.error.reconnect": { + "message": "Échec de la reconnexion" + }, "notify.error.switchProfile": { "message": "Échec du passage à {profile}" }, diff --git a/client/ui/i18n/locales/hu/common.json b/client/ui/i18n/locales/hu/common.json index dd5a1af6c6a..4260be76d89 100644 --- a/client/ui/i18n/locales/hu/common.json +++ b/client/ui/i18n/locales/hu/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "Bontás" }, + "tray.menu.reconnect": { + "message": "Újracsatlakozás" + }, "tray.menu.exitNode": { "message": "Exit Node" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "Bontás sikertelen" }, + "notify.error.reconnect": { + "message": "Újracsatlakozás sikertelen" + }, "notify.error.switchProfile": { "message": "Átváltás sikertelen erre: {profile}" }, diff --git a/client/ui/i18n/locales/it/common.json b/client/ui/i18n/locales/it/common.json index 7a2eb610cbf..ad7a98e1e39 100644 --- a/client/ui/i18n/locales/it/common.json +++ b/client/ui/i18n/locales/it/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "Disconnetti" }, + "tray.menu.reconnect": { + "message": "Riconnetti" + }, "tray.menu.exitNode": { "message": "Nodo di uscita" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "Disconnessione non riuscita" }, + "notify.error.reconnect": { + "message": "Riconnessione non riuscita" + }, "notify.error.switchProfile": { "message": "Impossibile passare a {profile}" }, diff --git a/client/ui/i18n/locales/ja/common.json b/client/ui/i18n/locales/ja/common.json index 326c825bf39..9e86695b18a 100644 --- a/client/ui/i18n/locales/ja/common.json +++ b/client/ui/i18n/locales/ja/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "切断" }, + "tray.menu.reconnect": { + "message": "再接続" + }, "tray.menu.exitNode": { "message": "出口ノード" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "切断に失敗しました" }, + "notify.error.reconnect": { + "message": "再接続に失敗しました" + }, "notify.error.switchProfile": { "message": "{profile} への切り替えに失敗しました" }, diff --git a/client/ui/i18n/locales/pt/common.json b/client/ui/i18n/locales/pt/common.json index 37b02d5a819..396e7d59020 100644 --- a/client/ui/i18n/locales/pt/common.json +++ b/client/ui/i18n/locales/pt/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "Desconectar" }, + "tray.menu.reconnect": { + "message": "Reconectar" + }, "tray.menu.exitNode": { "message": "Nó de saída" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "Falha ao desconectar" }, + "notify.error.reconnect": { + "message": "Falha ao reconectar" + }, "notify.error.switchProfile": { "message": "Falha ao alternar para {profile}" }, diff --git a/client/ui/i18n/locales/ru/common.json b/client/ui/i18n/locales/ru/common.json index b9ae59df26e..bcb258a3d9e 100644 --- a/client/ui/i18n/locales/ru/common.json +++ b/client/ui/i18n/locales/ru/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "Отключиться" }, + "tray.menu.reconnect": { + "message": "Переподключиться" + }, "tray.menu.exitNode": { "message": "Выходной узел" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "Не удалось отключиться" }, + "notify.error.reconnect": { + "message": "Не удалось переподключиться" + }, "notify.error.switchProfile": { "message": "Не удалось переключиться на {profile}" }, diff --git a/client/ui/i18n/locales/zh-CN/common.json b/client/ui/i18n/locales/zh-CN/common.json index 2141a770d41..707071ae894 100644 --- a/client/ui/i18n/locales/zh-CN/common.json +++ b/client/ui/i18n/locales/zh-CN/common.json @@ -59,6 +59,9 @@ "tray.menu.disconnect": { "message": "断开连接" }, + "tray.menu.reconnect": { + "message": "重新连接" + }, "tray.menu.exitNode": { "message": "出口节点" }, @@ -131,6 +134,9 @@ "notify.error.disconnect": { "message": "断开连接失败" }, + "notify.error.reconnect": { + "message": "重新连接失败" + }, "notify.error.switchProfile": { "message": "切换到 {profile} 失败" }, diff --git a/client/ui/services/connection.go b/client/ui/services/connection.go index fae7ddd239d..75188e74913 100644 --- a/client/ui/services/connection.go +++ b/client/ui/services/connection.go @@ -166,6 +166,25 @@ func (s *Connection) WaitSSOLoginAndUp(ctx context.Context, wait WaitSSOParams, return email, nil } +// Reconnect tears the session down and brings it straight back up, for the +// stale state a sleep/wake or a network roam leaves behind. Down is +// synchronous, so Up goes out only once the daemon reports the teardown +// finished, and a failed Down aborts rather than stacking an Up on a session +// that is still up. A ctx cancelled between the two legs aborts as well: a +// Disconnect or a quit racing the round trip must not be undone by a late Up. +func (s *Connection) Reconnect(ctx context.Context) error { + if err := s.Down(ctx); err != nil { + return fmt.Errorf("down: %w", err) + } + if err := ctx.Err(); err != nil { + return err + } + if err := s.Up(ctx, UpParams{}); err != nil { + return fmt.Errorf("up: %w", err) + } + return nil +} + func (s *Connection) Down(ctx context.Context) error { cli, err := s.conn.Client() if err != nil { diff --git a/client/ui/services/connection_test.go b/client/ui/services/connection_test.go new file mode 100644 index 00000000000..5cdf159446a --- /dev/null +++ b/client/ui/services/connection_test.go @@ -0,0 +1,79 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + + "github.com/netbirdio/netbird/client/proto" +) + +// fakeDaemonClient records the Down/Up calls Reconnect makes. Embedding the +// generated interface keeps the fake to the two RPCs under test. +type fakeDaemonClient struct { + proto.DaemonServiceClient + + downErr error + upErr error + calls []string + // onDown runs after Down is recorded, to simulate a Disconnect or a quit + // landing while the teardown is still in flight. + onDown func() +} + +func (c *fakeDaemonClient) Down(context.Context, *proto.DownRequest, ...grpc.CallOption) (*proto.DownResponse, error) { + c.calls = append(c.calls, "down") + if c.onDown != nil { + c.onDown() + } + if c.downErr != nil { + return nil, c.downErr + } + return &proto.DownResponse{}, nil +} + +func (c *fakeDaemonClient) Up(context.Context, *proto.UpRequest, ...grpc.CallOption) (*proto.UpResponse, error) { + c.calls = append(c.calls, "up") + if c.upErr != nil { + return nil, c.upErr + } + return &proto.UpResponse{}, nil +} + +type fakeDaemonConn struct{ client proto.DaemonServiceClient } + +func (c fakeDaemonConn) Client() (proto.DaemonServiceClient, error) { return c.client, nil } + +func TestConnectionReconnectSendsDownThenUp(t *testing.T) { + client := &fakeDaemonClient{} + conn := NewConnection(fakeDaemonConn{client: client}, nil, nil) + + require.NoError(t, conn.Reconnect(context.Background())) + require.Equal(t, []string{"down", "up"}, client.calls) +} + +func TestConnectionReconnectFailedDownSkipsUp(t *testing.T) { + client := &fakeDaemonClient{downErr: errors.New("teardown refused")} + conn := NewConnection(fakeDaemonConn{client: client}, nil, nil) + + require.Error(t, conn.Reconnect(context.Background())) + require.Equal(t, []string{"down"}, client.calls, + "Up must not stack on a session that is still up") +} + +func TestConnectionReconnectCancelDuringDownSkipsUp(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + // A tray Disconnect or a quit cancels the round trip mid-teardown; the Up + // must not bring the session back afterwards. + client := &fakeDaemonClient{onDown: cancel} + conn := NewConnection(fakeDaemonConn{client: client}, nil, nil) + + require.ErrorIs(t, conn.Reconnect(ctx), context.Canceled) + require.Equal(t, []string{"down"}, client.calls) +} diff --git a/client/ui/tray.go b/client/ui/tray.go index 3050d159acd..2ad365adbbf 100644 --- a/client/ui/tray.go +++ b/client/ui/tray.go @@ -4,6 +4,7 @@ package main import ( "context" + "errors" "runtime" "strings" "sync" @@ -31,6 +32,10 @@ const ( statusError = "Error" quitDownTimeout = 5 * time.Second + // reconnectTimeout bounds the Reconnect round trip so a daemon that never + // answers Down cannot leave the entry greyed out for the rest of the + // session. Matched to DaemonFeed's own switch-suppression window. + reconnectTimeout = 30 * time.Second urlGitHubRepo = "https://github.com/netbirdio/netbird" urlDocs = "https://docs.netbird.io" @@ -77,6 +82,7 @@ type Tray struct { sessionExpiresItem *application.MenuItem upItem *application.MenuItem downItem *application.MenuItem + reconnectItem *application.MenuItem exitNodeItem *application.MenuItem exitNodeSubmenu *application.Menu profileSubmenu *application.Menu @@ -87,6 +93,17 @@ type Tray struct { updater *trayUpdater + // reconnectMu guards reconnectCancel, which is non-nil exactly while the + // Reconnect entry's Down → Up round trip is in flight. It carries both + // duties: handleReconnect and relayoutMenu read it to keep a second click + // from sending a Down into the in-flight Up, and Disconnect/quit call it so + // the Up cannot bring the session back after the user asked for it to go + // down. The click-time SetEnabled(false) cannot stand in for the first duty: + // buildMenu recreates the item on every relayout, and the status pushes the + // round trip itself produces are what triggers those relayouts. + reconnectMu sync.Mutex + reconnectCancel context.CancelFunc + // statusMu guards the daemon-status core mirrored on the tray. One mutex // covers these fields because applyStatus writes them together on every // Status push and the menu painters read them. @@ -331,6 +348,16 @@ func (t *Tray) relayoutMenu() { t.downItem.SetHidden(!connected && !connecting) t.downItem.SetEnabled(connected || connecting) } + if t.reconnectItem != nil { + // Same visibility gate as Disconnect, but stays greyed out while a + // round trip is in flight: buildMenu has just recreated the item, so + // handleReconnect's SetEnabled(false) is gone by now. + t.reconnectMu.Lock() + reconnecting := t.reconnectCancel != nil + t.reconnectMu.Unlock() + t.reconnectItem.SetHidden(!connected && !connecting) + t.reconnectItem.SetEnabled((connected || connecting) && !reconnecting) + } if t.exitNodeItem != nil { t.exitNodeItem.SetEnabled(connected && len(exitNodeEntries) > 0 && !disableNetworks) } @@ -380,6 +407,13 @@ func (t *Tray) buildMenu() *application.Menu { downItem.OnClick(func(*application.Context) { t.handleDisconnect(downItem) }) downItem.SetHidden(true) t.downItem = downItem + // Reconnect collapses the Disconnect → wait → Connect dance users repeat + // after sleep or a network roam. It shares Disconnect's visibility: with no + // session to tear down, a reconnect is just Connect. + reconnectItem := menu.Add(t.loc.T("tray.menu.reconnect")) + reconnectItem.OnClick(func(*application.Context) { t.handleReconnect(reconnectItem) }) + reconnectItem.SetHidden(true) + t.reconnectItem = reconnectItem menu.AddSeparator() @@ -453,6 +487,7 @@ func (t *Tray) buildMenu() *application.Menu { func (t *Tray) handleQuit() { services.BeginShutdown() + t.cancelReconnect() t.profileMu.Lock() if t.switchCancel != nil { t.switchCancel() @@ -512,6 +547,8 @@ func (t *Tray) handleConnect(upItem *application.MenuItem) { // Receives the clicked item from the buildMenu closure (see handleConnect). func (t *Tray) handleDisconnect(downItem *application.MenuItem) { downItem.SetEnabled(false) + // A Reconnect mid-flight would otherwise finish its Up behind this Down. + t.cancelReconnect() t.profileMu.Lock() if t.switchCancel != nil { t.switchCancel() @@ -528,6 +565,113 @@ func (t *Tray) handleDisconnect(downItem *application.MenuItem) { }() } +// handleReconnect drives the Reconnect entry, for the stale sessions a +// wake-from-sleep or a network roam leaves behind. The Down → Up sequence +// itself is Connection.Reconnect; what lives here is the tray's share of it — +// the in-flight guard, the transitional paint, and the failure toast. Receives +// the clicked item from the buildMenu closure (see handleConnect). +// +// The transitional paint is DaemonFeed's: BeginProfileSwitch emits the +// optimistic Connecting and swallows the stale Connected/Idle blink the daemon +// pushes while Down tears the session down — the same sequence a profile switch +// performs, so it is reused rather than duplicated. It also arms the +// login-watch, so an Up that lands in NeedsLogin (an expired session, say) +// still opens browser-login instead of stalling. +func (t *Tray) handleReconnect(reconnectItem *application.MenuItem) { + // A profile switch is already a Down → Up. Cancelling one midway can leave + // the daemon and the CLI's on-disk profile state disagreeing (see + // ProfileSwitcher.switchActive), and the Up below carries no profile of its + // own, so which one came back would depend on where the cancel landed. + t.profileMu.Lock() + switching := t.switchCancel != nil + t.profileMu.Unlock() + if switching { + log.Infof("reconnect: profile switch in flight, ignoring click") + return + } + + ctx, ok := t.beginReconnect() + if !ok { + return + } + reconnectItem.SetEnabled(false) + + // DaemonFeed's login-watch owns the SSO handoff for this Up, so drop any + // flag handleConnect left armed: both firing on one NeedsLogin push would + // emit EventTriggerLogin twice. + t.statusMu.Lock() + t.pendingConnectLogin = false + t.statusMu.Unlock() + + t.svc.DaemonFeed.BeginProfileSwitch() + + go func() { + err := t.svc.Connection.Reconnect(ctx) + // Read before endReconnect, which cancels ctx itself and would make + // every outcome look aborted. Keyed off ctx rather than err because a + // cancel landing inside the Down RPC surfaces as a classified gRPC + // error, not context.Canceled. + aborted := errors.Is(ctx.Err(), context.Canceled) + t.endReconnect() + switch { + case err == nil: + case aborted: + // Disconnect or quit aborted us; that path paints its own state. + log.Infof("reconnect aborted: %v", err) + default: + log.Errorf("reconnect: %v", err) + // A failed Down left the daemon exactly as it was, and + // SubscribeStatus only pushes on state changes — waiting for one + // would strand the optimistic Connecting. Repaint from a snapshot. + t.svc.DaemonFeed.CancelProfileSwitch() + if st, serr := t.svc.DaemonFeed.Get(context.Background()); serr == nil { + t.applyStatus(st) + } + t.notifyError(t.loc.T("notify.error.reconnect")) + } + // reconnectItem is detached by now — buildMenu ran on the optimistic + // Connecting — so the row comes back through a relayout, not SetEnabled. + t.relayoutMenu() + }() +} + +// beginReconnect claims the reconnect slot and returns the context the round +// trip runs under, or false when one is already in flight. +func (t *Tray) beginReconnect() (context.Context, bool) { + t.reconnectMu.Lock() + defer t.reconnectMu.Unlock() + if t.reconnectCancel != nil { + return nil, false + } + ctx, cancel := context.WithTimeout(context.Background(), reconnectTimeout) + t.reconnectCancel = cancel + return ctx, true +} + +// endReconnect releases the slot and the context held since beginReconnect. +func (t *Tray) endReconnect() { + t.reconnectMu.Lock() + cancel := t.reconnectCancel + t.reconnectCancel = nil + t.reconnectMu.Unlock() + if cancel != nil { + cancel() + } +} + +// cancelReconnect aborts an in-flight round trip. Disconnect and quit both send +// their own Down; without this, the Up still queued behind ours would bring the +// session back up right after the user asked for it to go down. The slot itself +// is cleared by endReconnect once the round trip unwinds. +func (t *Tray) cancelReconnect() { + t.reconnectMu.Lock() + cancel := t.reconnectCancel + t.reconnectMu.Unlock() + if cancel != nil { + cancel() + } +} + // 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) {