From f943926fca798a10a9ba521ab5fb59574c4bcf7d Mon Sep 17 00:00:00 2001
From: hkfires <10558748+hkfires@users.noreply.github.com>
Date: Mon, 27 Jul 2026 22:37:27 +0800
Subject: [PATCH] feat(home): improve error handling in cluster discovery and
 enhance GetConfig tests

---
 internal/home/client.go      | 26 +++++++++---
 internal/home/client_test.go | 76 ++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 6 deletions(-)

diff --git a/internal/home/client.go b/internal/home/client.go
index 8f0dcc28..fde6ed8f 100644
--- a/internal/home/client.go
+++ b/internal/home/client.go
@@ -82,6 +82,8 @@ func IsAmbiguousDispatchError(err error) bool {
 	return errors.As(err, &dispatchErr) && dispatchErr.Ambiguous
 }
 
+var errClusterDiscoveryTransport = errors.New("home cluster discovery transport failed")
+
 var (
 	ErrDisabled              = errors.New("home client disabled")
 	ErrNotConnected          = errors.New("home not connected")
@@ -658,20 +660,21 @@ func (c *Client) clusterDiscoveryEnabledLocked() bool {
 	return !c.homeCfg.DisableClusterDiscovery
 }
 
-func (c *Client) refreshBestClusterNode(ctx context.Context) {
+func (c *Client) refreshBestClusterNode(ctx context.Context) error {
 	if !c.clusterDiscoveryEnabled() {
-		return
+		return nil
 	}
 	switched, errRefresh := c.refreshClusterNodes(ctx)
 	if errRefresh != nil {
 		log.Debugf("home cluster nodes unavailable: %v", errRefresh)
-		return
+		return errRefresh
 	}
 	if switched {
 		if addr, ok := c.addr(); ok {
 			log.Infof("home cluster target switched to %s", addr)
 		}
 	}
+	return nil
 }
 
 func (c *Client) refreshClusterNodes(ctx context.Context) (bool, error) {
@@ -683,12 +686,21 @@ func (c *Client) refreshClusterNodes(ctx context.Context) (bool, error) {
 	}
 	cmd, errClient := c.commandClient()
 	if errClient != nil {
-		return false, errClient
+		return false, fmt.Errorf("%w: %w", errClusterDiscoveryTransport, errClient)
 	}
-	raw, errDo := cmd.Do(ctx, "CLUSTER", "NODES").Text()
+	nodesCommand := cmd.Do(ctx, "CLUSTER", "NODES")
+	errDo := nodesCommand.Err()
 	if errDo != nil {
+		var redisErr redis.Error
+		if !errors.As(errDo, &redisErr) {
+			return false, fmt.Errorf("%w: %w", errClusterDiscoveryTransport, errDo)
+		}
 		return false, errDo
 	}
+	raw, errText := nodesCommand.Text()
+	if errText != nil {
+		return false, errText
+	}
 
 	nodes, errParse := parseClusterNodesPayload([]byte(raw))
 	if errParse != nil {
@@ -844,7 +856,9 @@ func (c *Client) resetReconnectFailures() {
 }
 
 func (c *Client) GetConfig(ctx context.Context) ([]byte, error) {
-	c.refreshBestClusterNode(ctx)
+	if errRefresh := c.refreshBestClusterNode(ctx); errors.Is(errRefresh, errClusterDiscoveryTransport) {
+		return nil, errRefresh
+	}
 	cmd, errClient := c.commandClient()
 	if errClient != nil {
 		return nil, errClient
diff --git a/internal/home/client_test.go b/internal/home/client_test.go
index 173bb533..4cfd9251 100644
--- a/internal/home/client_test.go
+++ b/internal/home/client_test.go
@@ -153,6 +153,82 @@ func TestRefreshClusterNodesDisabledSkipsRedisCommand(t *testing.T) {
 	}
 }
 
+func TestGetConfigSkipsSecondDialAfterClusterTransportFailure(t *testing.T) {
+	client := New(config.HomeConfig{Enabled: true, Host: "127.0.0.1", Port: 1})
+	var dialMu sync.Mutex
+	dialAttempts := 0
+	options := &redis.Options{
+		Addr:                  "127.0.0.1:1",
+		DialTimeout:           time.Second,
+		MaxRetries:            -1,
+		DialerRetries:         1,
+		ContextTimeoutEnabled: true,
+		Dialer: func(context.Context, string, string) (net.Conn, error) {
+			dialMu.Lock()
+			dialAttempts++
+			dialMu.Unlock()
+			return nil, errors.New("test Home unavailable")
+		},
+	}
+	client.cmdOptions = cloneRedisOptions(options)
+	client.cmd = redis.NewClient(options)
+	t.Cleanup(client.Close)
+
+	_, errGet := client.GetConfig(context.Background())
+	if !errors.Is(errGet, errClusterDiscoveryTransport) {
+		t.Fatalf("GetConfig() error = %v, want cluster discovery transport error", errGet)
+	}
+	dialMu.Lock()
+	attempts := dialAttempts
+	dialMu.Unlock()
+	if attempts != 1 {
+		t.Fatalf("GetConfig() dial attempts = %d, want 1", attempts)
+	}
+}
+
+func TestGetConfigContinuesAfterClusterDiscoveryResponseError(t *testing.T) {
+	tests := []struct {
+		name     string
+		response string
+	}{
+		{name: "protocol error", response: "-ERR cluster command unsupported\r\n"},
+		{name: "response type error", response: ":1\r\n"},
+	}
+
+	for _, testCase := range tests {
+		t.Run(testCase.name, func(t *testing.T) {
+			client, commands := newRedisCommandTestClient(t, func(args []string) string {
+				switch {
+				case len(args) >= 2 && strings.EqualFold(args[0], "CLUSTER") && strings.EqualFold(args[1], "NODES"):
+					return testCase.response
+				case len(args) >= 2 && strings.EqualFold(args[0], "GET") && args[1] == redisKeyConfig:
+					payload := "host: 127.0.0.1\n"
+					return fmt.Sprintf("$%d\r\n%s\r\n", len(payload), payload)
+				default:
+					return "-ERR unexpected command\r\n"
+				}
+			})
+			client.mu.Lock()
+			client.homeCfg.DisableClusterDiscovery = false
+			client.mu.Unlock()
+
+			raw, errGet := client.GetConfig(context.Background())
+			if errGet != nil {
+				t.Fatalf("GetConfig() error = %v", errGet)
+			}
+			if string(raw) != "host: 127.0.0.1\n" {
+				t.Fatalf("GetConfig() = %q", raw)
+			}
+			if count := commands.CountCommandKey("CLUSTER", "NODES"); count != 1 {
+				t.Fatalf("CLUSTER NODES count = %d, want 1", count)
+			}
+			if count := commands.CountCommandKey("GET", redisKeyConfig); count != 1 {
+				t.Fatalf("GET config count = %d, want 1", count)
+			}
+		})
+	}
+}
+
 func TestFailoverAfterReconnectFailureDisabledDoesNotSwitchToClusterNode(t *testing.T) {
 	client := New(config.HomeConfig{
 		Enabled:                 true,
-- 
2.54.0