Response exception after discarded a transaction

[yaxing]

Sample code:

Jedis j = new Jedis("host", port);
Transaction t = j.multi();
Response<String> resp = t.get("key");
t.discard();
String data = resp.get();

Such exception got thrown at the last line:

redis.clients.jedis.exceptions.JedisDataException: Please close pipeline or multi block before calling this method.

I see that generateResponse is not called in discard().

Is this expected? The confusing part is that it's the same exception where you forgot to call exec() or even the same when you forgot to sync a pipeline, thus making it hard to handle for a user. Is there any way to handle it better, maybe throwing a different exception saying the transaction got discarded?

[marcosnils]

@yaxing thanks for reporting the issue. I'll check this out during the weekend.

Any suggestions about how you'd expect this to work as a Jedis user?

[yaxing]

@marcosnils thanks. What I think might work better would be a different type of exception for such cases. like TransactionDiscarded, PipelineClosed etc.

[marcosnils]

I thought about this and I believe the best approach would be to change the type of the exception to JedisException instead of JedisDataException and to improve the logging message. The reason for this are:

1. Currently JedisDataException is used mainly when Redis returns an error about the operation. The pipeline/transaction validation we're doing it's something from the Jedis side, so it doesn't make sense to return this type of exception

2. What would be the benefit of returning a custom exception on this case?. You usually return a custom exception when you want your code to do some custom exception handling. In the case of Pipeline / Transaction being used incorrectly (as in the example of this issue) there's nothing to recover from because it's an incorrect use and Jedis users would need to change to code to fix this.

3. I see that build method in Response class also returns a JedisDataException which also leads to confusion about what to do in terms of the state of the transaction/pipeline vs what redis returned. I guess this reinforces the fact that we should use a different exception (JedisException) when pipline / transaction is being used incorrectly.

@HeartSaVioR @xetorthio @yaxing what do you think?

[HeartSaVioR]

@marcosnils
Sorry to be late.
I also think we shouldn't use JedisDataException which is not ERR from Redis side.
Could you address this? Thanks!

[marcosnils]

I'm on vacation ATM I'll fix this once I get back next week.

sent from mobile
On Jan 6, 2016 2:27 AM, "Jungtaek Lim" notifications@github.com wrote:

@marcosnils https://github.com/marcosnils
Sorry to be late.
I also think we shouldn't use JedisDataException which is not ERR from
Redis side.
Could you address this? Thanks!

—
Reply to this email directly or view it on GitHub
#1183 (comment).

[sazzad16]

Considered resolved by #2393