Skip to content

Commit 73fadf6

Browse files
Fuuderikpetzold
andauthored
fix handling GatewayTimeout + fix flushing servlet request in async thread (#2018)
* fix handling GatewayTimeout + fix flushing servlet request in async thread * Update spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/web/AbstractInstancesProxyControllerIntegrationTest.java Co-authored-by: Erik Petzold <[email protected]>
1 parent fff116d commit 73fadf6

File tree

3 files changed

+30
-16
lines changed

3 files changed

+30
-16
lines changed

spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/web/InstanceWebProxy.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,11 @@ private <V> Mono<V> forward(Instance instance, ForwardRequest forwardRequest,
104104
log.trace("No Endpoint found for Proxy-Request for instance {} with URL '{}'", instance.getId(),
105105
forwardRequest.getUri());
106106
return responseHandler.apply(ClientResponse.create(HttpStatus.NOT_FOUND, this.strategies).build());
107-
}).onErrorResume(WebClientRequestException.class, (ex) -> {
108-
Throwable cause = ex.getCause();
107+
}).onErrorResume((ex) -> {
108+
Throwable cause = ex;
109+
if (ex instanceof WebClientRequestException) {
110+
cause = ex.getCause();
111+
}
109112
if (cause instanceof ReadTimeoutException || cause instanceof TimeoutException) {
110113
log.trace("Timeout for Proxy-Request for instance {} with URL '{}'", instance.getId(),
111114
forwardRequest.getUri());

spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/web/servlet/InstancesProxyController.java

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.net.URI;
2222
import java.util.Set;
2323

24+
import javax.servlet.AsyncContext;
2425
import javax.servlet.http.HttpServletRequest;
2526
import javax.servlet.http.HttpServletResponse;
2627

@@ -84,24 +85,30 @@ public InstancesProxyController(String adminContextPath, Set<String> ignoredHead
8485
@ResponseBody
8586
@RequestMapping(path = INSTANCE_MAPPED_PATH, method = { RequestMethod.GET, RequestMethod.HEAD, RequestMethod.POST,
8687
RequestMethod.PUT, RequestMethod.PATCH, RequestMethod.DELETE, RequestMethod.OPTIONS })
87-
public void endpointProxy(@PathVariable("instanceId") String instanceId, HttpServletRequest servletRequest,
88-
HttpServletResponse servletResponse) {
89-
ServletServerHttpRequest request = new ServletServerHttpRequest(servletRequest);
90-
Flux<DataBuffer> requestBody = DataBufferUtils.readInputStream(request::getBody, this.bufferFactory, 4096);
91-
InstanceWebProxy.ForwardRequest fwdRequest = createForwardRequest(request, requestBody,
88+
public void instanceProxy(@PathVariable("instanceId") String instanceId, HttpServletRequest servletRequest) {
89+
// start async because we will commit from different thread.
90+
// otherwise incorrect thread local objects (session and security context) will be stored.
91+
// check for example org.springframework.security.web.context.HttpSessionSecurityContextRepository.SaveToSessionRequestWrapper.startAsync()
92+
AsyncContext asyncContext = servletRequest.startAsync();
93+
asyncContext.setTimeout(-1); // no timeout because instanceWebProxy will handle it for us
94+
try {
95+
ServletServerHttpRequest request = new ServletServerHttpRequest((HttpServletRequest) asyncContext.getRequest());
96+
Flux<DataBuffer> requestBody = DataBufferUtils.readInputStream(request::getBody, this.bufferFactory, 4096);
97+
InstanceWebProxy.ForwardRequest fwdRequest = createForwardRequest(request, requestBody,
9298
this.adminContextPath + INSTANCE_MAPPED_PATH);
9399

94-
this.instanceWebProxy
100+
101+
this.instanceWebProxy
95102
.forward(this.registry.getInstance(InstanceId.of(instanceId)), fwdRequest, (clientResponse) -> {
96-
ServerHttpResponse response = new ServletServerHttpResponse(servletResponse);
103+
ServerHttpResponse response = new ServletServerHttpResponse((HttpServletResponse) asyncContext.getResponse());
97104
response.setStatusCode(clientResponse.statusCode());
98105
response.getHeaders()
99-
.addAll(this.httpHeadersFilter.filterHeaders(clientResponse.headers().asHttpHeaders()));
106+
.addAll(this.httpHeadersFilter.filterHeaders(clientResponse.headers().asHttpHeaders()));
100107
try {
101108
OutputStream responseBody = response.getBody();
102109
response.flush();
103110
return clientResponse.body(BodyExtractors.toDataBuffers()).window(1)
104-
.concatMap((body) -> writeAndFlush(body, responseBody)).then();
111+
.concatMap((body) -> writeAndFlush(body, responseBody)).then();
105112
}
106113
catch (IOException ex) {
107114
return Mono.error(ex);
@@ -111,6 +118,10 @@ public void endpointProxy(@PathVariable("instanceId") String instanceId, HttpSer
111118
// before any async dispatch otherwise the FrameworkServlet will add wrong
112119
// Allow header for OPTIONS request
113120
.block();
121+
}
122+
finally {
123+
asyncContext.complete();
124+
}
114125
}
115126

116127
@ResponseBody

spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/web/AbstractInstancesProxyControllerIntegrationTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public abstract class AbstractInstancesProxyControllerIntegrationTest {
7272

7373
@BeforeAll
7474
public static void setUp() {
75-
StepVerifier.setDefaultTimeout(Duration.ofSeconds(60));
75+
StepVerifier.setDefaultTimeout(Duration.ofSeconds(600));
7676
}
7777

7878
@AfterAll
@@ -124,10 +124,10 @@ public void should_return_status_502() {
124124

125125
@Test
126126
public void should_return_status_504() {
127-
// 502 on invalid response
128-
this.client.get().uri("/instances/{instanceId}/actuator/invalid", this.instanceId)
129-
.accept(new MediaType(ApiVersion.LATEST.getProducedMimeType())).exchange().expectStatus()
130-
.isEqualTo(HttpStatus.BAD_GATEWAY);
127+
// 504 on read timeout
128+
this.client.get().uri("/instances/{instanceId}/actuator/timeout", this.instanceId)
129+
.accept(new MediaType(ApiVersion.LATEST.getProducedMimeType()))
130+
.exchange().expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT);
131131
}
132132

133133
@Test

0 commit comments

Comments
 (0)