-
Notifications
You must be signed in to change notification settings - Fork 0
Feature implementation from commits de7384e..870e29d #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
[client] Fix DNS Interceptor Build Error
* Avoid recalculating next peer expiration - Check if an account schedule is already running - Cancel executing schedules only when changes occurs - Add more context info to logs * fix tests
…te (netbirdio#3994) [client] Fix logic in updateStatus to correctly handle connection state (netbirdio#3994)
[client] fix connection state handling (netbirdio#3995)
On Android ignore the dynamic roots in the route notifications
…ewall rules port range support (netbirdio#4003) Adds backward compatibility for clients with versions prior to v0.48.0 that do not support port range firewall rules. - Skips generation of firewall rules with multi-port ranges for older clients - Preserves support for single-port ranges by treating them as individual port rules, ensuring compatibility with older clients
…nd primary account update (netbirdio#4014)
…dio#3943) * Activate new lazy routing peers if the HA group is active * Prevent lazy peers going to idle if HA group members are active (netbirdio#3948)
…on status checks (netbirdio#4026) This PR refactors showLoginURL to improve error handling and connection status checks by delaying the login fetch until user interaction and closing the pop-up if already connected. - Moved s.login(false) call into the click handler to defer network I/O. - Added a conn.Status check after opening the URL to skip reconnection if already connected. - Enhanced error logs for missing verification URLs and service status failures.
This PR fixes a bug by ensuring that the advanced settings and re-authentication windows are closed appropriately when the main GUI process exits. - Updated runSelfCommand calls throughout the UI to pass a context parameter. - Modified runSelfCommand’s signature and its internal command invocation to use exec.CommandContext for proper cancellation handling.
* add additional metrics we are collecting active rosenpass, ssh from the client side we are also collecting active user peers and active users * remove duplicated
| } | ||
|
|
||
| func (m *Manager) AddPeer(peerCfg lazyconn.PeerConfig) (bool, error) { | ||
| func (m *Manager) AddPeer(ctx context.Context, peerCfg lazyconn.PeerConfig) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Correctness Issue
Breaking API Change: Missing Context Parameter.
Adding a context parameter to AddPeer() method will break all existing callers that don't provide this parameter, causing compilation failures.
📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| func (m *Manager) AddPeer(ctx context.Context, peerCfg lazyconn.PeerConfig) (bool, error) { | |
| func (m *Manager) AddPeer(ctx context.Context, peerCfg lazyconn.PeerConfig) (bool, error) { |
| go func() { | ||
| defer h.client.mNetworks.Enable() | ||
| h.runSelfCommand("networks", "true") | ||
| h.runSelfCommand(h.client.ctx, "networks", "true") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Correctness Issue
Missing required context parameter.
The runSelfCommand function requires a context parameter that was missing, which would cause compilation failure or runtime errors.
Current Code (Diff):
- h.runSelfCommand("networks", "true")
+ h.runSelfCommand(h.client.ctx, "networks", "true")📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| h.runSelfCommand(h.client.ctx, "networks", "true") | |
| h.runSelfCommand(h.client.ctx, "networks", "true") |
|
|
||
| for _, p := range userPeers { | ||
| aclPeers, _ := account.GetPeerConnectionResources(ctx, p.ID, approvedPeersMap) | ||
| aclPeers, _ := account.GetPeerConnectionResources(ctx, p, approvedPeersMap) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Correctness Issue
Type mismatch in function parameter.
Changed from passing p.ID to passing the entire p object to GetPeerConnectionResources, which could cause runtime errors if the function expects an ID rather than an object.
Current Code (Diff):
- aclPeers, _ := account.GetPeerConnectionResources(ctx, p, approvedPeersMap)
+ aclPeers, _ := account.GetPeerConnectionResources(ctx, p.ID, approvedPeersMap)📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| aclPeers, _ := account.GetPeerConnectionResources(ctx, p, approvedPeersMap) | |
| aclPeers, _ := account.GetPeerConnectionResources(ctx, p.ID, approvedPeersMap) |
| } | ||
|
|
||
| if len(routeToSave.Domains) > 0 { | ||
| routeToSave.Network = getPlaceholderIP() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Correctness Issue
Undefined function getPlaceholderIP().
The code calls getPlaceholderIP() which is not defined in this hunk, potentially causing runtime errors if not defined elsewhere in the codebase.
| return err | ||
| } | ||
|
|
||
| return checkRoutePrefixOrDomainsExistForPeers(ctx, transaction, accountID, routeToSave, groupsMap) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Correctness Issue
Undefined function checkRoutePrefixOrDomainsExistForPeers().
The function checkRoutePrefixOrDomainsExistForPeers() is called but not defined in this hunk, which could lead to runtime errors if not defined elsewhere.
| } | ||
|
|
||
| if len(routeToSave.PeerGroups) > 0 { | ||
| if err = validateGroups(routeToSave.PeerGroups, groupsMap); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Correctness Issue
Undefined function validateGroups().
The function validateGroups() is called multiple times but not defined in this hunk, potentially causing runtime errors if not defined elsewhere.
This PR contains changes from a range of commits from the original repository.
Commit Range:
de7384e..870e29dFiles Changed: 41 (39 programming files)
Programming Ratio: 95.1%
Commits included:
... and 5 more commits