spring-cloud / spring-cloud/spring-cloud-commons

Continue rebinding the ConfigurationProperties when any of them fails

Open
#1,129 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

waiting-for-triage
Dominant language
Java
Stars
751
Forks
744
Avg merge
1d 14h
Merged PRs (30d)
9

Description

Is your feature request related to a problem? Please describe.
Currently when ConfigurationPropertiesRebinder.rebind is executed and any ConfigurationProperties bean fails, the method throws an exception and stops rebinding the following ConfigurationProperties beans.

I do know that exists the property spring.cloud.refresh.never-refreshable which prevents rebind some specific beans. However we are building a common library which uses the rebind functionality and it is necessary for the users of the library to declare FactoryBeans to create their own objects (it is one of the cases which provokes that rebind fails). So we cannot know the classes beforehand.

Besides if you see the code of RefreshScopeHealthIndicator, it accepts a Map with the errors. However with the current implementation of ConfigurationPropertiesRebinder at most only one error will be reported.

Describe the solution you'd like
I would like that even if a single ConfigurationProperties bean fails rebinding, the following ConfigurationProperties still be rebinded and do not throw any exception.

Describe alternatives you've considered
I would suggest to not to throw any exception when there is an exception rebinding a ConfigurationProperties and log the error instead. Thus all the ConfigurationProperties that do not fail will be rebinded and the list of the errors would be complete. Something like:

        @ManagedOperation
	public boolean rebind(String name) {
		if (!this.beans.getBeanNames().contains(name)) {
			return false;
		}
		if (this.applicationContext != null) {
			try {
				Object bean = this.applicationContext.getBean(name);
				if (AopUtils.isAopProxy(bean)) {
					bean = ProxyUtils.getTargetObject(bean);
				}
				if (bean != null) {
					// TODO: determine a more general approach to fix this.
					// see https://github.com/spring-cloud/spring-cloud-commons/issues/571
					if (getNeverRefreshable().contains(bean.getClass().getName())) {
						return false; // ignore
					}
					this.applicationContext.getAutowireCapableBeanFactory().destroyBean(bean);
					this.applicationContext.getAutowireCapableBeanFactory().initializeBean(bean, name);
					return true;
				}
			}
			catch (RuntimeException e) {
				this.errors.put(name, e);
				LOG.warn(String.format("Error rebinding the bean '%s'", e);
			}
			catch (Exception e) {
				this.errors.put(name, e);
				LOG.warn(String.format("Error rebinding the bean '%s'", e);
			}
		}
		return false;
	}

If you agree with the solution I could make a PR with this.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with ConfigurationPropertiesRebinder.rebind in spring-cloud-context/src/main/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinder.java and compare its error handling with RefreshScopeHealthIndicator in spring-cloud-context/src/main/java/org/springframework/cloud/health/RefreshScopeHealthIndicator.java. Done means later ConfigurationProperties continue rebinding after one failure, while each failure is retained in the errors map and logged.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring, spring-boot
Domain
backend
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.