Skip to content

Commit fd0906f

Browse files
lixmallixmal
committed
Fix Connect/Disconnect buttons being enabled or disabled at the same time
1 parent 4545ab9 commit fd0906f

File tree

3 files changed

+81
-59
lines changed

3 files changed

+81
-59
lines changed

‎client/ui/client_ui.go‎

Lines changed: 34 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,9 @@ type serviceClient struct {
296296
mExitNodeDeselectAll *systray.MenuItem
297297
logFile string
298298
wLoginURL fyne.Window
299+
300+
connectMu sync.Mutex
301+
connectCancel context.CancelFunc
299302
}
300303

301304
type menuHandler struct {
@@ -592,102 +595,87 @@ func (s *serviceClient) getSettingsForm() *widget.Form {
592595
}
593596
}
594597

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

602604
activeProf, err := s.profileManager.GetActiveProfile()
603605
if err != nil {
604-
log.Errorf("get active profile: %v", err)
605-
return nil, err
606+
return nil, fmt.Errorf("get active profile: %w", err)
606607
}
607608

608609
currUser, err := user.Current()
609610
if err != nil {
610611
return nil, fmt.Errorf("get current user: %w", err)
611612
}
612613

613-
loginResp, err := conn.Login(s.ctx, &proto.LoginRequest{
614+
loginResp, err := conn.Login(ctx, &proto.LoginRequest{
614615
IsUnixDesktopClient: runtime.GOOS == "linux" || runtime.GOOS == "freebsd",
615616
ProfileName: &activeProf.Name,
616617
Username: &currUser.Username,
617618
})
618619
if err != nil {
619-
log.Errorf("login to management URL with: %v", err)
620-
return nil, err
620+
return nil, fmt.Errorf("login to management: %w", err)
621621
}
622622

623623
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
624+
if err = s.handleSSOLogin(ctx, loginResp, conn); err != nil {
625+
return nil, fmt.Errorf("SSO login: %w", err)
628626
}
629627
}
630628

631629
return loginResp, nil
632630
}
633631

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
632+
func (s *serviceClient) handleSSOLogin(ctx context.Context, loginResp *proto.LoginResponse, conn proto.DaemonServiceClient) error {
633+
if err := openURL(loginResp.VerificationURIComplete); err != nil {
634+
return fmt.Errorf("open browser: %w", err)
639635
}
640636

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

647642
if resp.Email != "" {
648-
err := s.profileManager.SetActiveProfileState(&profilemanager.ProfileState{
643+
if err := s.profileManager.SetActiveProfileState(&profilemanager.ProfileState{
649644
Email: resp.Email,
650-
})
651-
if err != nil {
652-
log.Warnf("failed to set profile state: %v", err)
645+
}); err != nil {
646+
log.Debugf("failed to set profile state: %v", err)
653647
} else {
654648
s.mProfile.refresh()
655649
}
656-
657650
}
658651

659652
return nil
660653
}
661654

662-
func (s *serviceClient) menuUpClick() error {
655+
func (s *serviceClient) menuUpClick(ctx context.Context) error {
663656
systray.SetTemplateIcon(iconConnectingMacOS, s.icConnecting)
664657
conn, err := s.getSrvClient(defaultFailTimeout)
665658
if err != nil {
666659
systray.SetTemplateIcon(iconErrorMacOS, s.icError)
667-
log.Errorf("get client: %v", err)
668-
return err
660+
return fmt.Errorf("get daemon client: %w", err)
669661
}
670662

671-
_, err = s.login(true)
663+
_, err = s.login(ctx, true)
672664
if err != nil {
673-
log.Errorf("login failed with: %v", err)
674-
return err
665+
return fmt.Errorf("login: %w", err)
675666
}
676667

677-
status, err := conn.Status(s.ctx, &proto.StatusRequest{})
668+
status, err := conn.Status(ctx, &proto.StatusRequest{})
678669
if err != nil {
679-
log.Errorf("get service status: %v", err)
680-
return err
670+
return fmt.Errorf("get status: %w", err)
681671
}
682672

683673
if status.Status == string(internal.StatusConnected) {
684-
log.Warnf("already connected")
685674
return nil
686675
}
687676

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

693681
return nil
@@ -697,24 +685,20 @@ func (s *serviceClient) menuDownClick() error {
697685
systray.SetTemplateIcon(iconConnectingMacOS, s.icConnecting)
698686
conn, err := s.getSrvClient(defaultFailTimeout)
699687
if err != nil {
700-
log.Errorf("get client: %v", err)
701-
return err
688+
return fmt.Errorf("get daemon client: %w", err)
702689
}
703690

704691
status, err := conn.Status(s.ctx, &proto.StatusRequest{})
705692
if err != nil {
706-
log.Errorf("get service status: %v", err)
707-
return err
693+
return fmt.Errorf("get status: %w", err)
708694
}
709695

710696
if status.Status != string(internal.StatusConnected) && status.Status != string(internal.StatusConnecting) {
711-
log.Warnf("already down")
712697
return nil
713698
}
714699

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

720704
return nil
@@ -1381,7 +1365,7 @@ func (s *serviceClient) showLoginURL() context.CancelFunc {
13811365
return
13821366
}
13831367

1384-
resp, err := s.login(false)
1368+
resp, err := s.login(ctx, false)
13851369
if err != nil {
13861370
log.Errorf("failed to fetch login URL: %v", err)
13871371
return
@@ -1401,15 +1385,15 @@ func (s *serviceClient) showLoginURL() context.CancelFunc {
14011385
return
14021386
}
14031387

1404-
_, err = conn.WaitSSOLogin(s.ctx, &proto.WaitSSOLoginRequest{UserCode: resp.UserCode})
1388+
_, err = conn.WaitSSOLogin(ctx, &proto.WaitSSOLoginRequest{UserCode: resp.UserCode})
14051389
if err != nil {
14061390
log.Errorf("Waiting sso login failed with: %v", err)
14071391
label.SetText("Waiting login failed, please create \na debug bundle in the settings and contact support.")
14081392
return
14091393
}
14101394

14111395
label.SetText("Re-authentication successful.\nReconnecting")
1412-
status, err := conn.Status(s.ctx, &proto.StatusRequest{})
1396+
status, err := conn.Status(ctx, &proto.StatusRequest{})
14131397
if err != nil {
14141398
log.Errorf("get service status: %v", err)
14151399
return
@@ -1422,7 +1406,7 @@ func (s *serviceClient) showLoginURL() context.CancelFunc {
14221406
return
14231407
}
14241408

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

‎client/ui/event_handler.go‎

Lines changed: 44 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,21 +69,57 @@ func (h *eventHandler) listen(ctx context.Context) {
6769

6870
func (h *eventHandler) handleConnectClick() {
6971
h.client.mUp.Disable()
72+
73+
connectCtx, connectCancel := context.WithCancel(h.client.ctx)
74+
75+
h.client.connectMu.Lock()
76+
h.client.connectCancel = connectCancel
77+
h.client.connectMu.Unlock()
78+
7079
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"))
80+
defer func() {
81+
h.client.connectMu.Lock()
82+
h.client.connectCancel = nil
83+
h.client.connectMu.Unlock()
84+
}()
85+
86+
if err := h.client.menuUpClick(connectCtx); err != nil {
87+
st, ok := status.FromError(err)
88+
if errors.Is(err, context.Canceled) || (ok && st.Code() == codes.Canceled) {
89+
log.Debugf("connect operation cancelled by user")
90+
} else {
91+
h.client.app.SendNotification(fyne.NewNotification("Error", "Failed to connect"))
92+
log.Errorf("connect failed: %v", err)
93+
}
7494
}
95+
96+
go h.client.updateStatus()
7597
}()
7698
}
7799

78100
func (h *eventHandler) handleDisconnectClick() {
79101
h.client.mDown.Disable()
80102
go func() {
81-
defer h.client.mDown.Enable()
103+
h.client.connectMu.Lock()
104+
cancel := h.client.connectCancel
105+
h.client.connectMu.Unlock()
106+
107+
if cancel != nil {
108+
log.Debugf("cancelling ongoing connect operation")
109+
cancel()
110+
}
111+
82112
if err := h.client.menuDownClick(); err != nil {
83-
h.client.app.SendNotification(fyne.NewNotification("Error", "Failed to connect to NetBird daemon"))
113+
st, ok := status.FromError(err)
114+
if !errors.Is(err, context.Canceled) && !(ok && st.Code() == codes.Canceled) {
115+
h.client.app.SendNotification(fyne.NewNotification("Error", "Failed to disconnect"))
116+
log.Errorf("disconnect failed: %v", err)
117+
} else {
118+
log.Debugf("disconnect cancelled or already disconnecting")
119+
}
84120
}
121+
122+
go h.client.updateStatus()
85123
}()
86124
}
87125

@@ -245,6 +283,6 @@ func (h *eventHandler) logout(ctx context.Context) error {
245283
}
246284

247285
h.client.getSrvConfig()
248-
286+
249287
return nil
250288
}

‎client/ui/profile.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ type profileMenu struct {
396396
logoutSubItem *subItem
397397
profilesState []Profile
398398
downClickCallback func() error
399-
upClickCallback func() error
399+
upClickCallback func(context.Context) error
400400
getSrvClientCallback func(timeout time.Duration) (proto.DaemonServiceClient, error)
401401
loadSettingsCallback func()
402402
app fyne.App
@@ -409,7 +409,7 @@ type newProfileMenuArgs struct {
409409
profileMenuItem *systray.MenuItem
410410
emailMenuItem *systray.MenuItem
411411
downClickCallback func() error
412-
upClickCallback func() error
412+
upClickCallback func(context.Context) error
413413
getSrvClientCallback func(timeout time.Duration) (proto.DaemonServiceClient, error)
414414
loadSettingsCallback func()
415415
app fyne.App
@@ -569,7 +569,7 @@ func (p *profileMenu) refresh() {
569569
}
570570
}
571571

572-
if err := p.upClickCallback(); err != nil {
572+
if err := p.upClickCallback(p.ctx); err != nil {
573573
log.Errorf("failed to handle up click after switching profile: %v", err)
574574
}
575575

0 commit comments

Comments
 (0)