Skip to content

Commit d3a34ad

Browse files
lixmallixmal
authored
[client] Fix Connect/Disconnect buttons being enabled or disabled at the same time (#4711)
1 parent d7321c1 commit d3a34ad

File tree

3 files changed

+91
-59
lines changed

3 files changed

+91
-59
lines changed

‎client/ui/client_ui.go‎

Lines changed: 33 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,8 @@ type serviceClient struct {
296296
mExitNodeDeselectAll *systray.MenuItem
297297
logFile string
298298
wLoginURL fyne.Window
299+
300+
connectCancel context.CancelFunc
299301
}
300302

301303
type menuHandler struct {
@@ -592,102 +594,87 @@ func (s *serviceClient) getSettingsForm() *widget.Form {
592594
}
593595
}
594596

595-
func (s *serviceClient) login(openURL bool) (*proto.LoginResponse, error) {
597+
func (s *serviceClient) login(ctx context.Context, openURL bool) (*proto.LoginResponse, error) {
596598
conn, err := s.getSrvClient(defaultFailTimeout)
597599
if err != nil {
598-
log.Errorf("get client: %v", err)
599-
return nil, err
600+
return nil, fmt.Errorf("get daemon client: %w", err)
600601
}
601602

602603
activeProf, err := s.profileManager.GetActiveProfile()
603604
if err != nil {
604-
log.Errorf("get active profile: %v", err)
605-
return nil, err
605+
return nil, fmt.Errorf("get active profile: %w", err)
606606
}
607607

608608
currUser, err := user.Current()
609609
if err != nil {
610610
return nil, fmt.Errorf("get current user: %w", err)
611611
}
612612

613-
loginResp, err := conn.Login(s.ctx, &proto.LoginRequest{
613+
loginResp, err := conn.Login(ctx, &proto.LoginRequest{
614614
IsUnixDesktopClient: runtime.GOOS == "linux" || runtime.GOOS == "freebsd",
615615
ProfileName: &activeProf.Name,
616616
Username: &currUser.Username,
617617
})
618618
if err != nil {
619-
log.Errorf("login to management URL with: %v", err)
620-
return nil, err
619+
return nil, fmt.Errorf("login to management: %w", err)
621620
}
622621

623622
if loginResp.NeedsSSOLogin && openURL {
624-
err = s.handleSSOLogin(loginResp, conn)
625-
if err != nil {
626-
log.Errorf("handle SSO login failed: %v", err)
627-
return nil, err
623+
if err = s.handleSSOLogin(ctx, loginResp, conn); err != nil {
624+
return nil, fmt.Errorf("SSO login: %w", err)
628625
}
629626
}
630627

631628
return loginResp, nil
632629
}
633630

634-
func (s *serviceClient) handleSSOLogin(loginResp *proto.LoginResponse, conn proto.DaemonServiceClient) error {
635-
err := openURL(loginResp.VerificationURIComplete)
636-
if err != nil {
637-
log.Errorf("opening the verification uri in the browser failed: %v", err)
638-
return err
631+
func (s *serviceClient) handleSSOLogin(ctx context.Context, loginResp *proto.LoginResponse, conn proto.DaemonServiceClient) error {
632+
if err := openURL(loginResp.VerificationURIComplete); err != nil {
633+
return fmt.Errorf("open browser: %w", err)
639634
}
640635

641-
resp, err := conn.WaitSSOLogin(s.ctx, &proto.WaitSSOLoginRequest{UserCode: loginResp.UserCode})
636+
resp, err := conn.WaitSSOLogin(ctx, &proto.WaitSSOLoginRequest{UserCode: loginResp.UserCode})
642637
if err != nil {
643-
log.Errorf("waiting sso login failed with: %v", err)
644-
return err
638+
return fmt.Errorf("wait for SSO login: %w", err)
645639
}
646640

647641
if resp.Email != "" {
648-
err := s.profileManager.SetActiveProfileState(&profilemanager.ProfileState{
642+
if err := s.profileManager.SetActiveProfileState(&profilemanager.ProfileState{
649643
Email: resp.Email,
650-
})
651-
if err != nil {
652-
log.Warnf("failed to set profile state: %v", err)
644+
}); err != nil {
645+
log.Debugf("failed to set profile state: %v", err)
653646
} else {
654647
s.mProfile.refresh()
655648
}
656-
657649
}
658650

659651
return nil
660652
}
661653

662-
func (s *serviceClient) menuUpClick() error {
654+
func (s *serviceClient) menuUpClick(ctx context.Context) error {
663655
systray.SetTemplateIcon(iconConnectingMacOS, s.icConnecting)
664656
conn, err := s.getSrvClient(defaultFailTimeout)
665657
if err != nil {
666658
systray.SetTemplateIcon(iconErrorMacOS, s.icError)
667-
log.Errorf("get client: %v", err)
668-
return err
659+
return fmt.Errorf("get daemon client: %w", err)
669660
}
670661

671-
_, err = s.login(true)
662+
_, err = s.login(ctx, true)
672663
if err != nil {
673-
log.Errorf("login failed with: %v", err)
674-
return err
664+
return fmt.Errorf("login: %w", err)
675665
}
676666

677-
status, err := conn.Status(s.ctx, &proto.StatusRequest{})
667+
status, err := conn.Status(ctx, &proto.StatusRequest{})
678668
if err != nil {
679-
log.Errorf("get service status: %v", err)
680-
return err
669+
return fmt.Errorf("get status: %w", err)
681670
}
682671

683672
if status.Status == string(internal.StatusConnected) {
684-
log.Warnf("already connected")
685673
return nil
686674
}
687675

688-
if _, err := s.conn.Up(s.ctx, &proto.UpRequest{}); err != nil {
689-
log.Errorf("up service: %v", err)
690-
return err
676+
if _, err := conn.Up(ctx, &proto.UpRequest{}); err != nil {
677+
return fmt.Errorf("start connection: %w", err)
691678
}
692679

693680
return nil
@@ -697,24 +684,20 @@ func (s *serviceClient) menuDownClick() error {
697684
systray.SetTemplateIcon(iconConnectingMacOS, s.icConnecting)
698685
conn, err := s.getSrvClient(defaultFailTimeout)
699686
if err != nil {
700-
log.Errorf("get client: %v", err)
701-
return err
687+
return fmt.Errorf("get daemon client: %w", err)
702688
}
703689

704690
status, err := conn.Status(s.ctx, &proto.StatusRequest{})
705691
if err != nil {
706-
log.Errorf("get service status: %v", err)
707-
return err
692+
return fmt.Errorf("get status: %w", err)
708693
}
709694

710695
if status.Status != string(internal.StatusConnected) && status.Status != string(internal.StatusConnecting) {
711-
log.Warnf("already down")
712696
return nil
713697
}
714698

715-
if _, err := s.conn.Down(s.ctx, &proto.DownRequest{}); err != nil {
716-
log.Errorf("down service: %v", err)
717-
return err
699+
if _, err := conn.Down(s.ctx, &proto.DownRequest{}); err != nil {
700+
return fmt.Errorf("stop connection: %w", err)
718701
}
719702

720703
return nil
@@ -1381,7 +1364,7 @@ func (s *serviceClient) showLoginURL() context.CancelFunc {
13811364
return
13821365
}
13831366

1384-
resp, err := s.login(false)
1367+
resp, err := s.login(ctx, false)
13851368
if err != nil {
13861369
log.Errorf("failed to fetch login URL: %v", err)
13871370
return
@@ -1401,15 +1384,15 @@ func (s *serviceClient) showLoginURL() context.CancelFunc {
14011384
return
14021385
}
14031386

1404-
_, err = conn.WaitSSOLogin(s.ctx, &proto.WaitSSOLoginRequest{UserCode: resp.UserCode})
1387+
_, err = conn.WaitSSOLogin(ctx, &proto.WaitSSOLoginRequest{UserCode: resp.UserCode})
14051388
if err != nil {
14061389
log.Errorf("Waiting sso login failed with: %v", err)
14071390
label.SetText("Waiting login failed, please create \na debug bundle in the settings and contact support.")
14081391
return
14091392
}
14101393

14111394
label.SetText("Re-authentication successful.\nReconnecting")
1412-
status, err := conn.Status(s.ctx, &proto.StatusRequest{})
1395+
status, err := conn.Status(ctx, &proto.StatusRequest{})
14131396
if err != nil {
14141397
log.Errorf("get service status: %v", err)
14151398
return
@@ -1422,7 +1405,7 @@ func (s *serviceClient) showLoginURL() context.CancelFunc {
14221405
return
14231406
}
14241407

1425-
_, err = conn.Up(s.ctx, &proto.UpRequest{})
1408+
_, err = conn.Up(ctx, &proto.UpRequest{})
14261409
if err != nil {
14271410
label.SetText("Reconnecting failed, please create \na debug bundle in the settings and contact support.")
14281411
log.Errorf("Reconnecting failed with: %v", err)

‎client/ui/event_handler.go‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"fyne.io/fyne/v2"
1313
"fyne.io/systray"
1414
log "github.com/sirupsen/logrus"
15+
"google.golang.org/grpc/codes"
16+
"google.golang.org/grpc/status"
1517

1618
"github.com/netbirdio/netbird/client/proto"
1719
"github.com/netbirdio/netbird/version"
@@ -67,20 +69,55 @@ func (h *eventHandler) listen(ctx context.Context) {
6769

6870
func (h *eventHandler) handleConnectClick() {
6971
h.client.mUp.Disable()
72+
73+
if h.client.connectCancel != nil {
74+
h.client.connectCancel()
75+
}
76+
77+
connectCtx, connectCancel := context.WithCancel(h.client.ctx)
78+
h.client.connectCancel = connectCancel
79+
7080
go func() {
71-
defer h.client.mUp.Enable()
72-
if err := h.client.menuUpClick(); err != nil {
73-
h.client.app.SendNotification(fyne.NewNotification("Error", "Failed to connect to NetBird service"))
81+
defer connectCancel()
82+
83+
if err := h.client.menuUpClick(connectCtx); err != nil {
84+
st, ok := status.FromError(err)
85+
if errors.Is(err, context.Canceled) || (ok && st.Code() == codes.Canceled) {
86+
log.Debugf("connect operation cancelled by user")
87+
} else {
88+
h.client.app.SendNotification(fyne.NewNotification("Error", "Failed to connect"))
89+
log.Errorf("connect failed: %v", err)
90+
}
91+
}
92+
93+
if err := h.client.updateStatus(); err != nil {
94+
log.Debugf("failed to update status after connect: %v", err)
7495
}
7596
}()
7697
}
7798

7899
func (h *eventHandler) handleDisconnectClick() {
79100
h.client.mDown.Disable()
101+
102+
if h.client.connectCancel != nil {
103+
log.Debugf("cancelling ongoing connect operation")
104+
h.client.connectCancel()
105+
h.client.connectCancel = nil
106+
}
107+
80108
go func() {
81-
defer h.client.mDown.Enable()
82109
if err := h.client.menuDownClick(); err != nil {
83-
h.client.app.SendNotification(fyne.NewNotification("Error", "Failed to connect to NetBird daemon"))
110+
st, ok := status.FromError(err)
111+
if !errors.Is(err, context.Canceled) && !(ok && st.Code() == codes.Canceled) {
112+
h.client.app.SendNotification(fyne.NewNotification("Error", "Failed to disconnect"))
113+
log.Errorf("disconnect failed: %v", err)
114+
} else {
115+
log.Debugf("disconnect cancelled or already disconnecting")
116+
}
117+
}
118+
119+
if err := h.client.updateStatus(); err != nil {
120+
log.Debugf("failed to update status after disconnect: %v", err)
84121
}
85122
}()
86123
}
@@ -245,6 +282,6 @@ func (h *eventHandler) logout(ctx context.Context) error {
245282
}
246283

247284
h.client.getSrvConfig()
248-
285+
249286
return nil
250287
}

‎client/ui/profile.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ type subItem struct {
387387
type profileMenu struct {
388388
mu sync.Mutex
389389
ctx context.Context
390+
serviceClient *serviceClient
390391
profileManager *profilemanager.ProfileManager
391392
eventHandler *eventHandler
392393
profileMenuItem *systray.MenuItem
@@ -396,20 +397,21 @@ type profileMenu struct {
396397
logoutSubItem *subItem
397398
profilesState []Profile
398399
downClickCallback func() error
399-
upClickCallback func() error
400+
upClickCallback func(context.Context) error
400401
getSrvClientCallback func(timeout time.Duration) (proto.DaemonServiceClient, error)
401402
loadSettingsCallback func()
402403
app fyne.App
403404
}
404405

405406
type newProfileMenuArgs struct {
406407
ctx context.Context
408+
serviceClient *serviceClient
407409
profileManager *profilemanager.ProfileManager
408410
eventHandler *eventHandler
409411
profileMenuItem *systray.MenuItem
410412
emailMenuItem *systray.MenuItem
411413
downClickCallback func() error
412-
upClickCallback func() error
414+
upClickCallback func(context.Context) error
413415
getSrvClientCallback func(timeout time.Duration) (proto.DaemonServiceClient, error)
414416
loadSettingsCallback func()
415417
app fyne.App
@@ -418,6 +420,7 @@ type newProfileMenuArgs struct {
418420
func newProfileMenu(args newProfileMenuArgs) *profileMenu {
419421
p := profileMenu{
420422
ctx: args.ctx,
423+
serviceClient: args.serviceClient,
421424
profileManager: args.profileManager,
422425
eventHandler: args.eventHandler,
423426
profileMenuItem: args.profileMenuItem,
@@ -569,10 +572,19 @@ func (p *profileMenu) refresh() {
569572
}
570573
}
571574

572-
if err := p.upClickCallback(); err != nil {
575+
if p.serviceClient.connectCancel != nil {
576+
p.serviceClient.connectCancel()
577+
}
578+
579+
connectCtx, connectCancel := context.WithCancel(p.ctx)
580+
p.serviceClient.connectCancel = connectCancel
581+
582+
if err := p.upClickCallback(connectCtx); err != nil {
573583
log.Errorf("failed to handle up click after switching profile: %v", err)
574584
}
575585

586+
connectCancel()
587+
576588
p.refresh()
577589
p.loadSettingsCallback()
578590
}

0 commit comments

Comments
 (0)