RedisConnection doesn't propogate redis connection exceptions when AbortOnConnectFail = true

[JuliusMikkela]

My understanding of AbortOnConnectFail is that it's used to toggle whether connection issues against redis should propogate immediately or on next attempt, which allows the multiplexer to be kept alive and continue retrying.

Currently RedisConnection's Execute and ExecuteAsync have try/catches that catches any exception thrown from IDatabase's Execute(Async), and then rethrows it as a normal exception and with its own formatting instead of propogating the actual exception.

One side-effect of this is that you can't catch connectivity exceptions in your application without digging into the InnerException, which makes building your application resilient to connectivity issues awkward at best.

I suggest the try/catches be removed to allow the real redisexception to propogate up.

Alternatively, if the wrapping is really desired for message formatting reasons or otherwise, then for it to be rethrown as some kind of redis.om or StackExchange.Redis exception rather than a basic Exception. If so, it might also be prudent to differentiate catching a RedisTimeoutException from a RedisCommandException, and so on.

Also, I see that neither RedisClientException or RedisIndexingException inherit from RedisException, which might be preferable to centralize all redis-related errors under the RedisException umbrella (makes catching them a lot easier).

[slorello89]

Hi @JuliusMikkela - so the issue here is that adding RedisException as a parent class of our internal exceptions is that it will increase the extent to which the Redis OM API is dependent on StackExchange.Redis' public API. To this date the only intermingling we have is that the RedisConnectionProvider will take a connection multiplexer (a highly localized API which can be kicked out to an extension one day if needed).

It's true that there isn't much (or really anything) by way of advanced exception handling for client interactions here. We should create a set of exceptions based off of common exceptions we might get from the inner client, but the rationale behind the catch/wrap/throw is to prevent an unnecessary comingling of the APIs

[JuliusMikkela]

Alright @slorello89, if that's the intended design then I agree that it would be better to create an umbrella/base exception set for Redis.OM than expand on and propogate StackExchange's models.

Personally I'd probably let the dependency be high until the OM is developed enough to take on a life of its own, and then make a breaking major revision, but that's just personal preference.

I'll adapt my PR, and take an extra look around for places where StackExchange might bleed through.