Skip to content

Commit 5f601ce

Browse files
committed
Mark response errors from @ExceptionHandler as handled
Also fix a couple of related issues: - add AsyncRequestNotUsableException to the list of exceptions that imply response issues. - handle exceptions from @ExceptionHandler regardless of whether thrown immediately or via Publisher. Closes gh-32359
1 parent 0955f54 commit 5f601ce

File tree

5 files changed

+69
-16
lines changed

5 files changed

+69
-16
lines changed

spring-web/src/main/java/org/springframework/web/util/DisconnectedClientHelper.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -39,7 +39,8 @@ public class DisconnectedClientHelper {
3939
Set.of("broken pipe", "connection reset by peer");
4040

4141
private static final Set<String> EXCEPTION_TYPE_NAMES =
42-
Set.of("AbortedException", "ClientAbortException", "EOFException", "EofException");
42+
Set.of("AbortedException", "ClientAbortException",
43+
"EOFException", "EofException", "AsyncRequestNotUsableException");
4344

4445
private final Log logger;
4546

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -307,21 +307,34 @@ private Mono<HandlerResult> handleException(
307307
exceptions.toArray(arguments); // efficient arraycopy call in ArrayList
308308
arguments[arguments.length - 1] = handlerMethod;
309309

310-
return invocable.invoke(exchange, bindingContext, arguments);
310+
return invocable.invoke(exchange, bindingContext, arguments)
311+
.onErrorResume(invocationEx ->
312+
handleExceptionHandlerFailure(exchange, exception, invocationEx, exceptions, invocable));
311313
}
312314
catch (Throwable invocationEx) {
313-
if (!disconnectedClientHelper.checkAndLogClientDisconnectedException(invocationEx)) {
314-
// Any other than the original exception (or a cause) is unintended here,
315-
// probably an accident (e.g. failed assertion or the like).
316-
if (!exceptions.contains(invocationEx) && logger.isWarnEnabled()) {
317-
logger.warn(exchange.getLogPrefix() + "Failure in @ExceptionHandler " + invocable, invocationEx);
318-
}
319-
}
315+
return handleExceptionHandlerFailure(exchange, exception, invocationEx, exceptions, invocable);
320316
}
321317
}
322318
return Mono.error(exception);
323319
}
324320

321+
private static Mono<HandlerResult> handleExceptionHandlerFailure(
322+
ServerWebExchange exchange, Throwable exception, Throwable invocationEx,
323+
ArrayList<Throwable> exceptions, InvocableHandlerMethod invocable) {
324+
325+
if (disconnectedClientHelper.checkAndLogClientDisconnectedException(invocationEx)) {
326+
return Mono.empty();
327+
}
328+
329+
// Any other than the original exception (or a cause) is unintended here,
330+
// probably an accident (e.g. failed assertion or the like).
331+
if (!exceptions.contains(invocationEx) && logger.isWarnEnabled()) {
332+
logger.warn(exchange.getLogPrefix() + "Failure in @ExceptionHandler " + invocable, invocationEx);
333+
}
334+
335+
return Mono.error(exception);
336+
}
337+
325338
@Override
326339
public Mono<HandlerResult> handleError(ServerWebExchange exchange, Throwable ex) {
327340
return handleException(exchange, ex, null, null);

spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@
3838
import org.springframework.http.codec.ServerCodecConfigurer;
3939
import org.springframework.stereotype.Controller;
4040
import org.springframework.web.bind.annotation.ControllerAdvice;
41+
import org.springframework.web.bind.annotation.ExceptionHandler;
4142
import org.springframework.web.bind.annotation.RequestBody;
4243
import org.springframework.web.bind.annotation.RequestMapping;
4344
import org.springframework.web.bind.annotation.ResponseBody;
45+
import org.springframework.web.context.request.async.AsyncRequestNotUsableException;
4446
import org.springframework.web.reactive.accept.HeaderContentTypeResolver;
4547
import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping;
4648
import org.springframework.web.reactive.resource.ResourceWebHandler;
@@ -195,6 +197,14 @@ void webExceptionHandler() {
195197
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
196198
}
197199

200+
@Test
201+
void asyncRequestNotUsableFromExceptionHandler() {
202+
ServerWebExchange exchange = MockServerWebExchange.from(
203+
MockServerHttpRequest.post("/request-not-usable-on-exception-handling"));
204+
205+
StepVerifier.create(this.dispatcherHandler.handle(exchange)).verifyComplete();
206+
}
207+
198208

199209
@Configuration
200210
@SuppressWarnings({"unused", "WeakerAccess"})
@@ -249,6 +259,16 @@ public Foo unknownReturnType() {
249259
public Publisher<String> requestBody(@RequestBody Publisher<String> body) {
250260
return Mono.from(body).map(s -> "hello " + s);
251261
}
262+
263+
@RequestMapping("/request-not-usable-on-exception-handling")
264+
public void handle() throws Exception {
265+
throw new IllegalAccessException();
266+
}
267+
268+
@ExceptionHandler
269+
public void handleException(IllegalAccessException ex) throws AsyncRequestNotUsableException {
270+
throw new AsyncRequestNotUsableException("Simulated response failure");
271+
}
252272
}
253273

254274

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -432,12 +432,13 @@ protected ModelAndView doResolveHandlerMethodException(HttpServletRequest reques
432432
exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, arguments);
433433
}
434434
catch (Throwable invocationEx) {
435-
if (!disconnectedClientHelper.checkAndLogClientDisconnectedException(invocationEx)) {
436-
// Any other than the original exception (or a cause) is unintended here,
437-
// probably an accident (e.g. failed assertion or the like).
438-
if (!exceptions.contains(invocationEx) && logger.isWarnEnabled()) {
439-
logger.warn("Failure in @ExceptionHandler " + exceptionHandlerMethod, invocationEx);
440-
}
435+
if (disconnectedClientHelper.checkAndLogClientDisconnectedException(invocationEx)) {
436+
return new ModelAndView();
437+
}
438+
// Any other than the original exception (or a cause) is unintended here,
439+
// probably an accident (e.g. failed assertion or the like).
440+
if (!exceptions.contains(invocationEx) && logger.isWarnEnabled()) {
441+
logger.warn("Failure in @ExceptionHandler " + exceptionHandlerMethod, invocationEx);
441442
}
442443
// Continue with default processing of the original exception...
443444
return null;

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Locale;
2525

2626
import jakarta.servlet.ServletException;
27+
import jakarta.servlet.http.HttpServletResponse;
2728
import org.junit.jupiter.api.BeforeAll;
2829
import org.junit.jupiter.api.BeforeEach;
2930
import org.junit.jupiter.api.Test;
@@ -50,6 +51,7 @@
5051
import org.springframework.web.bind.annotation.ResponseBody;
5152
import org.springframework.web.bind.annotation.ResponseStatus;
5253
import org.springframework.web.bind.annotation.RestControllerAdvice;
54+
import org.springframework.web.context.request.async.AsyncRequestNotUsableException;
5355
import org.springframework.web.context.support.WebApplicationObjectSupport;
5456
import org.springframework.web.method.HandlerMethod;
5557
import org.springframework.web.method.annotation.ModelMethodProcessor;
@@ -65,6 +67,8 @@
6567

6668
import static org.assertj.core.api.Assertions.assertThat;
6769
import static org.assertj.core.api.InstanceOfAssertFactories.LIST;
70+
import static org.mockito.BDDMockito.given;
71+
import static org.mockito.Mockito.mock;
6872

6973
/**
7074
* Test fixture with {@link ExceptionHandlerExceptionResolver}.
@@ -401,6 +405,20 @@ void resolveExceptionViaMappedHandlerPredicate() throws Exception {
401405
assertExceptionHandledAsBody(mav, "DefaultTestExceptionResolver: IllegalStateException");
402406
}
403407

408+
@Test
409+
void resolveExceptionAsyncRequestNotUsable() throws Exception {
410+
HttpServletResponse response = mock();
411+
given(response.getOutputStream()).willThrow(new AsyncRequestNotUsableException("Simulated I/O failure"));
412+
413+
IllegalArgumentException ex = new IllegalArgumentException();
414+
HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle");
415+
this.resolver.afterPropertiesSet();
416+
ModelAndView mav = this.resolver.resolveException(this.request, response, handlerMethod, ex);
417+
418+
assertThat(mav).isNotNull();
419+
assertThat(mav.isEmpty()).isTrue();
420+
}
421+
404422

405423
private void assertMethodProcessorCount(int resolverCount, int handlerCount) {
406424
assertThat(this.resolver.getArgumentResolvers().getResolvers()).hasSize(resolverCount);

0 commit comments

Comments
 (0)