From af8bcd3ed2bb9bb416192335ffd05bb225d73968 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 4 Dec 2023 16:15:49 +0100
Subject: [PATCH] Print useful error when switch fails outside user control

---
 internal/config/auth_config_test.go |  4 ++--
 internal/config/config.go           |  6 ++++--
 pkg/cmd/auth/switch/switch.go       |  6 +++++-
 pkg/cmd/auth/switch/switch_test.go  | 24 ++++++++++++++++++++++++
 4 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go
index eb35159a8..944bcea65 100644
--- a/internal/config/auth_config_test.go
+++ b/internal/config/auth_config_test.go
@@ -507,7 +507,7 @@ func TestSwitchUserErrorsAndRestoresUserAndInsecureConfigUnderFailure(t *testing
 	err = authCfg.SwitchUser("github.com", "test-user-1")
 
 	// Then it returns an error
-	require.EqualError(t, err, "no token found for 'test-user-1'")
+	require.EqualError(t, err, "no token found for test-user-1")
 
 	// And restores the previous state
 	activeUser, err := authCfg.User("github.com")
@@ -533,7 +533,7 @@ func TestSwitchUserErrorsAndRestoresUserAndKeyringUnderFailure(t *testing.T) {
 	err = authCfg.SwitchUser("github.com", "test-user-1")
 
 	// Then it returns an error
-	require.EqualError(t, err, "no token found for 'test-user-1'")
+	require.EqualError(t, err, "no token found for test-user-1")
 
 	// And restores the previous state
 	activeUser, err := authCfg.User("github.com")
diff --git a/internal/config/config.go b/internal/config/config.go
index 5f9b34ca8..8dadf03e8 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -369,7 +369,9 @@ func (c *AuthConfig) SwitchUser(hostname, user string) error {
 		// to its previous clean state just in case something else tries to make use of the config, or tries
 		// to write it again.
 		if previousSource == "keyring" {
-			err = errors.Join(err, keyring.Set(keyringServiceName(hostname), "", previouslyActiveToken))
+			if setErr := keyring.Set(keyringServiceName(hostname), "", previouslyActiveToken); setErr != nil {
+				err = errors.Join(err, setErr)
+			}
 		}
 
 		if previousSource == "oauth_token" {
@@ -442,7 +444,7 @@ func (c *AuthConfig) activateUser(hostname, user string) error {
 	}
 
 	if !tokenSwitched {
-		return fmt.Errorf("no token found for '%s'", user)
+		return fmt.Errorf("no token found for %s", user)
 	}
 
 	// Then we'll update the active user for the host
diff --git a/pkg/cmd/auth/switch/switch.go b/pkg/cmd/auth/switch/switch.go
index 1106b7832..c705291a6 100644
--- a/pkg/cmd/auth/switch/switch.go
+++ b/pkg/cmd/auth/switch/switch.go
@@ -163,11 +163,15 @@ func switchRun(opts *SwitchOptions) error {
 		return cmdutil.SilentError
 	}
 
+	cs := opts.IO.ColorScheme()
+
 	if err := authCfg.SwitchUser(hostname, username); err != nil {
+		fmt.Fprintf(opts.IO.ErrOut, "%s Failed to switch account for %s to %s\n",
+			cs.FailureIcon(), hostname, cs.Bold(username))
+
 		return err
 	}
 
-	cs := opts.IO.ColorScheme()
 	fmt.Fprintf(opts.IO.ErrOut, "%s Switched active account for %s to %s\n",
 		cs.SuccessIcon(), hostname, cs.Bold(username))
 
diff --git a/pkg/cmd/auth/switch/switch_test.go b/pkg/cmd/auth/switch/switch_test.go
index f16d41317..ee56c4c9d 100644
--- a/pkg/cmd/auth/switch/switch_test.go
+++ b/pkg/cmd/auth/switch/switch_test.go
@@ -3,10 +3,12 @@ package authswitch
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 	"testing"
 
 	"github.com/cli/cli/v2/internal/config"
+	"github.com/cli/cli/v2/internal/keyring"
 	"github.com/cli/cli/v2/internal/prompter"
 	"github.com/cli/cli/v2/pkg/cmdutil"
 	"github.com/cli/cli/v2/pkg/iostreams"
@@ -104,6 +106,8 @@ func TestSwitchRun(t *testing.T) {
 		stderr string
 	}
 
+	userWithMissingToken := "user-that-is-broken-by-the-test"
+
 	tests := []struct {
 		name     string
 		opts     SwitchOptions
@@ -316,6 +320,22 @@ func TestSwitchRun(t *testing.T) {
 				stderr:       "✓ Switched active account for ghe.io to inactive-user",
 			},
 		},
+		{
+			name: "when switching fails due to something other than user error, an informative message is printed to explain their new state",
+			opts: SwitchOptions{
+				Username: userWithMissingToken,
+			},
+			cfgHosts: []hostUsers{
+				{"github.com", []user{
+					{userWithMissingToken, "inactive-user-token"},
+					{"active-user", "active-user-token"},
+				}},
+			},
+			expectedFailure: failedExpectation{
+				err:    fmt.Errorf("no token found for %s", userWithMissingToken),
+				stderr: fmt.Sprintf("X Failed to switch account for github.com to %s", userWithMissingToken),
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -341,6 +361,10 @@ func TestSwitchRun(t *testing.T) {
 						user.token, "ssh", true,
 					)
 					require.NoError(t, err)
+
+					if user.name == userWithMissingToken {
+						require.NoError(t, keyring.Delete(fmt.Sprintf("gh:%s", hostUsers.host), userWithMissingToken))
+					}
 				}
 			}