stefanwille / crystal-redis

Full featured Redis client for Crystal
MIT License
380 stars 61 forks source link

Throw Redis::DisconnectedError rather than IO::Error #55

Closed d-m-cc closed 6 years ago

d-m-cc commented 6 years ago

Hello, I'm getting used to Crystal, and have been using both crystal-redis and redisoid to connect to a Redis server. I'm attempting to build a web server (Lucky framework) with an endpoint that will return the results of a Redis key. I've been testing edge cases, and it seems that when the Redis server is shut down, then a request is made, and then it is restarted, subsequent Redis requests throw IO::Error rather than Redis::DisconnectedError. Is this intended? If not, would it be possible to change this so that it throws the DisconnectedError? This would make redisoid automatically connect and probably make it easier to detect if the cause of the error is Redis or not. Thanks!

kostya commented 6 years ago

i assume this will fix it, but need to test it (and may be not good to silent all this errors):

diff --git a/src/redis/connection.cr b/src/redis/connection.cr
index dc425f3..38a81df 100644
--- a/src/redis/connection.cr
+++ b/src/redis/connection.cr
@@ -117,6 +117,12 @@ class Redis::Connection
     else
       raise Redis::Error.new("Cannot parse response with type #{type}: #{line.inspect}")
     end
+  rescue IO::Error
+    raise Redis::DisconnectedError.new
+  rescue Errno
+    raise Redis::DisconnectedError.new
+  rescue Socket::Error
+    raise Redis::DisconnectedError.new
+  rescue IO::Timeout
+    raise Redis::DisconnectedError.new
   end

   def receive_line
kostya commented 6 years ago

how to reproduce it?

redis-server --port 7777 --timeout 20

require "./src/redis"

r = nil

loop do
  begin
    r ||= Redis.new(host: "localhost", port: 7777)
    r.set "bla", "a"
    p r.get("bla")
  rescue ex : Redis::DisconnectedError
    p ex

    # reconnect
    begin
          r = Redis.new(host: "localhost", port: 7777)
        rescue ex
          p "reconnect #{ex.class} #{ex.inspect}"
          r = nil
        end
  rescue ex
    p ex
  end
  sleep 1.0
end

and start stop redis in the middle:

"a"
"a"
#<Redis::DisconnectedError:RedisError: Disconnected>
"reconnect Errno #<Errno:Error connecting to 'localhost:7777': Connection refused>"
"a"
"a"
"a"

no any IO::Error, what is your OS, redis version?

stefanwille commented 6 years ago

@dwmcc same here, I tried @kostya's program, and it shows me a DisconnectedError and allows me to reconnect.