diff --git a/spring-core/src/main/java/org/springframework/core/NestedCheckedException.java b/spring-core/src/main/java/org/springframework/core/NestedCheckedException.java index 23c7ceb16e..0cb2920dce 100644 --- a/spring-core/src/main/java/org/springframework/core/NestedCheckedException.java +++ b/spring-core/src/main/java/org/springframework/core/NestedCheckedException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2017 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. @@ -80,13 +80,7 @@ public abstract class NestedCheckedException extends Exception { * @return the innermost exception, or {@code null} if none */ public Throwable getRootCause() { - Throwable rootCause = null; - Throwable cause = getCause(); - while (cause != null && cause != rootCause) { - rootCause = cause; - cause = cause.getCause(); - } - return rootCause; + return NestedExceptionUtils.getRootCause(this); } /** diff --git a/spring-core/src/main/java/org/springframework/core/NestedExceptionUtils.java b/spring-core/src/main/java/org/springframework/core/NestedExceptionUtils.java index 8b66390f46..e7c11aed75 100644 --- a/spring-core/src/main/java/org/springframework/core/NestedExceptionUtils.java +++ b/spring-core/src/main/java/org/springframework/core/NestedExceptionUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2017 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. @@ -39,17 +39,48 @@ public abstract class NestedExceptionUtils { * @return the full exception message */ public static String buildMessage(String message, Throwable cause) { - if (cause != null) { - StringBuilder sb = new StringBuilder(); - if (message != null) { - sb.append(message).append("; "); - } - sb.append("nested exception is ").append(cause); - return sb.toString(); - } - else { + if (cause == null) { return message; } + StringBuilder sb = new StringBuilder(64); + if (message != null) { + sb.append(message).append("; "); + } + sb.append("nested exception is ").append(cause); + return sb.toString(); + } + + /** + * Retrieve the innermost cause of the given exception, if any. + * @param original the original exception to introspect + * @return the innermost exception, or {@code null} if none + * @since 4.3.9 + */ + public static Throwable getRootCause(Throwable original) { + if (original == null) { + return null; + } + Throwable rootCause = null; + Throwable cause = original.getCause(); + while (cause != null && cause != rootCause) { + rootCause = cause; + cause = cause.getCause(); + } + return rootCause; + } + + /** + * Retrieve the most specific cause of the given exception, that is, + * either the innermost cause (root cause) or the exception itself. + *

Differs from {@link #getRootCause} in that it falls back + * to the original exception if there is no root cause. + * @param original the original exception to introspect + * @return the most specific cause (never {@code null}) + * @since 4.3.9 + */ + public static Throwable getMostSpecificCause(Throwable original) { + Throwable rootCause = getRootCause(original); + return (rootCause != null ? rootCause : original); } } diff --git a/spring-core/src/main/java/org/springframework/core/NestedRuntimeException.java b/spring-core/src/main/java/org/springframework/core/NestedRuntimeException.java index 8d20d81f88..6e8843f0f8 100644 --- a/spring-core/src/main/java/org/springframework/core/NestedRuntimeException.java +++ b/spring-core/src/main/java/org/springframework/core/NestedRuntimeException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2017 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. @@ -81,13 +81,7 @@ public abstract class NestedRuntimeException extends RuntimeException { * @since 2.0 */ public Throwable getRootCause() { - Throwable rootCause = null; - Throwable cause = getCause(); - while (cause != null && cause != rootCause) { - rootCause = cause; - cause = cause.getCause(); - } - return rootCause; + return NestedExceptionUtils.getRootCause(this); } /** 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 e4cd24d9fd..a83c18066f 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 @@ -24,7 +24,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; -import org.springframework.core.NestedCheckedException; +import org.springframework.core.NestedExceptionUtils; import org.springframework.http.HttpStatus; import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.http.server.reactive.HttpHandler; @@ -33,7 +33,6 @@ import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebHandler; -import org.springframework.web.server.handler.ExceptionHandlingWebHandler; import org.springframework.web.server.handler.WebHandlerDecorator; import org.springframework.web.server.session.DefaultWebSessionManager; import org.springframework.web.server.session.WebSessionManager; @@ -60,8 +59,17 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa * or a full stack trace only at TRACE level. */ private static final String DISCONNECTED_CLIENT_LOG_CATEGORY = - ExceptionHandlingWebHandler.class.getName() + ".DisconnectedClient"; + "org.springframework.web.server.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 = new HashSet<>(Arrays.asList("ClientAbortException", "EOFException", "EofException")); @@ -115,7 +123,7 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa * Return the configured {@link ServerCodecConfigurer}. */ public ServerCodecConfigurer getCodecConfigurer() { - return this.codecConfigurer != null ? this.codecConfigurer : ServerCodecConfigurer.create(); + return (this.codecConfigurer != null ? this.codecConfigurer : ServerCodecConfigurer.create()); } @@ -125,7 +133,7 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa return getDelegate().handle(exchange) .onErrorResume(ex -> { response.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR); - logException(ex); + logHandleFailure(ex); return Mono.empty(); }) .then(Mono.defer(response::setComplete)); @@ -135,25 +143,25 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa return new DefaultServerWebExchange(request, response, this.sessionManager, getCodecConfigurer()); } - @SuppressWarnings("serial") - private void logException(Throwable ex) { - NestedCheckedException nestedEx = new NestedCheckedException("", ex) {}; - if ("Broken pipe".equalsIgnoreCase(nestedEx.getMostSpecificCause().getMessage()) || - DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName())) { - + private void logHandleFailure(Throwable ex) { + if (indicatesDisconnectedClient(ex)) { if (disconnectedClientLogger.isTraceEnabled()) { disconnectedClientLogger.trace("Looks like the client has gone away", ex); } else if (disconnectedClientLogger.isDebugEnabled()) { - disconnectedClientLogger.debug( - "The client has gone away: " + nestedEx.getMessage() + - " (For a full stack trace, set the log category" + - "'" + DISCONNECTED_CLIENT_LOG_CATEGORY + "' to TRACE)"); + 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 { - logger.error("Could not complete request", ex); + logger.error("Failed to handle request", ex); } } + private boolean indicatesDisconnectedClient(Throwable ex) { + return ("Broken pipe".equalsIgnoreCase(NestedExceptionUtils.getMostSpecificCause(ex).getMessage()) || + DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName())); + } + } 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 91fe54caa2..e872b5f34c 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 @@ -19,7 +19,6 @@ package org.springframework.web.socket.sockjs.transport.session; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.Date; import java.util.HashSet; import java.util.List; @@ -31,7 +30,7 @@ import java.util.concurrent.ScheduledFuture; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.core.NestedCheckedException; +import org.springframework.core.NestedExceptionUtils; import org.springframework.util.Assert; import org.springframework.web.socket.CloseStatus; import org.springframework.web.socket.TextMessage; @@ -70,25 +69,25 @@ public abstract class AbstractSockJsSession implements SockJsSession { 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 = + new HashSet<>(Arrays.asList("ClientAbortException", "EOFException", "EofException")); + + /** * 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); - - private static final Set disconnectedClientExceptions; - - static { - Set set = new HashSet(4); - set.add("ClientAbortException"); // Tomcat - set.add("EOFException"); // Tomcat - set.add("EofException"); // Jetty - // java.io.IOException "Broken pipe" on WildFly, Glassfish (already covered) - disconnectedClientExceptions = Collections.unmodifiableSet(set); - } - - protected final Log logger = LogFactory.getLog(getClass()); protected final Object responseLock = new Object(); @@ -340,28 +339,28 @@ public abstract class AbstractSockJsSession implements SockJsSession { } } - private void logWriteFrameFailure(Throwable failure) { - @SuppressWarnings("serial") - NestedCheckedException nestedException = new NestedCheckedException("", failure) {}; - - if ("Broken pipe".equalsIgnoreCase(nestedException.getMostSpecificCause().getMessage()) || - disconnectedClientExceptions.contains(failure.getClass().getSimpleName())) { + 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", failure); + disconnectedClientLogger.trace("Looks like the client has gone away", ex); } else if (disconnectedClientLogger.isDebugEnabled()) { - disconnectedClientLogger.debug("Looks like the client has gone away: " + - nestedException.getMessage() + " (For full stack trace, set the '" + - DISCONNECTED_CLIENT_LOG_CATEGORY + "' log category to TRACE level)"); + 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 { - logger.debug("Terminating connection after failure to send message to client", failure); + logger.debug("Terminating connection after failure to send message to client", ex); } } - protected abstract void writeFrameInternal(SockJsFrame frame) throws IOException; + private boolean indicatesDisconnectedClient(Throwable ex) { + return ("Broken pipe".equalsIgnoreCase(NestedExceptionUtils.getMostSpecificCause(ex).getMessage()) || + DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName())); + } // Delegation methods @@ -421,7 +420,8 @@ public abstract class AbstractSockJsSession implements SockJsSession { delegateError(error); } catch (Throwable delegateException) { - // ignore + // Ignore + logger.debug("Exception from error handling delegate", delegateException); } try { close(closeStatus);