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

Don't log expected NoSuchMethodException after it's caught. #4545

Conversation

soul2zimate
Copy link
Contributor

@soul2zimate soul2zimate commented Feb 26, 2019

WildFly and Red Hat JBoss Enterprise Application users reports issue in https://developer.jboss.org/message/960985 and https://issues.jboss.org/browse/WFLY-6918

When a AnnotationProvider implementation doesn't have a two parameters(javax.servlet.ServletContext, com.sun.faces.spi.AnnotationProvider) which is optional,

ServiceFactoryUtils will log a confusing NoSuchMethodException when WildFly and Red Hat JBoss Enterprise Application with a TRACE logger level enabled.

This change stop logging for expected NoSuchMethodException after it's caught.

Downstream 2.3 branch PR: #4590

@soul2zimate soul2zimate force-pushed the AnnotationProviderFactory-fix-master branch 2 times, most recently from fae6162 to c7f4c91 Compare February 27, 2019 05:53
@soul2zimate soul2zimate changed the title remove useless getProviderFromEntry call with two parameters constructor Remove unnecessary getProviderFromEntry call with two parameters constructor Feb 27, 2019
@soul2zimate
Copy link
Contributor Author

Can anyone review this change please?

@soul2zimate
Copy link
Contributor Author

Red Hat JBoss Enterprise Application users have reported that when you turn on TRACE logging for javax.enterprise.resource then you can notice NoSuchMethodException during JSF app deployment. The problem is that org.jboss.as.jsf.injection.JandexAnnotationProvider doesn't have corresponding constructor (with two params - javax.servlet.ServletContext, com.sun.faces.spi.AnnotationProvider)
We think it needs to remove the unnecessary getProviderFromEntry call with two parameters constructor as described in PR description to avoid this NoSuchMethodException.

see more about issue report in forum discussion https://developer.jboss.org/message/960985 and WFLY community issue in https://issues.jboss.org/browse/WFLY-6918

@soul2zimate soul2zimate force-pushed the AnnotationProviderFactory-fix-master branch 4 times, most recently from 9bfca83 to f08091a Compare May 29, 2019 06:33
@soul2zimate soul2zimate changed the title Remove unnecessary getProviderFromEntry call with two parameters constructor rethrow serious problem exceptions in ServiceFactory and handle them differently after it's caught. May 29, 2019
Signed-off-by: Chao Wang <chaowan@redhat.com>
@soul2zimate soul2zimate changed the title rethrow serious problem exceptions in ServiceFactory and handle them differently after it's caught. Don't log expected NoSuchMethodException after it's caught. May 30, 2019
@soul2zimate soul2zimate force-pushed the AnnotationProviderFactory-fix-master branch from f08091a to 6d1d620 Compare May 30, 2019 07:35
@bmaxwell
Copy link

Can an admin review this PR so that we can resolve it? If the code is intended to handle multiple method signatures, then it should not be logging exceptions such as NoSuchMethodException which suggests an error occurred. Better trace logging if you want to know which method it invoke would be to log that the method signature you ended up invoking.

@xstefank
Copy link

@arjantijms @juneau001 can any of you take a look at this PR please?

@xstefank
Copy link

Review pinging again. Can this be merged, please?

@arjantijms arjantijms self-assigned this Jul 15, 2019
@arjantijms
Copy link
Contributor

Review pinging again. Can this be merged, please?

Thanks for your patience and pinging again, we're currently very busy with the Jakarta EE 8 release, but this (and a number of outstanding other Mojarra PRs) will have the very next priority. Should be within a few days. At a glance the PR seems fine.

@xstefank
Copy link

Thanks @arjantijms!

@soul2zimate
Copy link
Contributor Author

Hi, Any chance this can be merged?

@arjantijms arjantijms merged commit 9266d23 into eclipse-ee4j:master Jul 23, 2019
@soul2zimate soul2zimate deleted the AnnotationProviderFactory-fix-master branch July 25, 2019 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants