diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java index f8ac4cccac..cc593df745 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java @@ -16,7 +16,6 @@ package org.springframework.web.server.adapter; -import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import io.micrometer.observation.Observation; @@ -29,7 +28,6 @@ import reactor.core.publisher.Mono; import reactor.util.context.Context; import org.springframework.context.ApplicationContext; -import org.springframework.core.NestedExceptionUtils; import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -54,6 +52,7 @@ import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver; import org.springframework.web.server.i18n.LocaleContextResolver; import org.springframework.web.server.session.DefaultWebSessionManager; import org.springframework.web.server.session.WebSessionManager; +import org.springframework.web.util.DisconnectedClientHelper; /** * Default adapter of {@link WebHandler} to the {@link HttpHandler} contract. @@ -69,20 +68,14 @@ import org.springframework.web.server.session.WebSessionManager; public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHandler { /** - * Dedicated log category for disconnected client exceptions. - *

Servlet containers don't expose a client disconnected callback; see - * eclipse-ee4j/servlet-api#44. - *

To avoid filling logs with unnecessary stack traces, we make an - * effort to identify such network failures on a per-server basis, and then - * log under a separate log category a simple one-line message at DEBUG level - * or a full stack trace only at TRACE level. + * 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"; - // Similar declaration exists in AbstractSockJsSession. - private static final Set DISCONNECTED_CLIENT_EXCEPTIONS = - Set.of("AbortedException", "ClientAbortException", "EOFException", "EofException"); + private static final DisconnectedClientHelper disconnectedClientHelper = + new DisconnectedClientHelper(DISCONNECTED_CLIENT_LOG_CATEGORY); private static final ServerRequestObservationConvention DEFAULT_OBSERVATION_CONVENTION = new DefaultServerRequestObservationConvention(); @@ -90,8 +83,6 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa private static final Log logger = LogFactory.getLog(HttpWebHandlerAdapter.class); - private static final Log lostClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY); - private WebSessionManager sessionManager = new DefaultWebSessionManager(); @@ -341,7 +332,9 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa responseHeaders.toString() : responseHeaders.isEmpty() ? "{}" : "{masked}"; } - private Mono handleUnresolvedError(ServerWebExchange exchange, ServerRequestObservationContext observationContext, Throwable ex) { + private Mono handleUnresolvedError( + ServerWebExchange exchange, ServerRequestObservationContext observationContext, Throwable ex) { + ServerHttpRequest request = exchange.getRequest(); ServerHttpResponse response = exchange.getResponse(); String logPrefix = exchange.getLogPrefix(); @@ -353,14 +346,7 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa logger.error(logPrefix + "500 Server Error for " + formatRequest(request), ex); return Mono.empty(); } - else if (isDisconnectedClientError(ex)) { - if (lostClientLogger.isTraceEnabled()) { - lostClientLogger.trace(logPrefix + "Client went away", ex); - } - else if (lostClientLogger.isDebugEnabled()) { - lostClientLogger.debug(logPrefix + "Client went away: " + ex + - " (stacktrace at TRACE level for '" + DISCONNECTED_CLIENT_LOG_CATEGORY + "')"); - } + else if (disconnectedClientHelper.checkAndLogClientDisconnectedException(ex)) { observationContext.setConnectionAborted(true); return Mono.empty(); } @@ -372,16 +358,6 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa } } - private boolean isDisconnectedClientError(Throwable ex) { - String message = NestedExceptionUtils.getMostSpecificCause(ex).getMessage(); - if (message != null) { - String text = message.toLowerCase(); - if (text.contains("broken pipe") || text.contains("connection reset by peer")) { - return true; - } - } - return DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName()); - } private final class ObservationSignalListener extends DefaultSignalListener { diff --git a/spring-web/src/main/java/org/springframework/web/util/DisconnectedClientHelper.java b/spring-web/src/main/java/org/springframework/web/util/DisconnectedClientHelper.java new file mode 100644 index 0000000000..286650e562 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/util/DisconnectedClientHelper.java @@ -0,0 +1,96 @@ +/* + * Copyright 2002-2023 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.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 6.1 + */ +public class DisconnectedClientHelper { + + private static final Set EXCEPTION_PHRASES = + Set.of("broken pipe", "connection reset by peer"); + + private static final Set EXCEPTION_TYPE_NAMES = + Set.of("ClientAbortException", "EOFException", "EofException"); + + + private final Log logger; + + + public DisconnectedClientHelper(String logCategory) { + Assert.notNull(logCategory, "'logCategory' is required"); + this.logger = LogFactory.getLog(logCategory); + } + + + /** + * Whether the given exception indicates the client has gone away. + * Known cases covered: + *

+ */ + public 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()); + } + + /** + * 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; + } + +} diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java index cddfe1f41c..583a25616d 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -23,14 +23,12 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.core.NestedExceptionUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.web.socket.CloseStatus; @@ -43,6 +41,7 @@ import org.springframework.web.socket.sockjs.frame.SockJsFrame; import org.springframework.web.socket.sockjs.frame.SockJsMessageCodec; import org.springframework.web.socket.sockjs.transport.SockJsServiceConfig; import org.springframework.web.socket.sockjs.transport.SockJsSession; +import org.springframework.web.util.DisconnectedClientHelper; /** * An abstract base class for SockJS sessions implementing {@link SockJsSession}. @@ -57,37 +56,15 @@ public abstract class AbstractSockJsSession implements SockJsSession { /** - * Log category to use on network IO exceptions after a client has gone away. - *

Servlet containers don't expose a client disconnected callback; see - * eclipse-ee4j/servlet-api#44. - * Therefore, network IO failures may occur simply because a client has gone away, - * and that can fill the logs with unnecessary stack traces. - *

We make a best effort to identify such network failures, on a per-server - * basis, and log them under a separate log category. A simple one-line message - * is logged at DEBUG level, while a full stack trace is shown at TRACE level. - * @see #disconnectedClientLogger + * Log category to use for network failure after a client has gone away. + * @see DisconnectedClientHelper */ public static final String DISCONNECTED_CLIENT_LOG_CATEGORY = "org.springframework.web.socket.sockjs.DisconnectedClient"; - /** - * Tomcat: ClientAbortException or EOFException - * Jetty: EofException - * WildFly, GlassFish: java.io.IOException "Broken pipe" (already covered) - *

TODO: - * This definition is currently duplicated between HttpWebHandlerAdapter - * and AbstractSockJsSession. It is a candidate for a common utility class. - * @see #indicatesDisconnectedClient(Throwable) - */ - private static final Set DISCONNECTED_CLIENT_EXCEPTIONS = - Set.of("ClientAbortException", "EOFException", "EofException"); - + private static final DisconnectedClientHelper disconnectedClientHelper = + new DisconnectedClientHelper(DISCONNECTED_CLIENT_LOG_CATEGORY); - /** - * Separate logger to use on network IO failure after a client has gone away. - * @see #DISCONNECTED_CLIENT_LOG_CATEGORY - */ - protected static final Log disconnectedClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY); protected final Log logger = LogFactory.getLog(getClass()); @@ -346,28 +323,11 @@ public abstract class AbstractSockJsSession implements SockJsSession { protected abstract void writeFrameInternal(SockJsFrame frame) throws IOException; private void logWriteFrameFailure(Throwable ex) { - if (indicatesDisconnectedClient(ex)) { - if (disconnectedClientLogger.isTraceEnabled()) { - disconnectedClientLogger.trace("Looks like the client has gone away", ex); - } - else if (disconnectedClientLogger.isDebugEnabled()) { - disconnectedClientLogger.debug("Looks like the client has gone away: " + ex + - " (For a full stack trace, set the log category '" + DISCONNECTED_CLIENT_LOG_CATEGORY + - "' to TRACE level.)"); - } - } - else { + if (!disconnectedClientHelper.checkAndLogClientDisconnectedException(ex)) { logger.debug("Terminating connection after failure to send message to client", ex); } } - private boolean indicatesDisconnectedClient(Throwable ex) { - String message = NestedExceptionUtils.getMostSpecificCause(ex).getMessage(); - message = (message != null ? message.toLowerCase() : ""); - String className = ex.getClass().getSimpleName(); - return (message.contains("broken pipe") || DISCONNECTED_CLIENT_EXCEPTIONS.contains(className)); - } - // Delegation methods