Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@Cacheable throws NullPointerException when RuntimeException is thrown inside annotated code #33492

Closed
drg2000 opened this issue Sep 5, 2024 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@drg2000
Copy link

drg2000 commented Sep 5, 2024

Hi, I have a piece of code in a reactive project that looks something like that (similar code for demo purposes):

@Cacheable(value = RedisCacheNames.CONSTANT, cacheManager = "redisCacheManager")
public Mono<String> get(Integer id) {
      return Mono.just("result")
          .doOnSuccess(s -> throwException())
  }

  private void throwException() {
    throw new RuntimeException();
  }

I have a specific exception, but it basically extends from RuntimeException.

I've been debugging a little, and it seems that CacheAspectSupport had a piece of code added in commit 8974da2 that changed ReactiveCachingHandler from this:

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)));

to this:

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);
									return evaluate(null, invoker, method, contexts);
								}
								catch (RuntimeException exception) {
									return Mono.error(exception);
								}
							}));

The thing is, first evaluate call before the method execution sets the contexts.processed to true, and after my method throws the runtime exception, is caught by this onErrorResume, which calls evaluate with cacheHit set to null, and

if (contexts.processed) {
	return cacheHit;
}

returns null.

Is this an actual issue or I am missing something?

As of now I've done a couple of extra tests, and it seems that I cannot throw a RuntimeException. I have two CacheManagers configured in my Spring Boot project, one for Caffeine and the other one for Redis. This only happens when using the RedisCacheManager as seen above.

I think this might be because CaffeineCache implementation of Cache returns null when the element is not present whereas RedisCache implementation returns a cachedFuture.

Thanks in advance for any help provided!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 5, 2024
@drg2000 drg2000 changed the title @Cacheable throws NullPointerException when throwing exception inside annotated code @Cacheable throws NullPointerException when throwing RuntimeException inside annotated code Sep 5, 2024
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Sep 5, 2024
@sbrannen sbrannen changed the title @Cacheable throws NullPointerException when throwing RuntimeException inside annotated code @Cacheable throws NullPointerException when RuntimeException is thrown inside annotated code Sep 5, 2024
@simonbasle
Copy link
Contributor

If your exception is a subtype of RuntimeException, and you have an error handler in place, perhaps you can tune the error handler to rethrow that particular type of exception ?

@simonbasle simonbasle added the status: waiting-for-feedback We need additional information before we can continue label Sep 19, 2024
@simonbasle simonbasle self-assigned this Sep 19, 2024
@drg2000
Copy link
Author

drg2000 commented Sep 20, 2024

Hi @simonbasle ,

I think I don't understand your proposal. In the project we have a controller advice that catches our business runtime exceptions. The thing is, that if I throw SpecificException my advice expects that specific class type, but since @Cacheable throws a null pointer it is handled by our generic handler that catches Exception.class. For the client, the message should look like: "This is our business exception" and not like "The nextFactory returned a null Publisher".

What kind of error handling are you proposing in this case?

Thanks for your time

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 20, 2024
@simonbasle
Copy link
Contributor

@drg2000 I see, the NPE's message is important context. I'll investigate further, but if you can provide a unit test or simple reproducer it would help tremendously.

@simonbasle simonbasle added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Sep 23, 2024
@drg2000
Copy link
Author

drg2000 commented Sep 27, 2024

Hi @simonbasle and sorry for the late reply, been kind of busy lately.

I've created a small spring project which replicates the issue. You just can launch the Application test or run the app and NullPointerException should be displayed in the console. Any more information I can provide please let me know, the repo url is here: https://github.com/drg2000/bug-demo.

Thanks for your time

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 27, 2024
@simonbasle simonbasle added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Sep 30, 2024
@simonbasle simonbasle added this to the 6.1.14 milestone Sep 30, 2024
@simonbasle
Copy link
Contributor

Thanks for the reproducing project, it helped me verify that my tentative fix was working for your case 👍

@drg2000
Copy link
Author

drg2000 commented Sep 30, 2024

Hi @simonbasle, thanks for your help!

It would be helpful If you could post a comment here once the fix is done, just out of curiosity to see which was the solution.

Thanks for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants