Support validation of properties on refresh

[zhenbianshu]

I am using spring cloud configuration.

I have a property bean annotated with @RefreshScope, it keeps my dynamic properties, it refreshes my applicationContext using ContextRefresher after noticing my properties changed, I found that when new properties invalid, it can also refreshes successfully, but when I used this property bean, it throwed a exception with message "error creating a bean with name 'xxx'".

I looked up source code and found the reason, when ContextRefresher refreshed, beans in refresh scope will be destroyed, new bean will not be created immediately because of lazyInit. It created when I first used it, and then spring resolved new properties failed, leads to new bean create fail, so I got the exception. And worse, every time I use it, it is created again and the exception occurs again.

I hope that it could fallback to old properties bean when new properties is invalid.

Sorry for my poor English, Looking forward to your reply.

Thanks.

[ryanjbaxter]

I am not sure what we can do to make the experience any better. Since the properties are invalid, even if we eagerly created the bean you would still get an exception. I don't see us implementing a "fallback" solution for the case where your configuration is incorrect.

[qdaxb]

I have the same problem. Sometimes error configuration causes all my service to crash. I think invalid properties could make an exception during bean creating, but not destroy the pervious bean.

[spencergibb]

That would require a major redesign

[spencergibb]

Here's a similar problem in gateway, though because it's more focused the solution is simpler, but could provide inspiration here. https://github.com/spring-cloud/spring-cloud-gateway/issues/600#issuecomment-593339729

[andrew-yang]

We also had a use case where we wanted to be preventative in case bad config broke beans. Our approach was to create a new Scope called SafeRefreshScope. This Scope attempts to create the new instance with new configuration before destroying the old bean. If that creation with the new configuration fails, we still have the original bean in existence so we default back to it. If creation succeeds, we proceed to destroy the old bean and use the new one. (Downside is duplicate bean definitions for beans that are marked SafeRefreshScope, so there is some dirtyness there, but otherwise its been working well for us)

In our view, it was dangerous that configuration changes could possibly break your service. Our SafeRefreshScope added some resiliency against human/user error with updating config.

I also asked for something similar in the past. https://github.com/spring-cloud/spring-cloud-commons/issues/403

[dwen77]

IMO, it is a valid point to have the safe refresh scope. In some cases, people who makes the change of the yaml configuration file does not know exactly how the mapped POJO class is structured, therefore, some mistakes can be made. We must handle the human errors gracefully other than crashing service. It would be great if Spring could offer such a feature officially.

[spencergibb]

I'd recommend NOT using refresh scope in that case and responding to the event mentioned in #403. Nothing gets destroyed.

[dwen77]

Hi @spencergibb , thanks for the response!!

In my setup, I use Spring cloud bus to broadcast the events from config server to clients and I don't explicitly call /actuator/refresh endpoint. I did not know the @RefreshScope annotation is not needed for the class which is annotated by @ConfigurationProperties in such a setup. Thanks for pointing out it!

I just removed the @RefreshScope and made some tests. The properties are refreshed if the configuration change is valid, otherwise, the old configuration is kept and nothing is broken! It is good!

I have one more followup question though. Do you suggest to respond to the event so that we could validate the change and potentially react on errors (for instance, log the validation error or even create an alert)?