Skip to content

Commit dd5fe68

Browse files
committed
Dispatch again after disconnected client error
Dispatching was prevented for disconnected client errors after recent reports like #32042 when running on Tomcat, and the async request was completed from the onError notification. This has the side effect of not allowing exception resolvers to take final action even if the response is not writeable. After all updates for this issue, it appears the dispatch no longer causes issues. Tomcat actually does not do the dispatch, but it doesn't seem to cause any issues, and on other servers like Jetty where the dispatch works, applications can have a chance to handle the exception. This change removes the disconnected client checks and allows dispatching again. After the change DisconnectedClientHelper is no longer needed in the 5.3.x branch. See gh-32342
1 parent 2aca714 commit dd5fe68

File tree

5 files changed

+28
-125
lines changed

5 files changed

+28
-125
lines changed

spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import org.springframework.lang.Nullable;
3535
import org.springframework.util.Assert;
3636
import org.springframework.web.context.request.ServletWebRequest;
37-
import org.springframework.web.util.DisconnectedClientHelper;
3837

3938
/**
4039
* A Servlet implementation of {@link AsyncWebRequest}.
@@ -184,13 +183,6 @@ public void onError(AsyncEvent event) throws IOException {
184183
transitionToErrorState();
185184
Throwable ex = event.getThrowable();
186185
this.exceptionHandlers.forEach(consumer -> consumer.accept(ex));
187-
188-
// We skip ASYNC dispatches for "disconnected client" errors,
189-
// but can only complete from a Servlet container thread
190-
191-
if (DisconnectedClientHelper.isClientDisconnectedException(ex) && this.state != State.COMPLETED) {
192-
this.asyncContext.complete();
193-
}
194186
}
195187
finally {
196188
this.stateLock.unlock();

spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.springframework.util.Assert;
3737
import org.springframework.web.context.request.RequestAttributes;
3838
import org.springframework.web.context.request.async.DeferredResult.DeferredResultHandler;
39-
import org.springframework.web.util.DisconnectedClientHelper;
4039

4140
/**
4241
* The central class for managing asynchronous request processing, mainly intended
@@ -70,16 +69,6 @@ public final class WebAsyncManager {
7069

7170
private static final Log logger = LogFactory.getLog(WebAsyncManager.class);
7271

73-
/**
74-
* Log category to use for network failure after a client has gone away.
75-
* @see DisconnectedClientHelper
76-
*/
77-
private static final String DISCONNECTED_CLIENT_LOG_CATEGORY =
78-
"org.springframework.web.server.DisconnectedClient";
79-
80-
private static final DisconnectedClientHelper disconnectedClientHelper =
81-
new DisconnectedClientHelper(DISCONNECTED_CLIENT_LOG_CATEGORY);
82-
8372
private static final CallableProcessingInterceptor timeoutCallableInterceptor =
8473
new TimeoutCallableProcessingInterceptor();
8574

@@ -418,12 +407,6 @@ private void setConcurrentResultAndDispatch(@Nullable Object result) {
418407
logger.debug("Async result set to: " + result + " for " + formatUri(this.asyncWebRequest));
419408
}
420409

421-
if (result instanceof Exception) {
422-
if (disconnectedClientHelper.checkAndLogClientDisconnectedException((Exception) result)) {
423-
return;
424-
}
425-
}
426-
427410
if (this.asyncWebRequest.isAsyncComplete()) {
428411
if (logger.isDebugEnabled()) {
429412
logger.debug("Async request already completed for " + formatUri(this.asyncWebRequest));

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

Lines changed: 0 additions & 98 deletions
This file was deleted.

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 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.
@@ -46,6 +46,7 @@
4646
import org.springframework.web.bind.annotation.ModelAttribute;
4747
import org.springframework.web.bind.annotation.RequestBody;
4848
import org.springframework.web.bind.annotation.RequestPart;
49+
import org.springframework.web.context.request.async.AsyncRequestNotUsableException;
4950
import org.springframework.web.context.request.async.AsyncRequestTimeoutException;
5051
import org.springframework.web.multipart.MultipartFile;
5152
import org.springframework.web.multipart.support.MissingServletRequestPartException;
@@ -228,6 +229,10 @@ else if (ex instanceof AsyncRequestTimeoutException) {
228229
return handleAsyncRequestTimeoutException(
229230
(AsyncRequestTimeoutException) ex, request, response, handler);
230231
}
232+
else if (ex instanceof AsyncRequestNotUsableException) {
233+
return handleAsyncRequestNotUsableException(
234+
(AsyncRequestNotUsableException) ex, request, response, handler);
235+
}
231236
}
232237
catch (Exception handlerEx) {
233238
if (logger.isWarnEnabled()) {
@@ -541,6 +546,24 @@ protected ModelAndView handleAsyncRequestTimeoutException(AsyncRequestTimeoutExc
541546
return new ModelAndView();
542547
}
543548

549+
/**
550+
* Handle the case of an I/O failure from the ServletOutputStream.
551+
* <p>By default, do nothing since the response is not usable.
552+
* @param ex the {@link AsyncRequestTimeoutException} to be handled
553+
* @param request current HTTP request
554+
* @param response current HTTP response
555+
* @param handler the executed handler, or {@code null} if none chosen
556+
* at the time of the exception (for example, if multipart resolution failed)
557+
* @return an empty ModelAndView indicating the exception was handled
558+
* @throws IOException potentially thrown from {@link HttpServletResponse#sendError}
559+
* @since 5.3.33
560+
*/
561+
protected ModelAndView handleAsyncRequestNotUsableException(AsyncRequestNotUsableException ex,
562+
HttpServletRequest request, HttpServletResponse response, @Nullable Object handler) {
563+
564+
return new ModelAndView();
565+
}
566+
544567
/**
545568
* Invoked to send a server error. Sets the status to 500 and also sets the
546569
* request attribute "javax.servlet.error.exception" to the Exception.

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 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.
@@ -92,6 +92,9 @@ public void supportsAllDefaultHandlerExceptionResolverExceptionTypes() throws Ex
9292
Class<?>[] paramTypes = method.getParameterTypes();
9393
if (method.getName().startsWith("handle") && (paramTypes.length == 4)) {
9494
String name = paramTypes[0].getSimpleName();
95+
if (name.equals("AsyncRequestNotUsableException")) {
96+
continue;
97+
}
9598
assertThat(exceptionTypes.contains(paramTypes[0])).as("@ExceptionHandler is missing " + name).isTrue();
9699
}
97100
}

0 commit comments

Comments
 (0)