spring-projects / spring-projects/spring-framework

TransactionAwareCacheDecorator renders CacheErrorHandler useless

Open
#28,554 6 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

in: core status: feedback-provided type: enhancement
Dominant language
Java
Stars
60.2k
Forks
38.8k
Avg merge
5d 2h
Merged PRs (30d)
27

Description

Affects: All Versions since 3.2?

When configuring a custom cache error handler via CachingConfigurer::errorHandler and having transaction-aware caching enabled (for example, via RedisCacheManager.RedisCacheManagerBuilder::transactionAware, which decorates with TransactionAwareCacheDecorator), the cache error handler is never called when the put fails in TransactionSynchronization::afterCommit once the transaction was committed. In particular, this prevents users to suppress runtime exceptions from the cache backend by using the LoggingCacheErrorHandler, such as connection problems or command timeouts from Redis.

To illustrate the problem, I've created a simple demo project using Redis as a cache backend (which cannot connect as there's no Redis running on localhost:6379)

The test where I did not enable transaction-awareness does not throw any exception, whereas the test with transaction-awareness does, rather unexpectedly, as I've installed a LoggingCacheErrorHandler.

Note that I've configured a very dummy transaction handling to make the bug appear.

A workaround for the bug would be to not enable transaction-awareness via RedisCacheManager.RedisCacheManagerBuilder::transactionAware, but to instrument the cache manually. I did this with AOP on any cache instance by decorating the CacheManager::getCache with a BeanPostProcessor, but this is quite ugly:

@Aspect
@RequiredArgsConstructor
@EqualsAndHashCode
public class CacheTransactionAwareAspect {
    private final CacheErrorHandler cacheErrorHandler;
    private final Cache cache;

    private static Object proceedAfterCommit(ProceedingJoinPoint pjp, Consumer<RuntimeException> errorHandler) throws Throwable {
        if (TransactionSynchronizationManager.isSynchronizationActive()) {
            TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
                @Override
                public void afterCommit() {
                    try {
                        pjp.proceed();
                    } catch (RuntimeException e) {
                        errorHandler.accept(e);
                    } catch (Throwable e) {
                        throw new RuntimeException(e);
                    }
                }
            });
            return null;
        } else {
            return pjp.proceed();
        }
    }

    @Around("execution(* org.springframework.cache.Cache.put(..))")
    public Object wrapPutMethod(ProceedingJoinPoint pjp) throws Throwable {
        return proceedAfterCommit(pjp, e -> {
            var args = pjp.getArgs();
            cacheErrorHandler.handleCachePutError(e, cache, args[0], args[1]);
        });
    }

    @Around("execution(* org.springframework.cache.Cache.evict(..))")
    public Object wrapEvictMethod(ProceedingJoinPoint pjp) throws Throwable {
        return proceedAfterCommit(pjp, e -> {
            var args = pjp.getArgs();
            cacheErrorHandler.handleCacheEvictError(e, cache, args[0]);
        });
    }

    @Around("execution(* org.springframework.cache.Cache.clear(..))")
    public Object wrapClearMethod(ProceedingJoinPoint pjp) throws Throwable {
        return proceedAfterCommit(pjp, e -> {
            cacheErrorHandler.handleCacheClearError(e, cache);
        });
    }
}

Let me know if you need further information to reproduce the bug.

I've also just tried a fix, but I don't know how to get the error handler (which should be kind of a singleton from CachingConfigurer) into the AbstractTransactionSupportingCacheManager. Any hints would be appreciated and I'd create a PR if this attempt goes into the right direction.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with TransactionAwareCacheDecorator and the CacheErrorHandler contract, then trace how CachingConfigurer configures the handler and how AbstractTransactionSupportingCacheManager creates transaction-aware caches. Reproduce the difference using the linked transaction-aware and non-transaction-aware demo tests, then add coverage showing that afterCommit cache failures reach the configured handler without escaping as runtime exceptions.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, redis, spring
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.