Merge pull request #1026 from praboud-stripe/cluster-command

Ensure command details are populated for Cluster clients
redis · Sep 14, 2021 · 5c4f2e8 · 5c4f2e8
2 parents c97360b + ea24b5b
commit 5c4f2e8
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 9 deletions.
diff --git a/lib/redis/cluster/command_loader.rb b/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

diff --git a/test/cluster_client_key_hash_tags_test.rb b/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