Skip to content

Commit 98ed1df

Browse files
committed
Revise disconnected client error handling in Spring MVC
DisconnectedClientHelper identifies lost connection issues, but it's not always easy to know if it is the connection to the client or to another remote host. DisconnectedClientHelper does recognize and filter out common client exceptions, but there is a possibility for other similar custom exceptions. DefaultHandlerExceptionResolver now attempts to set the status to 500, which won't impact a client that has gone away, but it will set the status correct on the off chance that the exception is actually a server side issue. Closes gh-34481
1 parent 9b8a851 commit 98ed1df

2 files changed

Lines changed: 29 additions & 6 deletions

File tree

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,13 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
167167
protected static final Log pageNotFoundLogger = LogFactory.getLog(PAGE_NOT_FOUND_LOG_CATEGORY);
168168

169169

170+
private static final String DISCONNECTED_CLIENT_LOG_CATEGORY =
171+
"org.springframework.web.servlet.handler.DisconnectedClient";
172+
173+
private static final DisconnectedClientHelper disconnectedClientHelper =
174+
new DisconnectedClientHelper(DISCONNECTED_CLIENT_LOG_CATEGORY);
175+
176+
170177
/**
171178
* Sets the {@linkplain #setOrder(int) order} to {@link #LOWEST_PRECEDENCE}.
172179
*/
@@ -246,7 +253,7 @@ else if (ex instanceof AsyncRequestNotUsableException) {
246253
return handleAsyncRequestNotUsableException(
247254
(AsyncRequestNotUsableException) ex, request, response, handler);
248255
}
249-
else if (DisconnectedClientHelper.isClientDisconnectedException(ex)) {
256+
else if (disconnectedClientHelper.checkAndLogClientDisconnectedException(ex)) {
250257
return handleDisconnectedClientException(ex, request, response, handler);
251258
}
252259
}
@@ -506,11 +513,11 @@ protected ModelAndView handleAsyncRequestNotUsableException(AsyncRequestNotUsabl
506513
}
507514

508515
/**
509-
* Handle an Exception that indicates the client has gone away. This is
510-
* typically an {@link IOException} of a specific subtype or with a message
511-
* specific to the underlying Servlet container. Those are detected through
512-
* {@link DisconnectedClientHelper#isClientDisconnectedException(Throwable)}
513-
* <p>By default, do nothing since the response is not usable.
516+
* Handle an Exception that indicates the client has gone away as determined
517+
* via {@link DisconnectedClientHelper}.
518+
* <p>By default, as of 7.1 this method attempts to set the response status
519+
* to 500 in case the exception is due to a connection issue to a remote host,
520+
* as part of request handling, rather than to the client.
514521
* @param ex the {@code Exception} to be handled
515522
* @param request current HTTP request
516523
* @param response current HTTP response
@@ -522,6 +529,13 @@ protected ModelAndView handleAsyncRequestNotUsableException(AsyncRequestNotUsabl
522529
protected ModelAndView handleDisconnectedClientException(
523530
Exception ex, HttpServletRequest request, HttpServletResponse response, @Nullable Object handler) {
524531

532+
// Attempt to send 500 in case of onward (rather than client) connection issue
533+
try {
534+
sendServerError(ex, request, response);
535+
}
536+
catch (Exception ex2) {
537+
// ignore
538+
}
525539
return new ModelAndView();
526540
}
527541

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolverTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.web.servlet.mvc.support;
1818

19+
import java.io.IOException;
1920
import java.lang.reflect.Method;
2021
import java.util.Arrays;
2122
import java.util.Collections;
@@ -269,6 +270,14 @@ protected ModelAndView handleHttpRequestMethodNotSupported(
269270
assertThat(actual).isSameAs(expected);
270271
}
271272

273+
@Test // gh-34481
274+
void handleDisconnectedClientException() {
275+
Exception ex = new IOException("Broken pipe");
276+
ModelAndView mav = exceptionResolver.resolveException(request, response, null, ex);
277+
assertThat(mav).as("No ModelAndView returned").isNotNull();
278+
assertThat(mav.isEmpty()).as("No Empty ModelAndView returned").isTrue();
279+
assertThat(response.getStatus()).as("Should attempt to send server error").isEqualTo(500);
280+
}
272281

273282
@SuppressWarnings("unused")
274283
public void handle(String arg) {

0 commit comments

Comments
 (0)