Spring/spring-boot
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not handle WebFlux errors if response is committed

Browse Source 
If the WebFlux handler commits the response but still sends an error
signal in the reactive pipeline, Spring Boot error handling should not
try to handle that error: once committed, it is impossible to change the
response status or the response headers. Writing to the body might also
lead to invalid responses.

This commit skips error handling if the response is committed and
delegates to Spring Framework's `HttpWebHandlerAdapter` which will log
the error.

Fixes gh-11168
This commit is contained in:
Brian Clozel
2017-11-29 11:39:28 +01:00
parent 6be30045ec
commit de208cd372
2 changed files with 29 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java
View File

@@ -220,12 +220,17 @@ public abstract class AbstractErrorWebExceptionHandler
@Override
public Mono<Void> handle(ServerWebExchange exchange, Throwable throwable) {
this.errorAttributes.storeErrorInformation(throwable, exchange);
ServerRequest request = ServerRequest.create(exchange, this.messageReaders);
return getRoutingFunction(this.errorAttributes).route(request)
.switchIfEmpty(Mono.error(throwable))
.flatMap((handler) -> handler.handle(request))
.flatMap((response) -> write(exchange, response));
if (!exchange.getResponse().isCommitted()) {
this.errorAttributes.storeErrorInformation(throwable, exchange);
ServerRequest request = ServerRequest.create(exchange, this.messageReaders);
return getRoutingFunction(this.errorAttributes).route(request)
.switchIfEmpty(Mono.error(throwable))
.flatMap((handler) -> handler.handle(request))
.flatMap((response) -> write(exchange, response));
}
else {
return Mono.error(throwable);
}
}
private Mono<? extends Void> write(ServerWebExchange exchange,

18
spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerIntegrationTest.java
View File

@@ -52,6 +52,7 @@ import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.server.ResponseStatusException;
import org.springframework.web.server.ServerWebExchange;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.Matchers.allOf;
@@ -188,6 +189,16 @@ public class DefaultErrorWebExceptionHandlerIntegrationTest {
containsString("IllegalStateException")));
}
@Test
public void responseCommitted() throws Exception {
load();
this.webTestClient.get().uri("/commit").exchange()
.expectStatus().isEqualTo(HttpStatus.OK)
.expectBody().isEmpty();
this.output.expect(not(containsString("java.lang.UnsupportedOperationException")));
this.output.expect(containsString("java.lang.IllegalStateException: already committed!"));
}
private void load(String... arguments) {
List<String> args = new ArrayList<>();
args.add("--server.port=0");
@@ -235,6 +246,13 @@ public class DefaultErrorWebExceptionHandlerIntegrationTest {
return Mono.error(new ResponseStatusException(HttpStatus.BAD_REQUEST));
}
@GetMapping("/commit")
public Mono<Void> commit(ServerWebExchange exchange) {
return exchange
.getResponse().writeWith(Mono.empty())
.then(Mono.error(new IllegalStateException("already committed!")));
}
@PostMapping(path = "/bind", produces = "application/json")
@ResponseBody
public String bodyValidation(@Valid @RequestBody DummyBody body) {