From 286d86d0ad313a31e181c1bf67d41dd96d7c1595 Mon Sep 17 00:00:00 2001 From: riccardom Date: Tue, 22 Sep 2026 12:06:12 +0200 Subject: [PATCH] [client] Name only the refusals, not every FailedPrecondition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The classifier gained a blanket FailedPrecondition -> change_refused fallback so a refusal would stop reading as "Operation failed". It reaches too far: the daemon returns that code for two dozen states that are not settings refusals — "not logged in", "client is not running", "another capture is already running", "session can no longer be extended, log in again to reconnect" — and errorClassifier is shared with the session and connection services, not just the settings save. So the user was told the service had refused their change while what they actually had to do was log in again. The two refusals the daemon composes stay named by their message; everything else goes back to the generic message, which says nothing rather than something wrong. Reported by cubic on the PR. --- client/ui/i18n/locales/en/common.json | 4 ---- client/ui/services/errors.go | 12 +++++++----- client/ui/services/errors_test.go | 9 +++++---- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/client/ui/i18n/locales/en/common.json b/client/ui/i18n/locales/en/common.json index 69f17ebc7..bc948fb8b 100644 --- a/client/ui/i18n/locales/en/common.json +++ b/client/ui/i18n/locales/en/common.json @@ -1803,10 +1803,6 @@ "message": "This setting is managed by your organization and cannot be changed.", "description": "Error: the setting is enforced by an MDM policy. 'MDM' = mobile device management, the organization's device-management system." }, - "error.change_refused": { - "message": "The NetBird service refused this change.", - "description": "Error: the daemon answered the request and declined it. Generic fallback for a refusal with no more specific cause." - }, "error.unknown": { "message": "Operation failed.", "description": "Generic fallback error message used when no specific error applies." diff --git a/client/ui/services/errors.go b/client/ui/services/errors.go index 2b77548f5..d193e9f02 100644 --- a/client/ui/services/errors.go +++ b/client/ui/services/errors.go @@ -140,6 +140,13 @@ func (c errorClassifier) classify(err error) *ClientError { code = "settings_managed_by_mdm" } + // Deliberately no blanket mapping for FailedPrecondition below: the daemon + // returns it for two dozen states that are not settings refusals at all — + // "not logged in", "client is not running", "session can no longer be + // extended" — and this classifier is shared with the session and connection + // services. Only the two refusals the daemon composes are named, by their + // message. + // 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. @@ -149,11 +156,6 @@ func (c errorClassifier) classify(err error) *ClientError { code = "permission_denied" case gcodes.Unavailable, gcodes.DeadlineExceeded: code = "daemon_unreachable" - case gcodes.FailedPrecondition: - // The daemon answered and refused. The two refusals it composes - // are matched above; anything else that reaches here is still a - // refusal, so say that rather than "operation failed". - code = "change_refused" } } diff --git a/client/ui/services/errors_test.go b/client/ui/services/errors_test.go index c1f4badb9..c2a10442f 100644 --- a/client/ui/services/errors_test.go +++ b/client/ui/services/errors_test.go @@ -50,10 +50,11 @@ func TestErrorClassifier_Classify(t *testing.T) { require.Equal(t, "settings_managed_by_mdm", c.classify(err).Code) }) - t.Run("any other refusal is still a refusal", func(t *testing.T) { - // FailedPrecondition means the daemon answered and declined; falling - // through to "unknown" showed "Operation failed" instead. - require.Equal(t, "change_refused", c.classify(gstatus.Error(gcodes.FailedPrecondition, "something else")).Code) + t.Run("an unrelated FailedPrecondition is not called a refusal", func(t *testing.T) { + // The daemon uses this code for states that are not settings refusals, + // and this classifier is shared with the session and connection + // services, so only the two refusals it composes are named. + require.Equal(t, "unknown", c.classify(gstatus.Error(gcodes.FailedPrecondition, "not logged in")).Code) }) t.Run("unavailable code maps to daemon_unreachable", func(t *testing.T) {