Ensure command details are populated for Cluster clients

Previously, if a clustered client is unable to query any of the nodes
for command details during setup, it'd initialize successfully, but
without any command details. (This is a bit of a corner case - in order
to hit this case, at least one node needs to be reachable when querying
nodes + slots, but all nodes would need to be unreachable immediately
afterwards when querying command details.)
In this case, the client would be unable to determine which node a
given command should be routed to. Because the client falls back to
routing commands to a random node, and following redirects as
necessary, this wouldn't cause any overt problems, but leaves the client
following unnecessary redirects for the rest of the lifespan of the
client.

This patch changes the client to require that the initial fetch of
command details succeeds (as is already done with slot and node detail
fetching), and raises a CannotConnectError if that's not possible.
Users should already handle this error, and retry creating the client
later, or use some other suitable failure handling mechanism.

Author: praboud-stripe

lib/redis/cluster/command_loader.rb

@@ -10,22 +10,21 @@ module CommandLoader
       module_function
 
       def load(nodes)
-        details = {}
-
         nodes.each do |node|
-          details = fetch_command_details(node)
-          details.empty? ? next : break
+          begin
+            return fetch_command_details(node)
+          rescue CannotConnectError, ConnectionError, CommandError
+            next # can retry on another node
+          end
         end
 
-        details
+        raise CannotConnectError, 'Redis client could not connect to any cluster nodes'
       end
 
       def fetch_command_details(node)
         node.call(%i[command]).map do |reply|
           [reply[0], { arity: reply[1], flags: reply[2], first: reply[3], last: reply[4], step: reply[5] }]
         end.to_h
-      rescue CannotConnectError, ConnectionError, CommandError
-        {} # can retry on another node
       end
 
       private_class_method :fetch_command_details

test/cluster_client_key_hash_tags_test.rb

@@ -6,8 +6,8 @@
 class TestClusterClientKeyHashTags < Minitest::Test
   include Helper::Cluster
 
-  def build_described_class
-    option = Redis::Cluster::Option.new(cluster: ['redis://127.0.0.1:7000'])
+  def build_described_class(urls = ['redis://127.0.0.1:7000'])
+    option = Redis::Cluster::Option.new(cluster: urls)
     node = Redis::Cluster::Node.new(option.per_node_key)
     details = Redis::Cluster::CommandLoader.load(node)
     Redis::Cluster::Command.new(details)
@@ -85,4 +85,15 @@ def test_whether_the_command_effect_is_readonly_or_not
       assert_equal false, described_class.should_send_to_slave?([:info])
     end
   end
+
+  def test_cannot_build_details_from_bad_urls
+    assert_raises(Redis::CannotConnectError) do
+      build_described_class(['redis://127.0.0.1:7006'])
+    end
+  end
+
+  def test_builds_details_from_a_mix_of_good_and_bad_urls
+    described_class = build_described_class(['redis://127.0.0.1:7006', 'redis://127.0.0.1:7000'])
+    assert_equal 'dogs:1', described_class.extract_first_key(%w[get dogs:1])
+  end
 end