Skip to content

Non-helpful NoSuchBeanDefinitionException rather than BeanNotOfRequiredTypeException due to creation order [SPR-14504] #19073

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

Closed
spring-projects-issues opened this issue Jul 21, 2016 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 21, 2016

Andy Wilkinson opened SPR-14504 and commented

With reference to #19047, I am trying to add a FailureAnalyzer to Spring Boot that outputs some advice when refresh fails due to a BeanNotOfRequiredTypeException. In doing so I have observed an unpleasant side-effect of bean creation ordering not being deterministic. Sometimes refresh will fail with a BeanNotOfRequiredTypeException as the root cause and other times the root cause will be a NoSuchBeanDefinitionException. When it's the latter there is no information to help you figure out why there was no such bean.

Here's a small class that demonstrates the problem caused by the non-deterministic ordering:

public class NonDeterministicBeanCreationOrdering {

	public static void main(String[] args) {
		try {
			new AnnotationConfigApplicationContext(JdkProxyConfiguration.class).close();
		}
		catch (Exception ex) {
			Throwable rootCause = getRootCause(ex);
			if (!(rootCause instanceof BeanNotOfRequiredTypeException)) {
				throw new IllegalStateException("Unexpected root cause", rootCause);
			}
		}
	}

	private static Throwable getRootCause(Throwable candidate) {
		while (candidate.getCause() != null) {
			candidate = candidate.getCause();
		}
		return candidate;
	}

	@Configuration
	@EnableAsync
	static class JdkProxyConfiguration {

		@Bean
		public AsyncBean asyncBean() {
			return new AsyncBean();
		}

		@Bean
		public AsyncBeanUser user(AsyncBean bean) {
			return new AsyncBeanUser(bean);
		}

	}

	static class AsyncBean implements SomeInterface {

		@Async
		public void foo() {

		}

		@Override
		public void bar() {

		}

	}

	static interface SomeInterface {

		void bar();

	}

	static class AsyncBeanUser {

		AsyncBeanUser(AsyncBean asyncBean) {
		}

	}

}

If you run it a few times you should see the two different root causes. The key thing is the order in which asyncBean and user are created.

If asyncBean is created first its proxy is stored in the bean factory and, subsequently, the attempt to find an AsyncBean instance for injection into the user bean method fails as there's no matching bean.

If user is created first then its creation triggers the creation of asyncBean. I think this means that asyncBean is directly available to be passed into the user bean method rather than requiring a bean factory lookup. When this fails as the types don't actually match a BeanNotOfRequiredTypeException is thrown with details of the actual type.

Ideally, the ordering would be deterministic, but I realise that's almost certainly not possible in 4.3 and perhaps at all. Failing that, some information in the NoSuchBeanDefinitionException that points towards the proxied asyncBean would be very useful.


Affects: 4.3.1

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

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Bean initialization is generally deterministic from the core container's perspective, since we're always approaching it in registration order. However, with @Bean methods, this becomes dependent on the underlying JVM and the order of its Class#getDeclaredMethods result... We could only really try to sort the methods alphabetically or based on some other artificial order to work around that effect.

In any case, I'll see what we can do here towards better reporting at least.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Running the test above locally, it indeed is the Class#getDeclaredMethods() ordering. I wasn't aware it's that random; I thought it just differed between JVM environments but not between different runs within the exact same environment. But at least there seems to be no other non-determinism involved since it correlates 1:1.

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

Running the test above locally, it indeed is the Class#getDeclaredMethods() ordering. I wasn't aware it's that random

Wow. I'd assumed it perhaps varied based on the ordering of the byte code so it'd only change upon recompilation.

We could only really try to sort the methods alphabetically or based on some other artificial order to work around that effect.

That's a change that I would like to see. Perhaps it could be considered for 5.0?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 21, 2016

Juergen Hoeller commented

Indeed, see #19074 :-)

I'll keep this issue for the exception reporting improvement in such a scenario... which we should generally aim for (since even manual ordering may easily lead to such a situation) and which we'll backport to 4.3.2.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

An important note: Bean registration order deterministically follows source-code method declaration order with ASM parsing!
The non-determinism here comes with parsing via reflection. Since we choose the mechanism based on what we got registered, this means:

  • source code order when a class name String has been registered (or found through classpath scanning);
  • but arbitrary order when a Class has been registered.

As a consequence, the following variation of your sample code above provides consistent results for me:

public static void main(String[] args) {
     try {
          // new AnnotationConfigApplicationContext(JdkProxyConfiguration.class).close();
          AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
          ctx.registerBeanDefinition("config", new RootBeanDefinition(JdkProxyConfiguration.class.getName()));
          ctx.refresh();
          ctx.close();
     }
     catch (NestedRuntimeException ex) {
          if (!(ex.getRootCause() instanceof BeanNotOfRequiredTypeException)) {
               throw new IllegalStateException("Unexpected root cause", rootCause);
          }
     }
}

(As a side note, for Spring exceptions, you can catch NestedRuntimeException which provides methods such as getRootCause() directly :-))

So it seems that regular classpath scenarios - including regular Boot usage - get reliable results anyway, and that only Class-registering setups (typically tests and custom micro-bootstraps) suffer from the non-determinism based on the JVM that they are running on? That makes it hard to artifically declare a different order for ASM parsing since source-code declaration is arguably ideal... but aligning our reflection parsing to also provide source-code order seems impossible :-(

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I've addressed this with a general check for such bean target type mismatches, leading to a BeanNotOfRequiredTypeException if applicable, before we raise a plain NoSuchBeanDefinitionException in our resolveDependency algorithm.

This will be helpful for configuration class ordering issues but also for explicit ordering with the proxied bean definition coming first and a bean with a corresponding injection point later on.

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

2 participants