Skip to content

[client] Update fyne and add exit menu retry#5187

Merged
mlsmaycon merged 4 commits intomainfrom
fix/exit-node-menu
Jan 27, 2026
Merged

[client] Update fyne and add exit menu retry#5187
mlsmaycon merged 4 commits intomainfrom
fix/exit-node-menu

Conversation

@mlsmaycon
Copy link
Copy Markdown
Collaborator

@mlsmaycon mlsmaycon commented Jan 26, 2026

Describe your changes

  • Fix an extra arrow on macos by updating fyne/systray

  • Add retry to get exit node items to workaround async status

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • New Features

    • Exit nodes are now automatically refreshed when the system tray is opened, ensuring current information.
  • Chores

    • Updated system tray dependency to include improvements and stability enhancements.

✏️ Tip: You can customize this high-level summary in your review settings.

- Fix an extra arrow on macos by updating fyne/systray

- Add retry to get exit node items to workaround async status
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 26, 2026

📝 Walkthrough

Walkthrough

The changes introduce a trigger to update exit nodes when the systray tray opens, update the systray dependency to a newer version, and remove an empty line from a network utility file.

Changes

Cohort / File(s) Summary
Systray dependency update
go.mod
Updated fyne.io/systray from v1.11.1-0.20250603113521-ca66a66d8b58 to v1.12.1-0.20260116214250-81f8e1a496f9
Event handling for tray open
client/ui/event_handler.go
Added new event-handling branch that calls h.client.updateExitNodes() when systray tray is opened (TrayOpenedCh)
Formatting cleanup
client/ui/network.go
Removed empty line between error return block and subsequent exitNodes retrieval

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • braginini
  • pappz

Poem

🐰 When the tray comes alive and opens wide,
Exit nodes update with rabbit pride,
A tiny fix, dependencies renewed,
The UI's behavior, gracefully tuned!
Hop hop hop, all systems refined! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title describes the main changes: updating fyne/systray and adding exit node retry, which aligns with the actual code changes across multiple files.
Description check ✅ Passed The description covers the key changes (fyne update, exit node retry) and completes required template sections, though the issue ticket and docs PR fields are unfilled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@client/ui/client_ui.go`:
- Line 921: Restore the original user-facing status text by changing the call to
s.mStatus.SetTitle("Connected2") back to s.mStatus.SetTitle("Connected"); this
corrects the accidental debug/typo in the status label without affecting the
macOS arrow or retry logic, so update the string in the s.mStatus.SetTitle
invocation accordingly.
🧹 Nitpick comments (1)
client/ui/network.go (1)

344-357: Consider extracting retry constants and adding debug logging.

The retry mechanism addresses the asynchronous status retrieval issue as intended. A few suggestions to improve maintainability:

  1. Extract the magic numbers (3 attempts, 1 second delay) as named constants
  2. Add debug logging to indicate retry attempts for easier troubleshooting
♻️ Suggested improvements
+const (
+	exitNodeRetryAttempts = 3
+	exitNodeRetryInterval = 1 * time.Second
+)
+
 func (s *serviceClient) updateExitNodes() {
 	conn, err := s.getSrvClient(defaultFailTimeout)
 	if err != nil {
 		log.Errorf("get client: %v", err)
 		return
 	}
 	// todo: on a new GUI version, we should implementing a better way to get routes information since
 	// the client might appear ready, it doesn't have the network map yet.
 	var exitNodes []*proto.Network
-	for i := 0; i < 3; i++ {
+	for i := 0; i < exitNodeRetryAttempts; i++ {
 		exitNodes, err = s.getExitNodes(conn)
 		if err != nil {
 			log.Errorf("get exit nodes: %v", err)
 			return
 		}
 		if len(exitNodes) != 0 {
 			break
 		}
-		time.Sleep(1 * time.Second)
+		log.Debugf("exit nodes empty, retrying (%d/%d)", i+1, exitNodeRetryAttempts)
+		time.Sleep(exitNodeRetryInterval)
 	}

Comment thread client/ui/client_ui.go Outdated
@sonarqubecloud
Copy link
Copy Markdown

@braginini braginini self-requested a review January 27, 2026 08:51
@mlsmaycon mlsmaycon merged commit 5299549 into main Jan 27, 2026
40 checks passed
@mlsmaycon mlsmaycon deleted the fix/exit-node-menu branch January 27, 2026 08:52
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