retry connection prelude on connect errors

Author: nerdrew

lib/redis_client.rb

@@ -248,6 +248,7 @@ def initialize(config, **)
     super
     @middlewares = config.middlewares_stack.new(self)
     @raw_connection = nil
+    @prelude_sent = false
     @disable_reconnection = false
     @retry_attempt = nil
   end
@@ -745,6 +746,7 @@ def ensure_connected(retryable: true)
 
     if @disable_reconnection
       @raw_connection.retry_attempt = nil
+      send_prelude unless @prelude_sent
       if block_given?
         yield @raw_connection
       else
@@ -798,13 +800,17 @@ def ensure_connected(retryable: true)
   def raw_connection
     if @raw_connection.nil? || !@raw_connection.revalidate
       connect
+    elsif !@prelude_sent
+      send_prelude
     end
+
     @raw_connection.retry_attempt = @retry_attempt
     @raw_connection
   end
 
   def connect
     @pid = PIDCache.pid
+    @prelude_sent = false
 
     if @raw_connection&.revalidate
       @middlewares.connect(config) do
@@ -822,6 +828,24 @@ def connect
     end
     @raw_connection.retry_attempt = @retry_attempt
 
+    send_prelude
+  rescue FailoverError, CannotConnectError => error
+    error._set_config(config)
+    raise error
+  rescue ConnectionError => error
+    connect_error = CannotConnectError.with_config(error.message, config)
+    connect_error.set_backtrace(error.backtrace)
+    raise connect_error
+  rescue CommandError => error
+    if error.message.match?(/ERR unknown command ['`]HELLO['`]/)
+      raise UnsupportedServer,
+        "redis-client requires Redis 6+ with HELLO command available (#{config.server_url})"
+    else
+      raise
+    end
+  end
+
+  def send_prelude
     prelude = config.connection_prelude.dup
 
     if id
@@ -842,20 +866,8 @@ def connect
         end
       end
     end
-  rescue FailoverError, CannotConnectError => error
-    error._set_config(config)
-    raise error
-  rescue ConnectionError => error
-    connect_error = CannotConnectError.with_config(error.message, config)
-    connect_error.set_backtrace(error.backtrace)
-    raise connect_error
-  rescue CommandError => error
-    if error.message.match?(/ERR unknown command ['`]HELLO['`]/)
-      raise UnsupportedServer,
-        "redis-client requires Redis 6+ with HELLO command available (#{config.server_url})"
-    else
-      raise
-    end
+
+    @prelude_sent = true
   end
 end
 

test/shared/redis_client_tests.rb

@@ -328,7 +328,12 @@ def test_command_missing
   end
 
   def test_authentication
-    @redis.call("ACL", "SETUSER", "AzureDiamond", ">hunter2", "on", "+PING")
+    @redis.call("ACL", "DELUSER", "AzureDiamond")
+    @redis.call("ACL", "SETUSER", "AzureDiamond", ">hunter2", "on", "+PING", "+CLIENT")
+    @redis.call("ACL", "DELUSER", "backup_admin")
+    @redis.call("ACL", "SETUSER", "backup_admin", ">hunter2", "on", "~*", "&*", "+@all")
+    backup = new_client(username: "backup_admin", password: "hunter2")
+    backup.call("ACL", "SETUSER", "default", "off")
 
     client = new_client(username: "AzureDiamond", password: "hunter2")
     assert_equal "PONG", client.call("PING")
@@ -337,10 +342,70 @@ def test_authentication
       client.call("GET", "foo")
     end
 
+    # Wrong password
     client = new_client(username: "AzureDiamond", password: "trolilol")
-    assert_raises RedisClient::AuthenticationError do
+    error = assert_raises RedisClient::AuthenticationError do
       client.call("PING")
     end
+    assert_match(/WRONGPASS invalid username-password pair/, error.message)
+
+    # The same error is raised, this shows that the client retried AUTH and didn't fall back to the default user
+    error = assert_raises RedisClient::AuthenticationError do
+      client.call("PING")
+    end
+    assert_match(/WRONGPASS invalid username-password pair/, error.message)
+
+    # Correct password, but user disabled
+    backup.call("ACL", "SETUSER", "AzureDiamond", "<hunter2", ">trolilol", "off")
+    error = assert_raises RedisClient::AuthenticationError do
+      client.call_once("PING")
+    end
+    assert_match(/WRONGPASS invalid username-password pair/, error.message)
+
+    # Correct password, user enabled
+    backup.call("ACL", "SETUSER", "AzureDiamond", "on")
+    assert_equal "PONG", client.call_once("PING")
+    assert_match(/user=AzureDiamond/, client.call("CLIENT", "INFO"))
+
+    # Wrong username
+    client = new_client(username: "GreenOpal", password: "trolilol")
+    error = assert_raises RedisClient::AuthenticationError do
+      client.call("PING")
+    end
+    assert_match(/WRONGPASS invalid username-password pair/, error.message)
+  ensure
+    backup.call("ACL", "SETUSER", "default", "on")
+  end
+
+  def test_prelude_failure
+    client = new_client(db: 100)
+    error = assert_raises RedisClient::CommandError do
+      client.call("PING")
+    end
+    assert_match(/ERR DB index is out of range/, error.message)
+
+    error = assert_raises RedisClient::CommandError do
+      client.call("PING")
+    end
+    assert_match(/ERR DB index is out of range/, error.message)
+  end
+
+  def test_noauth
+    @redis.call("ACL", "DELUSER", "AzureDiamond")
+    @redis.call("ACL", "SETUSER", "AzureDiamond", ">hunter2", "on", "~*", "&*", "+@all")
+    backup = new_client(username: "AzureDiamond", password: "hunter2")
+    backup.call("ACL", "SETUSER", "default", "off")
+
+    client = new_client(protocol: 2)
+    error = assert_raises RedisClient::CommandError do
+      client.call("PING")
+    end
+    assert_match(/NOAUTH Authentication required/, error.message)
+
+    backup.call("ACL", "SETUSER", "default", "on")
+    client.call("PING")
+  ensure
+    backup.call("ACL", "SETUSER", "default", "on")
   end
 
   def test_transaction