`BlockingRedisDataSourceImpl` doesn't call DISCARD if consumer function throws an exception

[Ladicek]

Discussed in #49436

^{Originally posted by luca-bassoricci August 8, 2025}
In BlockingRedisDataSourceImpl#withTransaction() if the call to BiConsumer<I, TransactionalRedisDataSource> tx throws an exception DISCARD command is not issued left redis in a stale state.
Is this the expected flow?
Would be a good idea to try/catch tx.accept() call and send DISCARD from BlockingRedisDataSourceImpl#withTransaction() if TransactionalRedisDataSource#discard() has not been called?

Pseudo

try {
  tx.accept(t, u);
}
catch (Throwable e)  {
  if(!u.discarded())  {
    u.discard();
  }
}

Looking at ReactiveRedisDataSourceImpl it sends a DISCARD command in case of failure.

Thanks
Luca.

[quarkus-bot]

/cc @cescoffier (redis), @machi1990 (redis)

[Ladicek]

As I mentioned in the discussion, this seems sensible. It turns out that out of 3 withTransaction() methods in ReactiveRedisDataSourceImpl, only 2 of them automatically send DISCARD. BlockingRedisDataSourceImpl doesn't send DISCARD in either of the 3 withTransaction() methods.

I don't know if there's a difference between these methods, but I don't think so, at least when it comes to which of them should/shouldn't automatically send DISCARD. They should be consistent in this regard.

[Ladicek]

I have a fix locally (I think 😆), but I'm not done yet: need to write tests, and need to decide on the API (what to do when an exception is thrown, rethrow or swallow, and it should be consistent between blocking and reactive variants).

[luca-bassoricci]

Currently in my code I workarounded the problem in this way

1. catching exception
2. discarding connection if necessary
3. and rethrowing exception because I want to be notified of the error.

I'm not a big supporter of exception swallowing, especially if them can be user exceptions.