From 196db71d6e1d3d94263a5e50e6c354c22d034df7 Mon Sep 17 00:00:00 2001 From: denyskonakhevych Date: Tue, 24 Oct 2023 20:52:03 +0200 Subject: [PATCH 1/2] Bugfix/2755 IllegalStateException after read timeout --- spring-boot-admin-client/pom.xml | 5 +++++ ...pringBootAdminClientAutoConfiguration.java | 9 ++++++++ .../BlockingRegistrationClient.java | 9 +++++--- .../DefaultApplicationRegistrator.java | 8 ++++++- .../ReactiveRegistrationClient.java | 21 ++++++++++++++----- .../registration/RegistrationClient.java | 4 +++- .../AbstractRegistrationClientTest.java | 4 +++- .../DefaultApplicationRegistratorTest.java | 14 +++++++------ 8 files changed, 57 insertions(+), 17 deletions(-) diff --git a/spring-boot-admin-client/pom.xml b/spring-boot-admin-client/pom.xml index f1d22012fa2..fb58c770b17 100644 --- a/spring-boot-admin-client/pom.xml +++ b/spring-boot-admin-client/pom.xml @@ -60,6 +60,11 @@ spring-webflux true + + io.projectreactor.netty + reactor-netty-http + true + org.projectlombok lombok diff --git a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java index 31170899fe9..91ca399d959 100644 --- a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java +++ b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java @@ -18,7 +18,10 @@ import java.util.Collections; import java.util.List; +import java.util.concurrent.TimeUnit; +import io.netty.channel.ChannelOption; +import io.netty.handler.timeout.ReadTimeoutHandler; import jakarta.servlet.ServletContext; import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAutoConfiguration; @@ -42,8 +45,10 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Lazy; import org.springframework.core.env.Environment; +import org.springframework.http.client.reactive.ReactorClientHttpConnector; import org.springframework.web.client.RestTemplate; import org.springframework.web.reactive.function.client.WebClient; +import reactor.netty.http.client.HttpClient; import de.codecentric.boot.admin.client.registration.ApplicationFactory; import de.codecentric.boot.admin.client.registration.ApplicationRegistrator; @@ -163,6 +168,10 @@ public RegistrationClient registrationClient(ClientProperties client, WebClient. if (client.getUsername() != null && client.getPassword() != null) { webClient = webClient.filter(basicAuthentication(client.getUsername(), client.getPassword())); } + HttpClient httpClient = HttpClient.create() + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, Long.valueOf(client.getConnectTimeout().toMillis()).intValue()) + .doOnConnected(conn -> conn.addHandlerLast(new ReadTimeoutHandler(client.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS))); + webClient.clientConnector(new ReactorClientHttpConnector(httpClient)); return new ReactiveRegistrationClient(webClient.build(), client.getReadTimeout()); } diff --git a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java index 757a6a5c926..91aa9a719d7 100644 --- a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java +++ b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java @@ -18,6 +18,7 @@ import java.util.Collections; import java.util.Map; +import java.util.Optional; import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; @@ -39,10 +40,12 @@ public BlockingRegistrationClient(RestTemplate restTemplate) { } @Override - public String register(String adminUrl, Application application) { + public Optional register(String adminUrl, Application application) { ResponseEntity> response = this.restTemplate.exchange(adminUrl, HttpMethod.POST, - new HttpEntity<>(application, this.createRequestHeaders()), RESPONSE_TYPE); - return response.getBody().get("id").toString(); + new HttpEntity<>(application, this.createRequestHeaders()), RESPONSE_TYPE); + return Optional.ofNullable(response.getBody()) + .map(body -> body.get("id")) + .map(Object::toString); } @Override diff --git a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistrator.java b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistrator.java index c39c8e97681..c64f4300aaf 100644 --- a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistrator.java +++ b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistrator.java @@ -16,6 +16,7 @@ package de.codecentric.boot.admin.client.registration; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.LongAdder; @@ -77,7 +78,12 @@ public boolean register() { protected boolean register(Application application, String adminUrl, boolean firstAttempt) { try { - String id = this.registrationClient.register(adminUrl, application); + Optional response = this.registrationClient.register(adminUrl, application); + if (response.isEmpty()) { + LOGGER.debug("Request was no successful"); + return false; + } + String id = response.get(); if (this.registeredId.compareAndSet(null, id)) { LOGGER.info("Application registered itself as {}", id); } diff --git a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/ReactiveRegistrationClient.java b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/ReactiveRegistrationClient.java index 139dd66aaac..15ea4bcddc5 100644 --- a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/ReactiveRegistrationClient.java +++ b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/ReactiveRegistrationClient.java @@ -19,14 +19,22 @@ import java.time.Duration; import java.util.Collections; import java.util.Map; +import java.util.Optional; +import io.netty.channel.ConnectTimeoutException; +import io.netty.handler.timeout.ReadTimeoutException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.web.reactive.function.client.WebClient; +import org.springframework.web.reactive.function.client.WebClientRequestException; public class ReactiveRegistrationClient implements RegistrationClient { + private static final Logger LOGGER = LoggerFactory.getLogger(ReactiveRegistrationClient.class); + private static final ParameterizedTypeReference> RESPONSE_TYPE = new ParameterizedTypeReference>() { }; @@ -40,16 +48,19 @@ public ReactiveRegistrationClient(WebClient webclient, Duration timeout) { } @Override - public String register(String adminUrl, Application application) { - Map response = this.webclient.post() + public Optional register(String adminUrl, Application application) { + return this.webclient.post() .uri(adminUrl) .headers(this::setRequestHeaders) .bodyValue(application) .retrieve() .bodyToMono(RESPONSE_TYPE) - .timeout(this.timeout) - .block(); - return response.get("id").toString(); + .onErrorMap(WebClientRequestException.class, Throwable::getCause) + .doOnError(ConnectTimeoutException.class, e -> LOGGER.debug("Connection timeout")) + .doOnError(ReadTimeoutException.class, e -> LOGGER.debug("Request time out")) + .map(t -> t.get("id")) + .map(Object::toString) + .blockOptional(); } @Override diff --git a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/RegistrationClient.java b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/RegistrationClient.java index 49f28aeb792..e0091e55793 100644 --- a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/RegistrationClient.java +++ b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/RegistrationClient.java @@ -16,9 +16,11 @@ package de.codecentric.boot.admin.client.registration; +import java.util.Optional; + public interface RegistrationClient { - String register(String adminUrl, Application self); + Optional register(String adminUrl, Application self); void deregister(String adminUrl, String id); diff --git a/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/AbstractRegistrationClientTest.java b/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/AbstractRegistrationClientTest.java index 3c94f3f9880..462b14f3344 100644 --- a/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/AbstractRegistrationClientTest.java +++ b/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/AbstractRegistrationClientTest.java @@ -24,6 +24,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.Optional; + import static com.github.tomakehurst.wiremock.client.WireMock.created; import static com.github.tomakehurst.wiremock.client.WireMock.delete; import static com.github.tomakehurst.wiremock.client.WireMock.deleteRequestedFor; @@ -72,7 +74,7 @@ public void register_should_return_id_when_successful() { this.wireMock.stubFor(post(urlEqualTo("/instances")).willReturn(response)); assertThat(this.registrationClient.register(this.wireMock.url("/instances"), this.application)) - .isEqualTo("-id-"); + .isEqualTo(Optional.of("-id-")); RequestPatternBuilder expectedRequest = postRequestedFor(urlEqualTo("/instances")) .withHeader("Accept", equalTo("application/json")) diff --git a/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java b/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java index cf2a0a6d6b2..b64b02e2216 100644 --- a/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java +++ b/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java @@ -19,6 +19,8 @@ import org.junit.jupiter.api.Test; import org.springframework.web.client.RestClientException; +import java.util.Optional; + import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -44,7 +46,7 @@ public void register_should_return_true_when_successful() { this.registrationClient, new String[] { "http://sba:8080/instances", "http://sba2:8080/instances" }, true); - when(this.registrationClient.register(any(), eq(this.application))).thenReturn("-id-"); + when(this.registrationClient.register(any(), eq(this.application))).thenReturn(Optional.of("-id-")); assertThat(registrator.register()).isTrue(); assertThat(registrator.getRegisteredId()).isEqualTo("-id-"); } @@ -70,7 +72,7 @@ public void register_should_try_next_on_error() { when(this.registrationClient.register("http://sba:8080/instances", this.application)) .thenThrow(new RestClientException("Error")); - when(this.registrationClient.register("http://sba2:8080/instances", this.application)).thenReturn("-id-"); + when(this.registrationClient.register("http://sba2:8080/instances", this.application)).thenReturn(Optional.of("-id-")); assertThat(registrator.register()).isTrue(); assertThat(registrator.getRegisteredId()).isEqualTo("-id-"); @@ -82,7 +84,7 @@ public void deregister_should_deregister_at_server() { this.registrationClient, new String[] { "http://sba:8080/instances", "http://sba2:8080/instances" }, true); - when(this.registrationClient.register(any(), eq(this.application))).thenReturn("-id-"); + when(this.registrationClient.register(any(), eq(this.application))).thenReturn(Optional.of("-id-")); registrator.register(); registrator.deregister(); @@ -108,7 +110,7 @@ public void deregister_should_try_next_on_error() { this.registrationClient, new String[] { "http://sba:8080/instances", "http://sba2:8080/instances" }, true); - when(this.registrationClient.register(any(), eq(this.application))).thenReturn("-id-"); + when(this.registrationClient.register(any(), eq(this.application))).thenReturn(Optional.of("-id-")); doThrow(new RestClientException("Error")).when(this.registrationClient) .deregister("http://sba:8080/instances", "-id-"); @@ -126,7 +128,7 @@ public void register_should_register_on_multiple_servers() { this.registrationClient, new String[] { "http://sba:8080/instances", "http://sba2:8080/instances" }, false); - when(this.registrationClient.register(any(), eq(this.application))).thenReturn("-id-"); + when(this.registrationClient.register(any(), eq(this.application))).thenReturn(Optional.of("-id-")); assertThat(registrator.register()).isTrue(); assertThat(registrator.getRegisteredId()).isEqualTo("-id-"); @@ -141,7 +143,7 @@ public void deregister_should_deregister_on_multiple_servers() { this.registrationClient, new String[] { "http://sba:8080/instances", "http://sba2:8080/instances" }, false); - when(this.registrationClient.register(any(), eq(this.application))).thenReturn("-id-"); + when(this.registrationClient.register(any(), eq(this.application))).thenReturn(Optional.of("-id-")); registrator.register(); registrator.deregister(); From cbde51768ba7cb121b5ecc23994689a34f29b536 Mon Sep 17 00:00:00 2001 From: denyskonakhevych Date: Wed, 25 Oct 2023 22:58:15 +0200 Subject: [PATCH 2/2] Bugfix/2755 IllegalStateException after read timeout --- .../config/SpringBootAdminClientAutoConfiguration.java | 6 ++++-- .../client/registration/BlockingRegistrationClient.java | 6 ++---- .../registration/DefaultApplicationRegistratorTest.java | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java index 91ca399d959..f4245343046 100644 --- a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java +++ b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java @@ -169,8 +169,10 @@ public RegistrationClient registrationClient(ClientProperties client, WebClient. webClient = webClient.filter(basicAuthentication(client.getUsername(), client.getPassword())); } HttpClient httpClient = HttpClient.create() - .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, Long.valueOf(client.getConnectTimeout().toMillis()).intValue()) - .doOnConnected(conn -> conn.addHandlerLast(new ReadTimeoutHandler(client.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS))); + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, + Long.valueOf(client.getConnectTimeout().toMillis()).intValue()) + .doOnConnected(conn -> conn + .addHandlerLast(new ReadTimeoutHandler(client.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS))); webClient.clientConnector(new ReactorClientHttpConnector(httpClient)); return new ReactiveRegistrationClient(webClient.build(), client.getReadTimeout()); } diff --git a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java index 91aa9a719d7..85fe7282d0a 100644 --- a/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java +++ b/spring-boot-admin-client/src/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java @@ -42,10 +42,8 @@ public BlockingRegistrationClient(RestTemplate restTemplate) { @Override public Optional register(String adminUrl, Application application) { ResponseEntity> response = this.restTemplate.exchange(adminUrl, HttpMethod.POST, - new HttpEntity<>(application, this.createRequestHeaders()), RESPONSE_TYPE); - return Optional.ofNullable(response.getBody()) - .map(body -> body.get("id")) - .map(Object::toString); + new HttpEntity<>(application, this.createRequestHeaders()), RESPONSE_TYPE); + return Optional.ofNullable(response.getBody()).map(body -> body.get("id")).map(Object::toString); } @Override diff --git a/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java b/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java index b64b02e2216..ce954201da6 100644 --- a/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java +++ b/spring-boot-admin-client/src/test/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java @@ -72,7 +72,8 @@ public void register_should_try_next_on_error() { when(this.registrationClient.register("http://sba:8080/instances", this.application)) .thenThrow(new RestClientException("Error")); - when(this.registrationClient.register("http://sba2:8080/instances", this.application)).thenReturn(Optional.of("-id-")); + when(this.registrationClient.register("http://sba2:8080/instances", this.application)) + .thenReturn(Optional.of("-id-")); assertThat(registrator.register()).isTrue(); assertThat(registrator.getRegisteredId()).isEqualTo("-id-");