Redis lookup with a default value

Question

I have some code to perform a redis lookup, but if the key to look up either doesn't exist or there's an issue connecting to Redis at all, it will use a default value.

The logic currently looks like this:

begin
 redis = Redis.new(:url => url)
 returned_value = redis.get(key)
 if returned_value == nil and defined?(default) != nil
 default
 else
 returned_value
 end
rescue Exception => e
 if default
 debug "Connection to redis failed with #{e} - Returning default value of #{default}"
 default
 else
 raise(Puppet::Error, "connection to redis server failed - #{e}")
 end
end

It feels like there might be a simpler way of doing this, right now this code feels a little wordy.

Any suggestions?

Answer 1

First of all NEVER rescue ExceptionError. If you want a to rescue from any runtime exception, just omit exception class, otherwise you might catch SignalException which happens upon ruby process received some signal (e.g. SIGTERM) and instead of behave correctly you will silently skip that.

So, your rescue part should look actually like this:

begin
 # ...
rescue SocketError, Redis::CannotConnectError => e
 raise Puppet::Error, "connection to redis server failed - #{e}" unless default
 debug "Connection to redis failed with #{e} - Returning default value of #{default}"
 default
end

Now your main block can be also improved:

begin
 Redis.new(:url => url).get(key) || default
rescue
 # ...
end

Notice that redis returns either String or Nil, e.g.:

Redis.current.set(:x, 123)
Redis.current.get(:x) # => "123"
Redis.current.set(:x, false)
Redis.current.get(:x) # => "false"

So, summing all above together, you can form that into a method:

def redis_get(key, url:, default: nil)
 Redis.new(:url => url).get(key) || default
rescue SocketError, Redis::CannotConnectError => e
 raise Puppet::Error, "connection to redis server failed - #{e}" unless default
 debug "Connection to redis failed with #{e}; Return default: #{default}"
 default
end

• Peter Souter: Fantastic, that looks a lot cleaner. However my spec failed with a test where the redis server connection could not be found, so I added SocketError to the rescue then everything was green. I added that as an edit to the answer, other wise thanks for your fix, I'll keep this in mind next time :)
• Aleksey V Zapparov: Yeah, it's totally fine to add SocketError as well. :D
• Aleksey V Zapparov: Sorry, I saw that I can simply approve too late. Anyway. One note to your edit - there were no need in begin block. You can (and in this case should) rescue method's body... No need for extra level of depth. And for the sake of history, one might want to rescue SocketError as it's got raised when socket can't get ip address. e.g. when you specify :url => "redis://surprise" and surprise can't be resolved. So it's not actually redis connection issue, it' general networking issue.
• Peter Souter: Cool, works perfectly, accepted! :)