From f6e50995b68ff1596de314f4a255c5c7ae7c6540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Thu, 18 Jun 2026 13:37:58 +0200 Subject: [PATCH] [client] Classify session extend errors for the UI Route services.Session errors through the same classifier Connection uses so RequestExtend/WaitExtend return a structured ClientError with a clean localized short message instead of the raw daemon error. Extract the shared errorClassifier into errors.go, and fall back to the gRPC status code when no message substring matches, since the daemon now forwards a clean desc without the English marker text. --- client/ui/main.go | 2 +- client/ui/services/connection.go | 106 +----------------------- client/ui/services/errors.go | 133 ++++++++++++++++++++++++++++++ client/ui/services/errors_test.go | 50 +++++++++++ client/ui/services/session.go | 20 +++-- 5 files changed, 203 insertions(+), 108 deletions(-) create mode 100644 client/ui/services/errors.go create mode 100644 client/ui/services/errors_test.go diff --git a/client/ui/main.go b/client/ui/main.go index b56a0c549..e6b77762c 100644 --- a/client/ui/main.go +++ b/client/ui/main.go @@ -314,7 +314,7 @@ func buildI18n(app *application.App) (*i18n.Bundle, *preferences.Store, *Localiz // already built so the tray and feed loops share the same instances. func registerServices(app *application.App, conn *Conn, s registeredServices) { app.RegisterService(application.NewService(s.connection)) - app.RegisterService(application.NewService(services.NewSession(s.authSession))) + app.RegisterService(application.NewService(services.NewSession(s.authSession, s.bundle, s.prefStore))) app.RegisterService(application.NewService(s.settings)) app.RegisterService(application.NewService(s.networks)) app.RegisterService(application.NewService(services.NewForwarding(conn))) diff --git a/client/ui/services/connection.go b/client/ui/services/connection.go index ace4c1847..a23a526e6 100644 --- a/client/ui/services/connection.go +++ b/client/ui/services/connection.go @@ -4,60 +4,18 @@ package services import ( "context" - "encoding/json" "fmt" "os" "os/exec" "os/user" "runtime" - "strings" log "github.com/sirupsen/logrus" - gstatus "google.golang.org/grpc/status" "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/proto" - "github.com/netbirdio/netbird/client/ui/i18n" - "github.com/netbirdio/netbird/client/ui/preferences" ) -// ErrorTranslator localises daemon errors; runtime impl is *i18n.Bundle. -type ErrorTranslator interface { - Translate(lang i18n.LanguageCode, key string, args ...string) string -} - -// LanguagePreference reports the current UI language; runtime impl is *preferences.Store. -type LanguagePreference interface { - Get() preferences.UIPreferences -} - -// ClientError is a structured error returned to the frontend. The frontend -// translates Code via i18n; Short is an English fallback; Long carries the -// unwrapped daemon message. -type ClientError struct { - Code string `json:"code"` - Short string `json:"short"` - Long string `json:"long"` -} - -// Error returns the short message for plain Go callers. -func (e *ClientError) Error() string { - if e == nil { - return "" - } - return e.Short -} - -// MarshalJSON emits the struct so the Wails binding sends an object, not the -// default "error: ..." string. -func (e *ClientError) MarshalJSON() ([]byte, error) { - if e == nil { - return []byte("null"), nil - } - type alias ClientError - return json.Marshal((*alias)(e)) -} - // LoginParams are the inputs to Login. type LoginParams struct { ProfileName string `json:"profileName"` @@ -98,14 +56,13 @@ type LogoutParams struct { // Connection groups the daemon RPCs that drive login / connect / disconnect. type Connection struct { conn DaemonConn - translator ErrorTranslator - prefs LanguagePreference + classifier errorClassifier } // NewConnection wires up a Connection. translator or prefs may be nil, in which // case classifyDaemonError falls back to the bare error key. func NewConnection(conn DaemonConn, translator ErrorTranslator, prefs LanguagePreference) *Connection { - return &Connection{conn: conn, translator: translator, prefs: prefs} + return &Connection{conn: conn, classifier: errorClassifier{translator: translator, prefs: prefs}} } func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, error) { @@ -260,62 +217,7 @@ func (s *Connection) Logout(ctx context.Context, p LogoutParams) error { return nil } -// classifyDaemonError maps a gRPC error to a ClientError by matching known -// substrings to a stable code. A missing locale entry surfaces as a visible -// "error." string — a deliberate fail-loud signal to update the bundle. +// classifyDaemonError maps a gRPC error to a localised ClientError. func (s *Connection) classifyDaemonError(err error) *ClientError { - if err == nil { - return nil - } - - msg := err.Error() - if st, ok := gstatus.FromError(err); ok { - msg = st.Message() - } - lower := strings.ToLower(msg) - - code := "unknown" - switch { - case strings.Contains(lower, "token used before issued"), - strings.Contains(lower, "token is not valid yet"): - code = "jwt_clock_skew" - case strings.Contains(lower, "token is expired"), - strings.Contains(lower, "token has expired"): - code = "jwt_expired" - case strings.Contains(lower, "token signature is invalid"): - code = "jwt_signature_invalid" - case strings.Contains(lower, "peer login has expired"): - code = "session_expired" - case strings.Contains(lower, "invalid setup-key"), - strings.Contains(lower, "invalid setup key"): - code = "invalid_setup_key" - case strings.Contains(lower, "permission denied"): - code = "permission_denied" - case strings.Contains(lower, "no connection could be made"), - strings.Contains(lower, "connection refused"), - strings.Contains(lower, "context deadline exceeded"): - code = "daemon_unreachable" - } - - return &ClientError{ - Code: code, - Short: s.translateShort(code), - Long: msg, - } -} - -// translateShort resolves the localised short message for code, returning the -// bare "error." key when no translation is available so the gap stays visible. -func (s *Connection) translateShort(code string) string { - key := "error." + code - if s.translator == nil { - return key - } - lang := i18n.DefaultLanguage - if s.prefs != nil { - if pref := s.prefs.Get().Language; pref != "" { - lang = pref - } - } - return s.translator.Translate(lang, key) + return s.classifier.classify(err) } diff --git a/client/ui/services/errors.go b/client/ui/services/errors.go new file mode 100644 index 000000000..f1679e764 --- /dev/null +++ b/client/ui/services/errors.go @@ -0,0 +1,133 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "encoding/json" + "strings" + + gcodes "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/ui/i18n" + "github.com/netbirdio/netbird/client/ui/preferences" +) + +// ErrorTranslator localises daemon errors; runtime impl is *i18n.Bundle. +type ErrorTranslator interface { + Translate(lang i18n.LanguageCode, key string, args ...string) string +} + +// LanguagePreference reports the current UI language; runtime impl is *preferences.Store. +type LanguagePreference interface { + Get() preferences.UIPreferences +} + +// ClientError is a structured error returned to the frontend. The frontend +// translates Code via i18n; Short is an English fallback; Long carries the +// unwrapped daemon message. +type ClientError struct { + Code string `json:"code"` + Short string `json:"short"` + Long string `json:"long"` +} + +// Error returns the short message for plain Go callers. +func (e *ClientError) Error() string { + if e == nil { + return "" + } + return e.Short +} + +// MarshalJSON emits the struct so the Wails binding sends an object, not the +// default "error: ..." string. +func (e *ClientError) MarshalJSON() ([]byte, error) { + if e == nil { + return []byte("null"), nil + } + type alias ClientError + return json.Marshal((*alias)(e)) +} + +// errorClassifier maps gRPC errors to a localised ClientError. Shared by the +// daemon-facing services so the frontend gets a clean short message instead of +// the wrapped gRPC chain. +type errorClassifier struct { + translator ErrorTranslator + prefs LanguagePreference +} + +// classify maps a gRPC error to a ClientError by matching known substrings to a +// stable code. A missing locale entry surfaces as a visible "error." +// string — a deliberate fail-loud signal to update the bundle. +func (c errorClassifier) classify(err error) *ClientError { + if err == nil { + return nil + } + + msg := err.Error() + grpcCode := gcodes.Unknown + if st, ok := gstatus.FromError(err); ok { + msg = st.Message() + grpcCode = st.Code() + } + lower := strings.ToLower(msg) + + code := "unknown" + switch { + case strings.Contains(lower, "token used before issued"), + strings.Contains(lower, "token is not valid yet"): + code = "jwt_clock_skew" + case strings.Contains(lower, "token is expired"), + strings.Contains(lower, "token has expired"): + code = "jwt_expired" + case strings.Contains(lower, "token signature is invalid"): + code = "jwt_signature_invalid" + case strings.Contains(lower, "peer login has expired"): + code = "session_expired" + case strings.Contains(lower, "invalid setup-key"), + strings.Contains(lower, "invalid setup key"): + code = "invalid_setup_key" + case strings.Contains(lower, "permission denied"): + code = "permission_denied" + case strings.Contains(lower, "no connection could be made"), + strings.Contains(lower, "connection refused"), + strings.Contains(lower, "context deadline exceeded"): + code = "daemon_unreachable" + } + + // Fall back to the gRPC status code when the message didn't match a known + // substring — the daemon now forwards the innermost code with a clean desc + // that no longer contains the English marker text. + if code == "unknown" { + switch grpcCode { + case gcodes.PermissionDenied: + code = "permission_denied" + case gcodes.Unavailable, gcodes.DeadlineExceeded: + code = "daemon_unreachable" + } + } + + return &ClientError{ + Code: code, + Short: c.translateShort(code), + Long: msg, + } +} + +// translateShort resolves the localised short message for code, returning the +// bare "error." key when no translation is available so the gap stays visible. +func (c errorClassifier) translateShort(code string) string { + key := "error." + code + if c.translator == nil { + return key + } + lang := i18n.DefaultLanguage + if c.prefs != nil { + if pref := c.prefs.Get().Language; pref != "" { + lang = pref + } + } + return c.translator.Translate(lang, key) +} diff --git a/client/ui/services/errors_test.go b/client/ui/services/errors_test.go new file mode 100644 index 000000000..2f8f3d039 --- /dev/null +++ b/client/ui/services/errors_test.go @@ -0,0 +1,50 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + gcodes "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" +) + +func TestErrorClassifier_Classify(t *testing.T) { + c := errorClassifier{} // nil translator → Short is the bare "error." key + + t.Run("permission denied by gRPC code with a clean desc", func(t *testing.T) { + // The daemon now forwards the innermost status: code + clean desc that + // no longer carries the English "permission denied" marker. + err := gstatus.Error(gcodes.PermissionDenied, "peer is already registered by a different User or a Setup Key") + + ce := c.classify(err) + require.NotNil(t, ce) + require.Equal(t, "permission_denied", ce.Code) + require.Equal(t, "error.permission_denied", ce.Short) + require.Equal(t, "peer is already registered by a different User or a Setup Key", ce.Long) + }) + + t.Run("substring match still wins for unclassified codes", func(t *testing.T) { + err := gstatus.Error(gcodes.Unknown, "peer login has expired") + + ce := c.classify(err) + require.NotNil(t, ce) + require.Equal(t, "session_expired", ce.Code) + }) + + t.Run("unavailable code maps to daemon_unreachable", func(t *testing.T) { + ce := c.classify(gstatus.Error(gcodes.Unavailable, "transport closing")) + require.Equal(t, "daemon_unreachable", ce.Code) + }) + + t.Run("unmatched stays unknown", func(t *testing.T) { + ce := c.classify(errors.New("something odd")) + require.Equal(t, "unknown", ce.Code) + }) + + t.Run("nil error", func(t *testing.T) { + require.Nil(t, c.classify(nil)) + }) +} diff --git a/client/ui/services/session.go b/client/ui/services/session.go index 5ee56788e..facb8e898 100644 --- a/client/ui/services/session.go +++ b/client/ui/services/session.go @@ -19,20 +19,30 @@ type ( // Session wraps authsession.Session, exposing only the subset the React frontend // calls; the tray uses authsession.Session directly, keeping the generated TS surface minimal. type Session struct { - inner *authsession.Session + inner *authsession.Session + classifier errorClassifier } // NewSession wraps inner; the caller retains ownership and may use it directly. -func NewSession(inner *authsession.Session) *Session { - return &Session{inner: inner} +// translator or prefs may be nil, in which case errors fall back to the bare code key. +func NewSession(inner *authsession.Session, translator ErrorTranslator, prefs LanguagePreference) *Session { + return &Session{inner: inner, classifier: errorClassifier{translator: translator, prefs: prefs}} } // RequestExtend starts the SSO session-extension flow; the result carries the verification URI to open. func (s *Session) RequestExtend(ctx context.Context, p ExtendStartParams) (ExtendStartResult, error) { - return s.inner.RequestExtend(ctx, p) + res, err := s.inner.RequestExtend(ctx, p) + if err != nil { + return ExtendStartResult{}, s.classifier.classify(err) + } + return res, nil } // WaitExtend blocks until the RequestExtend flow completes; the deadline is nil when the peer is ineligible. func (s *Session) WaitExtend(ctx context.Context, p ExtendWaitParams) (ExtendResult, error) { - return s.inner.WaitExtend(ctx, p) + res, err := s.inner.WaitExtend(ctx, p) + if err != nil { + return ExtendResult{}, s.classifier.classify(err) + } + return res, nil }