Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bugfix/2755 IllegalStateException after read timeout #2845

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Bugfix/2755 IllegalStateException after read timeout #2845

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
196db71
Bugfix/2755 IllegalStateException after read timeout
denyskonakhevych Oct 24, 2023
6ed3a9b
Merge branch 'master' into bugfix/2755-illegal_state_exception_after_…
denyskonakhevych Oct 24, 2023
28b57ec
Merge branch 'master' into bugfix/2755-illegal_state_exception_after_…
denyskonakhevych Oct 25, 2023
5d77c64
Merge branch 'master' into bugfix/2755-illegal_state_exception_after_…
SteKoe Oct 25, 2023
cbde517
Bugfix/2755 IllegalStateException after read timeout
denyskonakhevych Oct 25, 2023
ac8d750
Merge branch 'master' into bugfix/2755-illegal_state_exception_after_…
denyskonakhevych Oct 26, 2023
7524f52
Merge branch 'master' into bugfix/2755-illegal_state_exception_after_…
denyskonakhevych Oct 27, 2023
e327f48
Merge branch 'master' into bugfix/2755-illegal_state_exception_after_…
erikpetzold Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions spring-boot-admin-client/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@
<artifactId>spring-webflux</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>io.projectreactor.netty</groupId>
<artifactId>reactor-netty-http</artifactId>
<optional>true</optional>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to define the dependency here correctly.

webflux is optional and you only need the reactor-netty-http in webflux case, so this is optional, too.

But when webflux is used, this is now required, right? So the users would need to include this dependency additionally? How do they know this?

</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
Expand Down
11 changes: 11 additions & 0 deletions .../java/de/codecentric/boot/admin/client/config/SpringBootAdminClientAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -163,6 +168,12 @@ 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());
}

Expand Down
5 changes: 3 additions & 2 deletions ...c/main/java/de/codecentric/boot/admin/client/registration/BlockingRegistrationClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,10 +40,10 @@ public BlockingRegistrationClient(RestTemplate restTemplate) {
}

@Override
public String register(String adminUrl, Application application) {
public Optional<String> register(String adminUrl, Application application) {
ResponseEntity<Map<String, Object>> response = this.restTemplate.exchange(adminUrl, HttpMethod.POST,
new HttpEntity<>(application, this.createRequestHeaders()), RESPONSE_TYPE);
return response.getBody().get("id").toString();
return Optional.ofNullable(response.getBody()).map(body -> body.get("id")).map(Object::toString);
}

@Override
Expand Down
8 changes: 7 additions & 1 deletion ...ain/java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistrator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> 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);
}
Expand Down
21 changes: 16 additions & 5 deletions ...c/main/java/de/codecentric/boot/admin/client/registration/ReactiveRegistrationClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Map<String, Object>> RESPONSE_TYPE = new ParameterizedTypeReference<Map<String, Object>>() {
};

Expand All @@ -40,16 +48,19 @@ public ReactiveRegistrationClient(WebClient webclient, Duration timeout) {
}

@Override
public String register(String adminUrl, Application application) {
Map<String, Object> response = this.webclient.post()
public Optional<String> 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
Expand Down
4 changes: 3 additions & 1 deletion ...lient/src/main/java/de/codecentric/boot/admin/client/registration/RegistrationClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> register(String adminUrl, Application self);

void deregister(String adminUrl, String id);

Expand Down
4 changes: 3 additions & 1 deletion ...st/java/de/codecentric/boot/admin/client/registration/AbstractRegistrationClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"))
Expand Down
15 changes: 9 additions & 6 deletions ...java/de/codecentric/boot/admin/client/registration/DefaultApplicationRegistratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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-");
}
Expand All @@ -70,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("-id-");
when(this.registrationClient.register("http://sba2:8080/instances", this.application))
.thenReturn(Optional.of("-id-"));

assertThat(registrator.register()).isTrue();
assertThat(registrator.getRegisteredId()).isEqualTo("-id-");
Expand All @@ -82,7 +85,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();
Expand All @@ -108,7 +111,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-");

Expand All @@ -126,7 +129,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-");
Expand All @@ -141,7 +144,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();
Expand Down