ReactiveCachingHandler still not using error handler on sync cache.

[guga-ciandt]

Changes made to resolve issue 21590 didn't cover ReactiveCachingHandler, which still has the same issue.

When a reactive @Cacheable is set to sync=true, errors are not redirected to CacheErrorHandler.

Affects: 6.2.5

[jhoeller]

As of 6.2, we forward exceptions from the actual cache.retrieve call to the CacheErrorHandler but not exceptions produced by the returned CompletableFuture. As far as I can tell, this is consistent between sync=false and sync=true, it rather depends on the use of the CompletableFuture-based contract. What are you seeing specifically for sync=true there but not for sync=false?

Are you expecting late CompletableFuture-triggered exceptions to show up in the CacheErrorHandler as well? We'd have to additionally decorate the CompletableFuture then, both for Cache.retrieve(key) and Cache.retrieve(key, valueLoader).

[guga-ciandt]

Current ReactiveCachingHandler code does handle it for findInCaches, which is used when sync=false:

...
				if (adapter.isMultiValue()) {
					return adapter.fromPublisher(Flux.from(Mono.fromFuture(cachedFuture))
							.switchIfEmpty(Flux.defer(() -> (Flux) evaluate(null, invoker, method, contexts)))
							.flatMap(v -> evaluate(valueToFlux(v, contexts), invoker, method, contexts))
							.onErrorResume(RuntimeException.class, ex -> {
								try {
									getErrorHandler().handleCacheGetError((RuntimeException) ex, cache, key);
									Object e = evaluate(null, invoker, method, contexts);
									return (e != null ? e : Flux.error((RuntimeException) ex));
								}
								catch (RuntimeException exception) {
									return Flux.error(exception);
								}
							}));
				}
				else {
					return adapter.fromPublisher(Mono.fromFuture(cachedFuture)
							.switchIfEmpty(Mono.defer(() -> (Mono) evaluate(null, invoker, method, contexts)))
							.flatMap(v -> evaluate(Mono.justOrEmpty(unwrapCacheValue(v)), invoker, method, contexts))
							.onErrorResume(RuntimeException.class, ex -> {
								try {
									getErrorHandler().handleCacheGetError((RuntimeException) ex, cache, key);
									Object e = evaluate(null, invoker, method, contexts);
									return (e != null ? e : Mono.error((RuntimeException) ex));
								}
								catch (RuntimeException exception) {
									return Mono.error(exception);
								}
							}));
				}
...

On sync=true, executeSynchronized is used, which doesn't handle it to the error handler:

		public Object executeSynchronized(CacheOperationInvoker invoker, Method method, Cache cache, Object key) {
			ReactiveAdapter adapter = this.registry.getAdapter(method.getReturnType());
			if (adapter != null) {
				if (adapter.isMultiValue()) {
					// Flux or similar
					return adapter.fromPublisher(Flux.from(Mono.fromFuture(
							cache.retrieve(key,
									() -> Flux.from(adapter.toPublisher(invokeOperation(invoker))).collectList().toFuture())))
							.flatMap(Flux::fromIterable));
				}
				else {
					// Mono or similar
					return adapter.fromPublisher(Mono.fromFuture(
							cache.retrieve(key,
									() -> Mono.from(adapter.toPublisher(invokeOperation(invoker))).toFuture())));
				}
			}
			if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isSuspendingFunction(method)) {
				return Mono.fromFuture(cache.retrieve(key, () -> {
					Mono<?> mono = ((Mono<?>) invokeOperation(invoker));
					if (mono == null) {
						mono = Mono.empty();
					}
					return mono.toFuture();
				}));
			}
			return NOT_HANDLED;
		}
```

[jhoeller]

Thanks for pointing that out. So it looks like it was actually PR #33073 that was incomplete. We'll close that gap for 6.2.6, also for CompletableFuture-based @Cacheable(sync=true) processing.

[jhoeller]

This is available in the latest 6.2.6 snapshot now. Please give it an early try if you have the chance!