Better error reporting for @Bean creating when bean instance has been replaced with an incompatible type

[junyongz]

This would be even tougher to spot the error if the BeanPostProcessor instance is from other libraries.

12:43:19.439 [main] WARN org.springframework.context.annotation.AnnotationConfigApplicationContext - Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'taskExecutor' defined in com.welcome.samples.junit.BeanPostProcessorComponentTest$CustomAsyncConfigurer: Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [java.util.concurrent.Executor]: Illegal arguments to factory method 'getAsyncExecutor'; args: ; nested exception is java.lang.IllegalArgumentException: object is not an instance of declaring class

class BeanPostProcessorComponentTest {

    @Test
    void endToEnd() {
        AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
        ctx.register(CustomAsyncConfigurer.class, ProcessingAsyncConfigurerPostProcessor.class);
        ctx.refresh();
        ctx.close();
    }
    
    
    @Configuration
    public static class CustomAsyncConfigurer implements AsyncConfigurer {
        
        @Bean("taskExecutor")
        public Executor getAsyncExecutor() {
            return new ConcurrentTaskExecutor();
        }
    }

    public static class ProcessingAsyncConfigurerPostProcessor implements BeanPostProcessor {
        public Object postProcessBeforeInitialization(Object bean, String beanName)
                throws BeansException {
            if (bean instanceof AsyncConfigurer && !(bean instanceof FullyCustomAsyncConfigurer)) {
                return new FullyCustomAsyncConfigurer((AsyncConfigurer) bean);
            }
            
            return bean;
        }
    }
    
    public static class FullyCustomAsyncConfigurer extends AsyncConfigurerSupport {
        
        private AsyncConfigurer delegate;
        
        public FullyCustomAsyncConfigurer(AsyncConfigurer delegate) {
            this.delegate = delegate;
        }
        
        @Override
        public Executor getAsyncExecutor() {
            return new Executor() {
                
                @Override
                public void execute(Runnable command) {
                    FullyCustomAsyncConfigurer.this.delegate.getAsyncExecutor().execute(command);
                }
            };
        }
        
        @Override
        public AsyncUncaughtExceptionHandler getAsyncUncaughtExceptionHandler() {
            return this.delegate.getAsyncUncaughtExceptionHandler();
        }
    }

}

[snicoll]

I agree that the exception could be improved but the fact that you replace a bean instance (CustomAsyncConfigurer) by an instance that is not assignable to it is really breaking the post-processor contract. FullyCustomAsyncConfigurer does not extend CustomAsyncConfigurer so the override of getAsyncExecutor is not overriding that method.

Looking at the catch block, it looks like it was primarily designed for providing an error report if the arguments specified for the method invocation has some sort of mismatch. We'll add an extra check there to throw a more specific exception so that we only pay the cost of the check if the exception is thrown.