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

SpringFactoriesLoader.instantiateFactory(…) should report offending class on error #22453

Closed
odrotbohm opened this issue Feb 21, 2019 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@odrotbohm
Copy link
Member

SpringFactoriesLoader.instantiateFactory(…) reports the name of the interface that implementations are looked up for. However, it does not report the name of the actual implementation class whose failed creation is likely to have caused the exception.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 21, 2019
@jhoeller jhoeller self-assigned this Feb 22, 2019
@jhoeller
Copy link
Contributor

Isn't the implementation class name usually part of the nested exception anyway?

@odrotbohm
Copy link
Member Author

It is indeed. I just thought that the primary exception message could reveal the original offender, especially as it states "Unable to instantiate factory class" but then prints the interface name.

@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 5, 2019
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 5, 2019
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 5, 2019
@sbrannen
Copy link
Member

sbrannen commented Mar 5, 2019

@jhoeller, are you actively working on this? If not, I'd be happy to pick it up.

@sbrannen sbrannen added this to the 5.2 M1 milestone Mar 6, 2019
@sbrannen
Copy link
Member

sbrannen commented Mar 6, 2019

I actually did something similar for the TestContext framework which uses SpringFactoriesLoader behind the scenes to find the factory class names:

protected Set<Class<? extends TestExecutionListener>> getDefaultTestExecutionListenerClasses() {
Set<Class<? extends TestExecutionListener>> defaultListenerClasses = new LinkedHashSet<>();
ClassLoader cl = getClass().getClassLoader();
for (String className : getDefaultTestExecutionListenerClassNames()) {
try {
defaultListenerClasses.add((Class<? extends TestExecutionListener>) ClassUtils.forName(className, cl));
}
catch (Throwable ex) {
if (logger.isDebugEnabled()) {
logger.debug("Could not load default TestExecutionListener class [" + className +
"]. Specify custom listener classes or make the default listener classes available.", ex);
}
}
}
return defaultListenerClasses;
}

So I'll improve the exception message in SpringFactoriesLoader.instantiateFactory(…) in a similar fashion.

@sbrannen
Copy link
Member

sbrannen commented Mar 6, 2019

@odrotbohm, are you satisfied with the outcome in f087fd5?

@odrotbohm
Copy link
Member Author

LGTM.

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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants