Kredis.hash fails with nil values in Redis 5+

[swanson]

We've been bite several times in production by trying to set a kredis hash with the value of a key being nil. As we know, accidental nils are not rare in Ruby :(

Kredis attempts to call hset in redis with a nil value, which is not supported and you get this exception:

/Users/matt/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/redis-client-0.19.1/lib/redis_client/command_builder.rb:37:in `block in generate': 
Unsupported command argument type: nil (TypeError)

            raise TypeError, "Unsupported command argument type: #{element.class}"
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This PR runs Hash#compact on the values passed to Kredis before trying to send them to Redis via hset. This would remove invalid key/value pairs. Later, when reading the stored hash back, you would rely on the Ruby default return value of nil when accessing a non-existing key.

[swanson]

The root cause can be traced back to adding support for v5 redis gem:

redis/redis-rb#1142 (comment)

which changed the behavior (for good reason): redis/redis-rb#1142 (comment)

[jeremy]

If this is disallowed, seems we should raise ArgumentError on nil values rather than mask the issue by compacting.

And perhaps handled more broadly in Kredis::TypeCasting - does this affect hashes only?

[swanson]

If this is disallowed, seems we should raise ArgumentError on nil values rather than mask the issue by compacting.

From my perspective, it seems desirable that Kredis would support nil hash values -- I am using kredis to interact with the "coherent object" (in this case a Ruby hash where nil values are fine). If that is the case, then compacting seems appropriate to me as it is Kredis that should manage the translation of a Ruby hash to a Redis internal representation.

If that is not desired, then I agree, the library should raise a more appropriate error before we are deep inside the redis-client.

[jeremy]

Agreed! That would be super confusing. This is practical.