Skip to content

Commit a8019f2

Browse files
rstoyanchevrstoyanchev
committed
Create reusable DisconnectedClientHelper
See gh-26181
1 parent 49ff96d commit a8019f2

File tree

3 files changed

+112
-80
lines changed

3 files changed

+112
-80
lines changed

‎spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java

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

1717
package org.springframework.web.server.adapter;
1818

19-
import java.util.Set;
2019
import java.util.concurrent.atomic.AtomicBoolean;
2120

2221
import io.micrometer.observation.Observation;
@@ -29,7 +28,6 @@
2928
import reactor.util.context.Context;
3029

3130
import org.springframework.context.ApplicationContext;
32-
import org.springframework.core.NestedExceptionUtils;
3331
import org.springframework.core.log.LogFormatUtils;
3432
import org.springframework.http.HttpHeaders;
3533
import org.springframework.http.HttpStatus;
@@ -54,6 +52,7 @@
5452
import org.springframework.web.server.i18n.LocaleContextResolver;
5553
import org.springframework.web.server.session.DefaultWebSessionManager;
5654
import org.springframework.web.server.session.WebSessionManager;
55+
import org.springframework.web.util.DisconnectedClientHelper;
5756

5857
/**
5958
* Default adapter of {@link WebHandler} to the {@link HttpHandler} contract.
@@ -69,29 +68,21 @@
6968
public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHandler {
7069

7170
/**
72-
* Dedicated log category for disconnected client exceptions.
73-
* <p>Servlet containers don't expose a client disconnected callback; see
74-
* <a href="https://github.com/eclipse-ee4j/servlet-api/issues/44">eclipse-ee4j/servlet-api#44</a>.
75-
* <p>To avoid filling logs with unnecessary stack traces, we make an
76-
* effort to identify such network failures on a per-server basis, and then
77-
* log under a separate log category a simple one-line message at DEBUG level
78-
* or a full stack trace only at TRACE level.
71+
* Log category to use for network failure after a client has gone away.
72+
* @see DisconnectedClientHelper
7973
*/
8074
private static final String DISCONNECTED_CLIENT_LOG_CATEGORY =
8175
"org.springframework.web.server.DisconnectedClient";
8276

83-
// Similar declaration exists in AbstractSockJsSession.
84-
private static final Set<String> DISCONNECTED_CLIENT_EXCEPTIONS =
85-
Set.of("AbortedException", "ClientAbortException", "EOFException", "EofException");
77+
private static final DisconnectedClientHelper disconnectedClientHelper =
78+
new DisconnectedClientHelper(DISCONNECTED_CLIENT_LOG_CATEGORY);
8679

8780
private static final ServerRequestObservationConvention DEFAULT_OBSERVATION_CONVENTION =
8881
new DefaultServerRequestObservationConvention();
8982

9083

9184
private static final Log logger = LogFactory.getLog(HttpWebHandlerAdapter.class);
9285

93-
private static final Log lostClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY);
94-
9586

9687
private WebSessionManager sessionManager = new DefaultWebSessionManager();
9788

@@ -341,7 +332,9 @@ private String formatHeaders(HttpHeaders responseHeaders) {
341332
responseHeaders.toString() : responseHeaders.isEmpty() ? "{}" : "{masked}";
342333
}
343334

344-
private Mono<Void> handleUnresolvedError(ServerWebExchange exchange, ServerRequestObservationContext observationContext, Throwable ex) {
335+
private Mono<Void> handleUnresolvedError(
336+
ServerWebExchange exchange, ServerRequestObservationContext observationContext, Throwable ex) {
337+
345338
ServerHttpRequest request = exchange.getRequest();
346339
ServerHttpResponse response = exchange.getResponse();
347340
String logPrefix = exchange.getLogPrefix();
@@ -353,14 +346,7 @@ private Mono<Void> handleUnresolvedError(ServerWebExchange exchange, ServerReque
353346
logger.error(logPrefix + "500 Server Error for " + formatRequest(request), ex);
354347
return Mono.empty();
355348
}
356-
else if (isDisconnectedClientError(ex)) {
357-
if (lostClientLogger.isTraceEnabled()) {
358-
lostClientLogger.trace(logPrefix + "Client went away", ex);
359-
}
360-
else if (lostClientLogger.isDebugEnabled()) {
361-
lostClientLogger.debug(logPrefix + "Client went away: " + ex +
362-
" (stacktrace at TRACE level for '" + DISCONNECTED_CLIENT_LOG_CATEGORY + "')");
363-
}
349+
else if (disconnectedClientHelper.checkAndLogClientDisconnectedException(ex)) {
364350
observationContext.setConnectionAborted(true);
365351
return Mono.empty();
366352
}
@@ -372,16 +358,6 @@ else if (lostClientLogger.isDebugEnabled()) {
372358
}
373359
}
374360

375-
private boolean isDisconnectedClientError(Throwable ex) {
376-
String message = NestedExceptionUtils.getMostSpecificCause(ex).getMessage();
377-
if (message != null) {
378-
String text = message.toLowerCase();
379-
if (text.contains("broken pipe") || text.contains("connection reset by peer")) {
380-
return true;
381-
}
382-
}
383-
return DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName());
384-
}
385361

386362
private final class ObservationSignalListener extends DefaultSignalListener<Void> {
387363

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright 2002-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.web.util;
18+
19+
import java.util.Set;
20+
21+
import org.apache.commons.logging.Log;
22+
import org.apache.commons.logging.LogFactory;
23+
24+
import org.springframework.core.NestedExceptionUtils;
25+
import org.springframework.util.Assert;
26+
27+
/**
28+
* Utility methods to assist with identifying and logging exceptions that indicate
29+
* the client has gone away. Such exceptions fill logs with unnecessary stack
30+
* traces. The utility methods help to log a single line message at DEBUG level,
31+
* and a full stacktrace at TRACE level.
32+
*
33+
* @author Rossen Stoyanchev
34+
* @since 6.1
35+
*/
36+
public class DisconnectedClientHelper {
37+
38+
private static final Set<String> EXCEPTION_PHRASES =
39+
Set.of("broken pipe", "connection reset by peer");
40+
41+
private static final Set<String> EXCEPTION_TYPE_NAMES =
42+
Set.of("ClientAbortException", "EOFException", "EofException");
43+
44+
45+
private final Log logger;
46+
47+
48+
public DisconnectedClientHelper(String logCategory) {
49+
Assert.notNull(logCategory, "'logCategory' is required");
50+
this.logger = LogFactory.getLog(logCategory);
51+
}
52+
53+
54+
/**
55+
* Whether the given exception indicates the client has gone away.
56+
* Known cases covered:
57+
* <ul>
58+
* <li>ClientAbortException or EOFException for Tomcat
59+
* <li>EofException for Jetty
60+
* <li>IOException "Broken pipe" or "connection reset by peer"
61+
* </ul>
62+
*/
63+
public boolean isClientDisconnectedException(Throwable ex) {
64+
String message = NestedExceptionUtils.getMostSpecificCause(ex).getMessage();
65+
if (message != null) {
66+
String text = message.toLowerCase();
67+
for (String phrase : EXCEPTION_PHRASES) {
68+
if (text.contains(phrase)) {
69+
return true;
70+
}
71+
}
72+
}
73+
return EXCEPTION_TYPE_NAMES.contains(ex.getClass().getSimpleName());
74+
}
75+
76+
/**
77+
* Check via {@link #isClientDisconnectedException} if the exception
78+
* indicates the remote client disconnected, and if so log a single line
79+
* message when DEBUG is on, and a full stacktrace when TRACE is on for
80+
* the configured logger.
81+
*/
82+
public boolean checkAndLogClientDisconnectedException(Throwable ex) {
83+
if (isClientDisconnectedException(ex)) {
84+
if (logger.isTraceEnabled()) {
85+
logger.trace("Looks like the client has gone away", ex);
86+
}
87+
else if (logger.isDebugEnabled()) {
88+
logger.debug("Looks like the client has gone away: " + ex +
89+
" (For a full stack trace, set the log category '" + logger + "' to TRACE level.)");
90+
}
91+
return true;
92+
}
93+
return false;
94+
}
95+
96+
}

‎spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java

Lines changed: 7 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 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.
@@ -23,14 +23,12 @@
2323
import java.util.Collections;
2424
import java.util.List;
2525
import java.util.Map;
26-
import java.util.Set;
2726
import java.util.concurrent.ConcurrentHashMap;
2827
import java.util.concurrent.ScheduledFuture;
2928

3029
import org.apache.commons.logging.Log;
3130
import org.apache.commons.logging.LogFactory;
3231

33-
import org.springframework.core.NestedExceptionUtils;
3432
import org.springframework.lang.Nullable;
3533
import org.springframework.util.Assert;
3634
import org.springframework.web.socket.CloseStatus;
@@ -43,6 +41,7 @@
4341
import org.springframework.web.socket.sockjs.frame.SockJsMessageCodec;
4442
import org.springframework.web.socket.sockjs.transport.SockJsServiceConfig;
4543
import org.springframework.web.socket.sockjs.transport.SockJsSession;
44+
import org.springframework.web.util.DisconnectedClientHelper;
4645

4746
/**
4847
* An abstract base class for SockJS sessions implementing {@link SockJsSession}.
@@ -57,37 +56,15 @@ private enum State {NEW, OPEN, CLOSED}
5756

5857

5958
/**
60-
* Log category to use on network IO exceptions after a client has gone away.
61-
* <p>Servlet containers don't expose a client disconnected callback; see
62-
* <a href="https://github.com/eclipse-ee4j/servlet-api/issues/44">eclipse-ee4j/servlet-api#44</a>.
63-
* Therefore, network IO failures may occur simply because a client has gone away,
64-
* and that can fill the logs with unnecessary stack traces.
65-
* <p>We make a best effort to identify such network failures, on a per-server
66-
* basis, and log them under a separate log category. A simple one-line message
67-
* is logged at DEBUG level, while a full stack trace is shown at TRACE level.
68-
* @see #disconnectedClientLogger
59+
* Log category to use for network failure after a client has gone away.
60+
* @see DisconnectedClientHelper
6961
*/
7062
public static final String DISCONNECTED_CLIENT_LOG_CATEGORY =
7163
"org.springframework.web.socket.sockjs.DisconnectedClient";
7264

73-
/**
74-
* Tomcat: ClientAbortException or EOFException
75-
* Jetty: EofException
76-
* WildFly, GlassFish: java.io.IOException "Broken pipe" (already covered)
77-
* <p>TODO:
78-
* This definition is currently duplicated between HttpWebHandlerAdapter
79-
* and AbstractSockJsSession. It is a candidate for a common utility class.
80-
* @see #indicatesDisconnectedClient(Throwable)
81-
*/
82-
private static final Set<String> DISCONNECTED_CLIENT_EXCEPTIONS =
83-
Set.of("ClientAbortException", "EOFException", "EofException");
84-
65+
private static final DisconnectedClientHelper disconnectedClientHelper =
66+
new DisconnectedClientHelper(DISCONNECTED_CLIENT_LOG_CATEGORY);
8567

86-
/**
87-
* Separate logger to use on network IO failure after a client has gone away.
88-
* @see #DISCONNECTED_CLIENT_LOG_CATEGORY
89-
*/
90-
protected static final Log disconnectedClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY);
9168

9269
protected final Log logger = LogFactory.getLog(getClass());
9370

@@ -346,28 +323,11 @@ protected void writeFrame(SockJsFrame frame) throws SockJsTransportFailureExcept
346323
protected abstract void writeFrameInternal(SockJsFrame frame) throws IOException;
347324

348325
private void logWriteFrameFailure(Throwable ex) {
349-
if (indicatesDisconnectedClient(ex)) {
350-
if (disconnectedClientLogger.isTraceEnabled()) {
351-
disconnectedClientLogger.trace("Looks like the client has gone away", ex);
352-
}
353-
else if (disconnectedClientLogger.isDebugEnabled()) {
354-
disconnectedClientLogger.debug("Looks like the client has gone away: " + ex +
355-
" (For a full stack trace, set the log category '" + DISCONNECTED_CLIENT_LOG_CATEGORY +
356-
"' to TRACE level.)");
357-
}
358-
}
359-
else {
326+
if (!disconnectedClientHelper.checkAndLogClientDisconnectedException(ex)) {
360327
logger.debug("Terminating connection after failure to send message to client", ex);
361328
}
362329
}
363330

364-
private boolean indicatesDisconnectedClient(Throwable ex) {
365-
String message = NestedExceptionUtils.getMostSpecificCause(ex).getMessage();
366-
message = (message != null ? message.toLowerCase() : "");
367-
String className = ex.getClass().getSimpleName();
368-
return (message.contains("broken pipe") || DISCONNECTED_CLIENT_EXCEPTIONS.contains(className));
369-
}
370-
371331

372332
// Delegation methods
373333

0 commit comments

Comments
 (0)