Browse Source

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
pull/32754/head
rstoyanchev 2 years ago
parent
commit
dd5fe68522
  1. 8
      spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java
  2. 17
      spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java
  3. 98
      spring-web/src/main/java/org/springframework/web/util/DisconnectedClientHelper.java
  4. 25
      spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java
  5. 5
      spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java

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

@ -34,7 +34,6 @@ import javax.servlet.http.HttpServletResponseWrapper; @@ -34,7 +34,6 @@ import javax.servlet.http.HttpServletResponseWrapper;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.util.DisconnectedClientHelper;
/**
* A Servlet implementation of {@link AsyncWebRequest}.
@ -184,13 +183,6 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements @@ -184,13 +183,6 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements
transitionToErrorState();
Throwable ex = event.getThrowable();
this.exceptionHandlers.forEach(consumer -> consumer.accept(ex));
// We skip ASYNC dispatches for "disconnected client" errors,
// but can only complete from a Servlet container thread
if (DisconnectedClientHelper.isClientDisconnectedException(ex) && this.state != State.COMPLETED) {
this.asyncContext.complete();
}
}
finally {
this.stateLock.unlock();

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

@ -36,7 +36,6 @@ import org.springframework.lang.Nullable; @@ -36,7 +36,6 @@ import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.web.context.request.RequestAttributes;
import org.springframework.web.context.request.async.DeferredResult.DeferredResultHandler;
import org.springframework.web.util.DisconnectedClientHelper;
/**
* The central class for managing asynchronous request processing, mainly intended
@ -70,16 +69,6 @@ public final class WebAsyncManager { @@ -70,16 +69,6 @@ public final class WebAsyncManager {
private static final Log logger = LogFactory.getLog(WebAsyncManager.class);
/**
* Log category to use for network failure after a client has gone away.
* @see DisconnectedClientHelper
*/
private static final String DISCONNECTED_CLIENT_LOG_CATEGORY =
"org.springframework.web.server.DisconnectedClient";
private static final DisconnectedClientHelper disconnectedClientHelper =
new DisconnectedClientHelper(DISCONNECTED_CLIENT_LOG_CATEGORY);
private static final CallableProcessingInterceptor timeoutCallableInterceptor =
new TimeoutCallableProcessingInterceptor();
@ -418,12 +407,6 @@ public final class WebAsyncManager { @@ -418,12 +407,6 @@ public final class WebAsyncManager {
logger.debug("Async result set to: " + result + " for " + formatUri(this.asyncWebRequest));
}
if (result instanceof Exception) {
if (disconnectedClientHelper.checkAndLogClientDisconnectedException((Exception) result)) {
return;
}
}
if (this.asyncWebRequest.isAsyncComplete()) {
if (logger.isDebugEnabled()) {
logger.debug("Async request already completed for " + formatUri(this.asyncWebRequest));

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

@ -1,98 +0,0 @@ @@ -1,98 +0,0 @@
/*
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.web.util;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.core.NestedExceptionUtils;
import org.springframework.util.Assert;
/**
* Utility methods to assist with identifying and logging exceptions that indicate
* the client has gone away. Such exceptions fill logs with unnecessary stack
* traces. The utility methods help to log a single line message at DEBUG level,
* and a full stacktrace at TRACE level.
*
* @author Rossen Stoyanchev
* @since 5.3.33
*/
public class DisconnectedClientHelper {
private static final Set<String> EXCEPTION_PHRASES =
new HashSet<>(Arrays.asList("broken pipe", "connection reset by peer"));
private static final Set<String> EXCEPTION_TYPE_NAMES =
new HashSet<>(Arrays.asList("AbortedException", "ClientAbortException",
"EOFException", "EofException", "AsyncRequestNotUsableException"));
private final Log logger;
public DisconnectedClientHelper(String logCategory) {
Assert.notNull(logCategory, "'logCategory' is required");
this.logger = LogFactory.getLog(logCategory);
}
/**
* Check via {@link #isClientDisconnectedException} if the exception
* indicates the remote client disconnected, and if so log a single line
* message when DEBUG is on, and a full stacktrace when TRACE is on for
* the configured logger.
*/
public boolean checkAndLogClientDisconnectedException(Throwable ex) {
if (isClientDisconnectedException(ex)) {
if (logger.isTraceEnabled()) {
logger.trace("Looks like the client has gone away", ex);
}
else if (logger.isDebugEnabled()) {
logger.debug("Looks like the client has gone away: " + ex +
" (For a full stack trace, set the log category '" + logger + "' to TRACE level.)");
}
return true;
}
return false;
}
/**
* Whether the given exception indicates the client has gone away.
* Known cases covered:
* <ul>
* <li>ClientAbortException or EOFException for Tomcat
* <li>EofException for Jetty
* <li>IOException "Broken pipe" or "connection reset by peer"
* </ul>
*/
public static boolean isClientDisconnectedException(Throwable ex) {
String message = NestedExceptionUtils.getMostSpecificCause(ex).getMessage();
if (message != null) {
String text = message.toLowerCase();
for (String phrase : EXCEPTION_PHRASES) {
if (text.contains(phrase)) {
return true;
}
}
}
return EXCEPTION_TYPE_NAMES.contains(ex.getClass().getSimpleName());
}
}

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -46,6 +46,7 @@ import org.springframework.web.bind.ServletRequestBindingException; @@ -46,6 +46,7 @@ import org.springframework.web.bind.ServletRequestBindingException;
import org.springframework.web.bind.annotation.ModelAttribute;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestPart;
import org.springframework.web.context.request.async.AsyncRequestNotUsableException;
import org.springframework.web.context.request.async.AsyncRequestTimeoutException;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.support.MissingServletRequestPartException;
@ -228,6 +229,10 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes @@ -228,6 +229,10 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
return handleAsyncRequestTimeoutException(
(AsyncRequestTimeoutException) ex, request, response, handler);
}
else if (ex instanceof AsyncRequestNotUsableException) {
return handleAsyncRequestNotUsableException(
(AsyncRequestNotUsableException) ex, request, response, handler);
}
}
catch (Exception handlerEx) {
if (logger.isWarnEnabled()) {
@ -541,6 +546,24 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes @@ -541,6 +546,24 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes
return new ModelAndView();
}
/**
* Handle the case of an I/O failure from the ServletOutputStream.
* <p>By default, do nothing since the response is not usable.
* @param ex the {@link AsyncRequestTimeoutException} to be handled
* @param request current HTTP request
* @param response current HTTP response
* @param handler the executed handler, or {@code null} if none chosen
* at the time of the exception (for example, if multipart resolution failed)
* @return an empty ModelAndView indicating the exception was handled
* @throws IOException potentially thrown from {@link HttpServletResponse#sendError}
* @since 5.3.33
*/
protected ModelAndView handleAsyncRequestNotUsableException(AsyncRequestNotUsableException ex,
HttpServletRequest request, HttpServletResponse response, @Nullable Object handler) {
return new ModelAndView();
}
/**
* Invoked to send a server error. Sets the status to 500 and also sets the
* request attribute "javax.servlet.error.exception" to the Exception.

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -92,6 +92,9 @@ public class ResponseEntityExceptionHandlerTests { @@ -92,6 +92,9 @@ public class ResponseEntityExceptionHandlerTests {
Class<?>[] paramTypes = method.getParameterTypes();
if (method.getName().startsWith("handle") && (paramTypes.length == 4)) {
String name = paramTypes[0].getSimpleName();
if (name.equals("AsyncRequestNotUsableException")) {
continue;
}
assertThat(exceptionTypes.contains(paramTypes[0])).as("@ExceptionHandler is missing " + name).isTrue();
}
}

Loading…
Cancel
Save