Resolve copilot comments

Author: mohamed-essam

client/internal/updatemanager/manager.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"time"
 
@@ -50,7 +51,7 @@ func NewUpdateManager(statusRecorder *peer.Status) *UpdateManager {
 
 func (u *UpdateManager) SetVersion(v string) {
 	if u.version != v {
-		log.Tracef("Auto-update verstion set to %s", v)
+		log.Tracef("Auto-update version set to %s", v)
 		u.version = v
 		go u.CheckForUpdates()
 	}
@@ -145,3 +146,9 @@ func downloadFileToTemporaryDir(ctx context.Context, fileURL string) (string, er
 
 	return out.Name(), nil
 }
+
+func urlWithVersionArch(url, version string) string { //nolint:unused
+	url = strings.ReplaceAll(url, "%version", version)
+	url = strings.ReplaceAll(url, "%arch", runtime.GOARCH)
+	return url
+}

client/internal/updatemanager/update_darwin.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"os/exec"
 	"os/user"
-	"runtime"
 	"strings"
 	"syscall"
 )
@@ -25,9 +24,7 @@ func (u *UpdateManager) triggerUpdate(targetVersion string) error {
 		return u.updateHomeBrew()
 	}
 	// Installed using pkg file
-	url := strings.ReplaceAll(pkgDownloadURL, "%version", targetVersion)
-	url = strings.ReplaceAll(url, "%arch", runtime.GOARCH)
-	path, err := downloadFileToTemporaryDir(u.ctx, url)
+	path, err := downloadFileToTemporaryDir(u.ctx, urlWithVersionArch(pkgDownloadURL, targetVersion))
 	if err != nil {
 		return fmt.Errorf("error downloading update file: %w", err)
 	}
@@ -99,7 +96,7 @@ func (u *UpdateManager) updateHomeBrew() error {
 	// Restart netbird service after the fact
 	// This is a workaround since attempting to restart using launchctl will kill the service and die before starting
 	// the service again as it's a child process
-	// using SigTerm should ensure a clean shutdown
+	// using SIGTERM should ensure a clean shutdown
 	process, err := os.FindProcess(currentPID)
 	if err != nil {
 		return fmt.Errorf("error finding current process: %w", err)

client/internal/updatemanager/update_windows.go

@@ -4,11 +4,8 @@ package updatemanager
 
 import (
 	"fmt"
-	"os/exec"
-	"runtime"
-	"strings"
-
 	log "github.com/sirupsen/logrus"
+	"os/exec"
 
 	"golang.org/x/sys/windows/registry"
 )
@@ -42,9 +39,7 @@ func installationMethod() string {
 }
 
 func (u *UpdateManager) updateMSI(targetVersion string) error {
-	url := strings.ReplaceAll(msiDownloadURL, "%version", targetVersion)
-	url = strings.ReplaceAll(url, "%arch", runtime.GOARCH)
-	path, err := downloadFileToTemporaryDir(u.ctx, url)
+	path, err := downloadFileToTemporaryDir(u.ctx, urlWithVersionArch(msiDownloadURL, targetVersion))
 	if err != nil {
 		return err
 	}
@@ -54,9 +49,7 @@ func (u *UpdateManager) updateMSI(targetVersion string) error {
 }
 
 func (u *UpdateManager) updateEXE(targetVersion string) error {
-	url := strings.ReplaceAll(exeDownloadURL, "%version", targetVersion)
-	url = strings.ReplaceAll(url, "%arch", runtime.GOARCH)
-	path, err := downloadFileToTemporaryDir(u.ctx, url)
+	path, err := downloadFileToTemporaryDir(u.ctx, urlWithVersionArch(exeDownloadURL, targetVersion))
 	if err != nil {
 		return err
 	}

management/server/http/handlers/accounts/accounts_handler.go

@@ -3,6 +3,7 @@ package accounts
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/netip"
 	"time"
@@ -12,11 +13,11 @@ import (
 	goversion "github.com/hashicorp/go-version"
 	"github.com/netbirdio/netbird/management/server/account"
 	nbcontext "github.com/netbirdio/netbird/management/server/context"
+	"github.com/netbirdio/netbird/management/server/settings"
+	"github.com/netbirdio/netbird/management/server/types"
 	"github.com/netbirdio/netbird/shared/management/http/api"
 	"github.com/netbirdio/netbird/shared/management/http/util"
-	"github.com/netbirdio/netbird/management/server/settings"
 	"github.com/netbirdio/netbird/shared/management/status"
-	"github.com/netbirdio/netbird/management/server/types"
 )
 
 const (
@@ -163,7 +164,7 @@ func (h *handler) getAllAccounts(w http.ResponseWriter, r *http.Request) {
 	util.WriteJSONObject(r.Context(), w, []*api.Account{resp})
 }
 
-func (h *handler) updateAccountRequestSettings(req api.PutApiAccountsAccountIdJSONRequestBody, w http.ResponseWriter) *types.Settings {
+func (h *handler) updateAccountRequestSettings(req api.PutApiAccountsAccountIdJSONRequestBody) (*types.Settings, error) {
 	returnSettings := &types.Settings{
 		PeerLoginExpirationEnabled: req.Settings.PeerLoginExpirationEnabled,
 		PeerLoginExpiration:        time.Duration(float64(time.Second.Nanoseconds()) * float64(req.Settings.PeerLoginExpiration)),
@@ -209,12 +210,11 @@ func (h *handler) updateAccountRequestSettings(req api.PutApiAccountsAccountIdJS
 			err == nil {
 			returnSettings.AutoUpdateVersion = *req.Settings.AutoUpdateVersion
 		} else if *req.Settings.AutoUpdateVersion != "" {
-			util.WriteErrorResponse("Invalid AutoUpdateVersion", http.StatusBadRequest, w)
-			return nil
+			return nil, fmt.Errorf("invalid AutoUpdateVersion")
 		}
 	}
 
-	return returnSettings
+	return returnSettings, nil
 }
 
 // updateAccount is HTTP PUT handler that updates the provided account. Updates only account settings (server.Settings)
@@ -241,9 +241,9 @@ func (h *handler) updateAccount(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	settings := h.updateAccountRequestSettings(req, w)
-	if settings == nil {
-		return
+	settings, err := h.updateAccountRequestSettings(req)
+	if err != nil {
+		util.WriteError(r.Context(), err, w)
 	}
 	if req.Settings.NetworkRange != nil && *req.Settings.NetworkRange != "" {
 		prefix, err := netip.ParsePrefix(*req.Settings.NetworkRange)