stefanwille / crystal-redis

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

Casting Int64 into Array(RedisValue) #80

Closed amedeiros closed 5 years ago

amedeiros commented 5 years ago

When using commands zrangebyscore and zremrangebyscore its possible for the command to return an Int64 from redis. in Redis::CommandExecution::ValueOriented#string_array_command and Redis::CommandExecution::ValueOriented#array_or_nil_command.

Yes I am in a multithreaded environment and yes I am using the PooledClient (saw the other ticket).

Making these changes stopped the exceptions. I can make a pull request if this is the appropriate fix thanks.

def array_or_nil_command(request : Request) : Array(RedisValue)?
  response = command(request)

  if typeof(response) == Int64
    return Array(RedisValue).new
  end

  response.as(Array(RedisValue)?)
end
def string_array_command(request : Request) : Array(RedisValue)
  response = command(request)
  if typeof(response) == Int64
    return Array(RedisValue).new
  end

  response.as(Array(RedisValue))
end
amedeiros commented 5 years ago

Also side note this could be completely because of the multi threading in my app.

kostya commented 5 years ago

this quite hack, i think this should not be the case. Should be some wrong usage, your env really threaded or just concurrent? what pool size?

amedeiros commented 5 years ago

A hack indeed. Well currently I spawn a few different fibers each one using the the same PooledClient instance with the default size of 5.

amedeiros commented 5 years ago

Closing this was my fault.

kostya commented 5 years ago

Actually, when you use same PooledClient between fibers, it should not raise this error even if pool small (can only be problem if you use threads). When connection busy, it wait or raise PoolTimeoutError error. So, i only can imagine some crazy combination of multi or pipeline with other commands in concurrent env.