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

AnnotationUtils.findAnnotation fails with poor diagnostics if it encounters an Annotation that references an unloadable class [SPR-11874] #16493

Closed
spring-projects-issues opened this issue Jun 12, 2014 · 13 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 12, 2014

Andy Wilkinson opened SPR-11874 and commented

We've hit this problem quite a few times recently with Boot's @ConditionalOnClass, @ConditionalOnMissingClass, @ConditionalOnBean, and @ConditionalOnMissingBean. They can all take a Class.

If Spring introspects a class or method with one of these annotations and the class that's referenced by the Annotation cannot be loaded it fails with java.lang.ArrayStoreException: sun.reflect.annotation.TypeNotPresentExceptionProxy and no information at all about what class could not be loaded. I don't think Spring can do anything about the lack of information about the class, but it could provide clearer information about which class/method was being introspected at the time of the failure.

We (the Boot team) have wondered if Spring should be using ASM-based annotation detection to avoid this problem altogether. An argument against this is that something else in the stack may well fail ungracefully even if Spring doesn't. In the case of the Boot conditions the problem's isolated to @Configuration classes. Could that reduce the chance of something else failing in a similar way and make it worthwhile to change Spring's behaviour?


Affects: 4.0.5

Reference URL: spring-projects/spring-boot#1065

Issue Links:

Referenced from: commits 74c878e, 2c0c081, fd809cd

@spring-projects-issues
Copy link
Collaborator Author

Phil Webb commented

Updating AnnotationUtils to swallow TypeNotPresentExceptionProxy might also help. Methods such as findAnnotation can probably safely assume that if the annotation refers to missing type it may as well not be there.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

All of our AnnotationUtils methods behave defensively now, catching unexpected exceptions from annotation retrieval attempts and proceeding like the annotation wasn't there in such a case.

This is analogous to what the JVM does for cases where the annotation type itself isn't present on the classpath. We're effectively extending that policy to values referenced within an annotation declaration.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

Juergen, could you add some logging when an exception is encountered? If the annotation is silently dropped I think it'll make it very hard to diagnose issues caused by the annotation not being honoured. I think something at info or even warning level would be appropriate.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Hmm, depends on whether we consider that case as valid or non-valid: I assumed Boot would run into such scenarios as part of its regular conditional bean definition arrangement? If that's not the case, I'm happy to add some log statements, of course...

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

Boot will only encounter the problem if one of its @Conditional annotations that takes a Class has been misused so, from a Boot perspective at least, logging something is appropriate.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Alright, consistently logging such failures at info level now. Since we might hit irrelevant classes with annotations that we don't really care about, I'm not keen on raising it to warn level.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Phil Webb commented

I'm not sure that I agree about INFO logging. Aren't these messages likely to confuse the user?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

It's just one-liners, no stacktraces... shouldn't be too confusing. We can suppress the weird TypeNotPresentExceptionProxy message part completely if that helps, just logging a regular could-not-load-annotation message. Or anyway, what would you prefer instead?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Phil Webb commented

INFO messages are logged by default with Spring Boot so probably DEBUG level would be better. At least there are no stack traces.

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

I disagree on DEBUG being better. The problem, and therefore the logging, will only occur in the event of a bug. If it's at DEBUG level then either we or the affected user will have to reproduce the problem with the appropriate logging configuration. If it's us trying to reproduce it, it may well be with very little to go on other than "some conditional stuff isn't working properly". With neither the ArrayStoreException nor any log output to guide us, that'll be like hunting for a needle in a haystack.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I'd like to leave it at INFO level as well... After all, the lookup for all annotations on a given element fails even if just one of those annotations refers to a non-loadable class. That element might not matter at all for Spring's purposes (which is why WARN is a bit too much) but it might also contain other significant annotations which on their own might be entire loadable but are now not due to some rogue annotation sitting next to them (which is why DEBUG is too low).

BTW, has anybody tested this on the IBM JVM? Does it fail the same way? Does it throw the same non-descriptive arrangement of nested exceptions?

If there's anything you'd like me to refine in terms of the log message, let me know.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

Good question about the J9 JVM. I'll try it. I intend to open an issue against the Oracle JVM about the non-descriptive exception so I'll need a simple reproduction anyway.

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

J9 behaves in exactly the same way as the Oracle JVM, right down to the use of sun.reflect.annotation.TypeNotPresentExceptionProxy.

There's already an open issue against OpenJDK. It's been deferred to Java 9.

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

2 participants