Improve error log message output

Previously when an error were received by a Redis server, it was not
clear whether this came from a Redis Sentinel or a Redis server in the
cluster. This commit adds the hostname/port of the server to the
exception message via a log context.

Note that this commit also fixes an existing bug where an error in a
MULTI/EXEC command would not properly be processed.

Closes redis-rb#177

Author: stanhu

hiredis-client/lib/redis_client/hiredis_connection.rb

@@ -119,6 +119,10 @@ def write_multi(commands)
 
     private
 
+    def host_context
+      @config.path || "#{@config.host}:#{@config.port}"
+    end
+
     def connect
       _connect(@config.path, @config.host, @config.port, @config.ssl_context)
     rescue SystemCallError => error

lib/redis_client.rb

@@ -79,10 +79,27 @@ def timeout=(timeout)
 
   Error = Class.new(StandardError)
 
-  ProtocolError = Class.new(Error)
-  UnsupportedServer = Class.new(Error)
+  class ErrorWithContext < Error
+    attr_reader :context
+    alias_method :original_to_s, :to_s
+
+    def _set_context(context)
+      @context = context
+    end
+
+    def to_s
+      if context
+        "#{original_to_s} (#{context})"
+      else
+        original_to_s
+      end
+    end
+  end
+
+  ProtocolError = Class.new(ErrorWithContext)
+  UnsupportedServer = Class.new(ErrorWithContext)
+  ConnectionError = Class.new(ErrorWithContext)
 
-  ConnectionError = Class.new(Error)
   CannotConnectError = Class.new(ConnectionError)
 
   FailoverError = Class.new(ConnectionError)
@@ -100,7 +117,7 @@ def _set_command(command)
     end
   end
 
-  class CommandError < Error
+  class CommandError < ErrorWithContext
     include HasCommand
 
     class << self
@@ -114,7 +131,7 @@ def parse(error_message)
         end
         code ||= error_message.split(' ', 2).first
         klass = ERRORS.fetch(code, self)
-        klass.new(error_message)
+        klass.new(error_message&.strip)
       end
     end
   end

lib/redis_client/connection_mixin.rb

@@ -32,6 +32,7 @@ def call(command, timeout)
       @pending_reads -= 1
       if result.is_a?(Error)
         result._set_command(command)
+        result._set_context(host_context) if result.is_a?(ErrorWithContext)
         raise result
       else
         result
@@ -50,10 +51,17 @@ def call_pipelined(commands, timeouts)
         timeout = timeouts && timeouts[index]
         result = read(timeout)
         @pending_reads -= 1
-        if result.is_a?(Error)
-          result._set_command(commands[index])
-          exception ||= result
+
+        # A multi/exec command can return an array of results
+        result_arr = Array(result)
+        result_arr.each do |res|
+          next unless res.is_a?(Error)
+
+          res._set_command(commands[index])
+          res._set_context(host_context) if res.is_a?(ErrorWithContext)
+          exception ||= res
         end
+
         results[index] = result
       end
 

lib/redis_client/ruby_connection.rb

@@ -109,6 +109,10 @@ def measure_round_trip_delay
 
     private
 
+    def host_context
+      @config.path || "#{@config.host}:#{@config.port}"
+    end
+
     def connect
       socket = if @config.path
         UNIXSocket.new(@config.path)

test/redis_client_test.rb

@@ -64,6 +64,7 @@ def call_pipelined(commands, *)
       client.call("PING")
     end
     assert_includes error.message, "redis-client requires Redis 6+ with HELLO command available"
+    assert_includes error.message, "(redis://"
   end
 
   def test_redis_6_server_with_missing_hello_command
@@ -82,6 +83,7 @@ def call_pipelined(commands, *)
       client.call("PING")
     end
     assert_includes error.message, "redis-client requires Redis 6+ with HELLO command available"
+    assert_includes error.message, "(redis://"
   end
 
   def test_handle_async_raise

test/shared/redis_client_tests.rb

@@ -271,28 +271,29 @@ def test_wrong_type
       @redis.call("SISMEMBER", "str", "member")
     end
     assert_equal ["SISMEMBER", "str", "member"], error.command
-    assert_includes error.message, "WRONGTYPE Operation against a key holding the wrong kind of value"
+    assert_match(/WRONGTYPE Operation against a key holding the wrong kind of value (.*:.*)/, error.message)
 
     error = assert_raises RedisClient::CommandError do
       @redis.pipelined do |pipeline|
         pipeline.call("SISMEMBER", "str", "member")
       end
     end
-    assert_includes error.message, "WRONGTYPE Operation against a key holding the wrong kind of value"
+    assert_match(/WRONGTYPE Operation against a key holding the wrong kind of value (.*:.*)/, error.message)
 
     error = assert_raises RedisClient::CommandError do
       @redis.multi do |transaction|
         transaction.call("SISMEMBER", "str", "member")
       end
     end
-    assert_includes error.message, "WRONGTYPE Operation against a key holding the wrong kind of value"
+
+    assert_match(/WRONGTYPE Operation against a key holding the wrong kind of value (.*:.*)/, error.message)
   end
 
   def test_command_missing
     error = assert_raises RedisClient::CommandError do
       @redis.call("DOESNOTEXIST", "foo")
     end
-    assert_match(/ERR unknown command .DOESNOTEXIST./, error.message)
+    assert_match(/ERR unknown command .DOESNOTEXIST.*\(.*:.*\)/, error.message)
   end
 
   def test_authentication