JedisFactory hides exceptions

[three-old-coders]

Expected behavior

I got the exception

redis.clients.jedis.exceptions.JedisExhaustedPoolException: Could not get a resource since the pool is exhausted
at redis.clients.jedis.util.Pool.getResource(Pool.java:53)

on first access to the pool using getResource. The underlying problem is missing access rights for the "PING" command, but JedisFactory swallos this error silently and turns it in a pool exhausted

Actual behavior

public boolean validateObject(PooledObject<Jedis> pooledJedis) {
    BinaryJedis jedis = (BinaryJedis)pooledJedis.getObject();

    try {
        HostAndPort hostAndPort = (HostAndPort)this.hostAndPort.get();
        String connectionHost = jedis.getClient().getHost();
        int connectionPort = jedis.getClient().getPort();
        return hostAndPort.getHost().equals(connectionHost) && hostAndPort.getPort() == connectionPort && jedis.isConnected() && jedis.ping().equals("PONG");
    } catch (Exception var6) {
        return false;
    }
}

Jedis version:

3.3.0 + 3.4.0

[sazzad16]

@three-old-coders I'm not sure whether throwing an exception would break PooledObjectFactory's contract. Would log of that exception (by slf4j) help?

[three-old-coders]

a Log would be helpful, at least to see the root cause. Otherwise, without debugging decompiled classes it is impossible to see the underlying jedis error message.
On the other hand: Because this is a user/ACL issue (user lacks rights to "ping"), the pool will never get up and running, regardless how many objects are validated. So in this case: why not failing fast by re-throwing the original jedis exception, the pool is dead anyway, no chance to recover.

[sazzad16]

@gkorland ping

[gkorland]

@sazzad16 I agree we should start with error log

[yangbodong22011]

@sazzad16 I agree we should start with error log

+1
IMO, we should log for every hides exceptions.

[sazzad16]

Mostly resolved by #2336 and #2368.
For remaining one, PR #2502

[sazzad16]

Resolved.