From 166ca7a5a3254c6d63551ad286a9e55375276e85 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 1 May 2013 15:57:55 -0400 Subject: [PATCH] Update exception handling Allow WebSocketHandler methods to raise an exception. By default we install ExceptionWebSocketHandlerDecorator, which logs unhandled exceptions and closes the session. That decorator can be extended or replaced. Any exceptions that remain unhandled still (i.e. no exception handling decorator), are caught in the lowest level before propagating to the WebSocket engine or a SockJS transport handler and handled the same way. That means default behavior is guaranteed but also fully customizable. --- .../sockjs/AbstractSockJsSession.java | 30 +++++++---- .../server/AbstractServerSockJsSession.java | 2 +- ...ption.java => SockJsRuntimeException.java} | 6 +-- ...AbstractHttpReceivingTransportHandler.java | 10 +++- .../AbstractHttpServerSockJsSession.java | 8 ++- .../transport/SockJsWebSocketHandler.java | 24 ++++++--- .../websocket/PartialMessageHandler.java | 28 ----------- .../websocket/WebSocketHandler.java | 40 ++++++++++++--- .../JettyWebSocketListenerAdapter.java | 50 +++++++++++++++---- .../adapter/StandardEndpointAdapter.java | 41 +++++++++++++-- .../adapter/WebSocketHandlerAdapter.java | 12 ++--- .../WebSocketConnectFailureException.java | 1 + .../server/DefaultHandshakeHandler.java | 12 ++--- .../server/HandshakeFailureException.java | 48 ++++++++++++++++++ .../websocket/server/HandshakeHandler.java | 16 +++++- .../server/RequestUpgradeStrategy.java | 11 ++-- .../GlassfishRequestUpgradeStrategy.java | 7 +-- .../support/JettyRequestUpgradeStrategy.java | 3 +- .../support/TomcatRequestUpgradeStrategy.java | 3 +- .../support/WebSocketHttpRequestHandler.java | 10 ++-- .../ExceptionWebSocketHandlerDecorator.java | 12 ++--- .../LoggingWebSocketHandlerDecorator.java | 8 +-- .../PerConnectionWebSocketHandlerProxy.java | 8 +-- .../support/WebSocketHandlerDecorator.java | 8 +-- 24 files changed, 277 insertions(+), 121 deletions(-) rename spring-websocket/src/main/java/org/springframework/sockjs/server/{NestedSockJsRuntimeException.java => SockJsRuntimeException.java} (81%) delete mode 100644 spring-websocket/src/main/java/org/springframework/websocket/PartialMessageHandler.java create mode 100644 spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeFailureException.java diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java b/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java index 3912dcb268b..30514835ad1 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java @@ -118,7 +118,7 @@ public abstract class AbstractSockJsSession implements WebSocketSession { this.timeLastActive = System.currentTimeMillis(); } - public void delegateConnectionEstablished() { + public void delegateConnectionEstablished() throws Exception { this.state = State.OPEN; this.handler.afterConnectionEstablished(this); } @@ -127,23 +127,28 @@ public abstract class AbstractSockJsSession implements WebSocketSession { * Close due to error arising from SockJS transport handling. */ protected void tryCloseWithSockJsTransportError(Throwable ex, CloseStatus closeStatus) { - delegateError(ex); + logger.error("Closing due to transport error for " + this, ex); try { - logger.error("Closing due to transport error for " + this, ex); - close(closeStatus); + delegateError(ex); } - catch (Throwable t) { - // ignore + catch (Throwable delegateEx) { + logger.error("Unhandled error for " + this, delegateEx); + try { + close(closeStatus); + } + catch (Throwable closeEx) { + logger.error("Unhandled error for " + this, closeEx); + } } } - public void delegateMessages(String[] messages) { + public void delegateMessages(String[] messages) throws Exception { for (String message : messages) { this.handler.handleMessage(this, new TextMessage(message)); } } - public void delegateError(Throwable ex) { + public void delegateError(Throwable ex) throws Exception { this.handler.handleTransportError(this, ex); } @@ -153,7 +158,7 @@ public abstract class AbstractSockJsSession implements WebSocketSession { * {@link TextMessageHandler}. This is in contrast to {@link #close()} that pro-actively * closes the connection. */ - public final void delegateConnectionClosed(CloseStatus status) { + public final void delegateConnectionClosed(CloseStatus status) throws Exception { if (!isClosed()) { if (logger.isDebugEnabled()) { logger.debug(this + " was closed, " + status); @@ -193,7 +198,12 @@ public abstract class AbstractSockJsSession implements WebSocketSession { } finally { this.state = State.CLOSED; - this.handler.afterConnectionClosed(this, status); + try { + this.handler.afterConnectionClosed(this, status); + } + catch (Throwable t) { + logger.error("Unhandled error for " + this, t); + } } } } diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java index fdd9f153305..554d813a551 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java @@ -111,7 +111,7 @@ public abstract class AbstractServerSockJsSession extends AbstractSockJsSession catch (Throwable ex) { logger.warn("Terminating connection due to failure to send message: " + ex.getMessage()); close(); - throw new NestedSockJsRuntimeException("Failed to write " + frame, ex); + throw new SockJsRuntimeException("Failed to write " + frame, ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/NestedSockJsRuntimeException.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/SockJsRuntimeException.java similarity index 81% rename from spring-websocket/src/main/java/org/springframework/sockjs/server/NestedSockJsRuntimeException.java rename to spring-websocket/src/main/java/org/springframework/sockjs/server/SockJsRuntimeException.java index 63f82da2575..e9026012bb0 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/NestedSockJsRuntimeException.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/SockJsRuntimeException.java @@ -24,13 +24,13 @@ import org.springframework.core.NestedRuntimeException; * @since 4.0 */ @SuppressWarnings("serial") -public class NestedSockJsRuntimeException extends NestedRuntimeException { +public class SockJsRuntimeException extends NestedRuntimeException { - public NestedSockJsRuntimeException(String msg) { + public SockJsRuntimeException(String msg) { super(msg); } - public NestedSockJsRuntimeException(String msg, Throwable cause) { + public SockJsRuntimeException(String msg, Throwable cause) { super(msg, cause); } diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java index 0f8d492eb1b..078c354eee9 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java @@ -27,9 +27,11 @@ import org.springframework.http.MediaType; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; import org.springframework.sockjs.AbstractSockJsSession; +import org.springframework.sockjs.server.SockJsRuntimeException; import org.springframework.sockjs.server.TransportErrorException; import org.springframework.sockjs.server.TransportHandler; import org.springframework.websocket.WebSocketHandler; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -89,7 +91,13 @@ public abstract class AbstractHttpReceivingTransportHandler implements Transport logger.trace("Received messages: " + Arrays.asList(messages)); } - session.delegateMessages(messages); + try { + session.delegateMessages(messages); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(session, t, logger); + throw new SockJsRuntimeException("Unhandled WebSocketHandler error in " + this, t); + } response.setStatusCode(getResponseStatus()); response.getHeaders().setContentType(new MediaType("text", "plain", Charset.forName("UTF-8"))); diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java index 055102b681e..856296ceb64 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java @@ -31,6 +31,7 @@ import org.springframework.sockjs.server.TransportErrorException; import org.springframework.util.Assert; import org.springframework.websocket.CloseStatus; import org.springframework.websocket.WebSocketHandler; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; /** * An abstract base class for use with HTTP-based transports. @@ -65,7 +66,12 @@ public abstract class AbstractHttpServerSockJsSession extends AbstractServerSock tryCloseWithSockJsTransportError(t, null); throw new TransportErrorException("Failed open SockJS session", t, getId()); } - delegateConnectionEstablished(); + try { + delegateConnectionEstablished(); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this, t, logger); + } } protected void writePrelude() throws IOException { diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java index 13a0b9e12e4..e1978fe226b 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java @@ -34,8 +34,16 @@ import com.fasterxml.jackson.databind.ObjectMapper; /** - * A wrapper around a {@link WebSocketHandler} instance that parses as well as adds SockJS - * messages frames as well as sends SockJS heartbeat messages. + * A wrapper around a {@link WebSocketHandler} instance that parses and adds SockJS + * messages frames and also sends SockJS heartbeat messages. + * + *

+ * Implementations of the {@link WebSocketHandler} interface in this class allow + * exceptions from the wrapped {@link WebSocketHandler} to propagate. However, any + * exceptions resulting from SockJS message handling (e.g. while sending SockJS frames or + * heartbeat messages) are caught and treated as transport errors, i.e. routed to the + * {@link WebSocketHandler#handleTransportError(WebSocketSession, Throwable) + * handleTransportError} method of the wrapped handler and the session closed. * * @author Rossen Stoyanchev * @since 4.0 @@ -66,24 +74,24 @@ public class SockJsWebSocketHandler extends TextWebSocketHandlerAdapter { } @Override - public void afterConnectionEstablished(WebSocketSession wsSession) { + public void afterConnectionEstablished(WebSocketSession wsSession) throws Exception { Assert.isTrue(this.sessionCount.compareAndSet(0, 1), "Unexpected connection"); this.sockJsSession = new WebSocketServerSockJsSession(getSockJsSessionId(wsSession), getSockJsConfig()); this.sockJsSession.initWebSocketSession(wsSession); } @Override - public void handleTextMessage(WebSocketSession wsSession, TextMessage message) { + public void handleTextMessage(WebSocketSession wsSession, TextMessage message) throws Exception { this.sockJsSession.handleMessage(message, wsSession); } @Override - public void afterConnectionClosed(WebSocketSession wsSession, CloseStatus status) { + public void afterConnectionClosed(WebSocketSession wsSession, CloseStatus status) throws Exception { this.sockJsSession.delegateConnectionClosed(status); } @Override - public void handleTransportError(WebSocketSession webSocketSession, Throwable exception) { + public void handleTransportError(WebSocketSession webSocketSession, Throwable exception) throws Exception { this.sockJsSession.delegateError(exception); } @@ -105,7 +113,7 @@ public class SockJsWebSocketHandler extends TextWebSocketHandlerAdapter { super(sessionId, config, SockJsWebSocketHandler.this.webSocketHandler); } - public void initWebSocketSession(WebSocketSession wsSession) { + public void initWebSocketSession(WebSocketSession wsSession) throws Exception { this.wsSession = wsSession; try { TextMessage message = new TextMessage(SockJsFrame.openFrame().getContent()); @@ -124,7 +132,7 @@ public class SockJsWebSocketHandler extends TextWebSocketHandlerAdapter { return this.wsSession.isOpen(); } - public void handleMessage(TextMessage message, WebSocketSession wsSession) { + public void handleMessage(TextMessage message, WebSocketSession wsSession) throws Exception { String payload = message.getPayload(); if (StringUtils.isEmpty(payload)) { logger.trace("Ignoring empty message"); diff --git a/spring-websocket/src/main/java/org/springframework/websocket/PartialMessageHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/PartialMessageHandler.java deleted file mode 100644 index 30524be5fa4..00000000000 --- a/spring-websocket/src/main/java/org/springframework/websocket/PartialMessageHandler.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2002-2013 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 - * - * http://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.websocket; - -/** - * A "marker" interface for {@link BinaryMessageHandler} types that wish to handle partial - * messages. - * - * @author Rossen Stoyanchev - * @since 4.0 - */ -public interface PartialMessageHandler { - -} diff --git a/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java index 4220facf782..21dc2150973 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java @@ -17,7 +17,15 @@ package org.springframework.websocket; /** - * A handler for WebSocket sessions. + * A handler for WebSocket messages and lifecycle events. + * + *

Implementations of this interface are encouraged to handle exceptions locally where + * it makes sense or alternatively let the exception bubble up in which case the exception + * is logged and the session closed with {@link CloseStatus#SERVER_ERROR SERVER_ERROR(101)} by default. + * The exception handling strategy is provided by + * {@link org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator ExceptionWebSocketHandlerDecorator}, + * which can be customized or replaced by decorating the {@link WebSocketHandler} with a + * different decorator. * * @param The type of message being handled {@link TextMessage}, {@link BinaryMessage} * (or {@link WebSocketMessage} for both). @@ -29,24 +37,40 @@ package org.springframework.websocket; public interface WebSocketHandler { /** - * A new WebSocket connection has been opened and is ready to be used. + * Invoked after WebSocket negotiation has succeeded and the WebSocket connection is + * opened and ready for use. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void afterConnectionEstablished(WebSocketSession session); + void afterConnectionEstablished(WebSocketSession session) throws Exception; /** - * Handle an incoming WebSocket message. + * Invoked when a new WebSocket message arrives. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void handleMessage(WebSocketSession session, WebSocketMessage message); + void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception; /** * Handle an error from the underlying WebSocket message transport. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void handleTransportError(WebSocketSession session, Throwable exception); + void handleTransportError(WebSocketSession session, Throwable exception) throws Exception; /** - * A WebSocket connection has been closed. + * Invoked after the WebSocket connection has been closed by either side, or after a + * transport error has occurred. Although the session may technically still be open, + * depending on the underlying implementation, sending messages at this point is + * discouraged and most likely will not succeed. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus); + void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception; /** * Whether this WebSocketHandler wishes to receive messages broken up in parts. diff --git a/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java b/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java index 163ffa057fd..04458b35601 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java @@ -16,6 +16,8 @@ package org.springframework.websocket.adapter; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.WebSocketListener; import org.springframework.util.Assert; @@ -24,6 +26,7 @@ import org.springframework.websocket.CloseStatus; import org.springframework.websocket.TextMessage; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.WebSocketSession; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; /** * Adapts Spring's {@link WebSocketHandler} to Jetty's {@link WebSocketListener}. @@ -33,6 +36,8 @@ import org.springframework.websocket.WebSocketSession; */ public class JettyWebSocketListenerAdapter implements WebSocketListener { + private static final Log logger = LogFactory.getLog(JettyWebSocketListenerAdapter.class); + private final WebSocketHandler webSocketHandler; private WebSocketSession wsSession; @@ -47,30 +52,55 @@ public class JettyWebSocketListenerAdapter implements WebSocketListener { @Override public void onWebSocketConnect(Session session) { this.wsSession = new JettyWebSocketSessionAdapter(session); - this.webSocketHandler.afterConnectionEstablished(this.wsSession); - } - - @Override - public void onWebSocketClose(int statusCode, String reason) { - CloseStatus closeStatus = new CloseStatus(statusCode, reason); - this.webSocketHandler.afterConnectionClosed(this.wsSession, closeStatus); + try { + this.webSocketHandler.afterConnectionEstablished(this.wsSession); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } @Override public void onWebSocketText(String payload) { TextMessage message = new TextMessage(payload); - this.webSocketHandler.handleMessage(this.wsSession, message); + try { + this.webSocketHandler.handleMessage(this.wsSession, message); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } @Override public void onWebSocketBinary(byte[] payload, int offset, int len) { BinaryMessage message = new BinaryMessage(payload, offset, len); - this.webSocketHandler.handleMessage(this.wsSession, message); + try { + this.webSocketHandler.handleMessage(this.wsSession, message); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } + } + + @Override + public void onWebSocketClose(int statusCode, String reason) { + CloseStatus closeStatus = new CloseStatus(statusCode, reason); + try { + this.webSocketHandler.afterConnectionClosed(this.wsSession, closeStatus); + } + catch (Throwable t) { + logger.error("Unhandled error for " + this.wsSession, t); + } } @Override public void onWebSocketError(Throwable cause) { - this.webSocketHandler.handleTransportError(this.wsSession, cause); + try { + this.webSocketHandler.handleTransportError(this.wsSession, cause); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java b/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java index a36249078e9..4f6f731281e 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java @@ -23,12 +23,15 @@ import javax.websocket.Endpoint; import javax.websocket.EndpointConfig; import javax.websocket.MessageHandler; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.util.Assert; import org.springframework.websocket.BinaryMessage; import org.springframework.websocket.CloseStatus; import org.springframework.websocket.TextMessage; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.WebSocketSession; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; /** @@ -39,6 +42,8 @@ import org.springframework.websocket.WebSocketSession; */ public class StandardEndpointAdapter extends Endpoint { + private static final Log logger = LogFactory.getLog(StandardEndpointAdapter.class); + private final WebSocketHandler handler; private WebSocketSession wsSession; @@ -54,7 +59,13 @@ public class StandardEndpointAdapter extends Endpoint { public void onOpen(final javax.websocket.Session session, EndpointConfig config) { this.wsSession = new StandardWebSocketSessionAdapter(session); - this.handler.afterConnectionEstablished(this.wsSession); + try { + this.handler.afterConnectionEstablished(this.wsSession); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + return; + } session.addMessageHandler(new MessageHandler.Whole() { @Override @@ -84,23 +95,43 @@ public class StandardEndpointAdapter extends Endpoint { private void handleTextMessage(javax.websocket.Session session, String payload) { TextMessage textMessage = new TextMessage(payload); - this.handler.handleMessage(this.wsSession, textMessage); + try { + this.handler.handleMessage(this.wsSession, textMessage); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } private void handleBinaryMessage(javax.websocket.Session session, ByteBuffer payload, boolean isLast) { BinaryMessage binaryMessage = new BinaryMessage(payload, isLast); - this.handler.handleMessage(this.wsSession, binaryMessage); + try { + this.handler.handleMessage(this.wsSession, binaryMessage); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } @Override public void onClose(javax.websocket.Session session, CloseReason reason) { CloseStatus closeStatus = new CloseStatus(reason.getCloseCode().getCode(), reason.getReasonPhrase()); - this.handler.afterConnectionClosed(this.wsSession, closeStatus); + try { + this.handler.afterConnectionClosed(this.wsSession, closeStatus); + } + catch (Throwable t) { + logger.error("Unhandled error for " + this.wsSession, t); + } } @Override public void onError(javax.websocket.Session session, Throwable exception) { - this.handler.handleTransportError(this.wsSession, exception); + try { + this.handler.handleTransportError(this.wsSession, exception); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java b/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java index 850cb2cacb2..fd88b052d67 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java @@ -37,11 +37,11 @@ import org.springframework.websocket.WebSocketSession; public class WebSocketHandlerAdapter implements WebSocketHandler { @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { } @Override - public final void handleMessage(WebSocketSession session, WebSocketMessage message) { + public final void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { if (message instanceof TextMessage) { handleTextMessage(session, (TextMessage) message); } @@ -54,18 +54,18 @@ public class WebSocketHandlerAdapter implements WebSocketHandler { } } - protected void handleTextMessage(WebSocketSession session, TextMessage message) { + protected void handleTextMessage(WebSocketSession session, TextMessage message) throws Exception { } - protected void handleBinaryMessage(WebSocketSession session, BinaryMessage message) { + protected void handleBinaryMessage(WebSocketSession session, BinaryMessage message) throws Exception { } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus status) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus status) throws Exception { } @Override diff --git a/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java b/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java index a6f904b1ef1..0ed7c32e8e6 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java @@ -25,6 +25,7 @@ import org.springframework.core.NestedRuntimeException; @SuppressWarnings("serial") public class WebSocketConnectFailureException extends NestedRuntimeException { + public WebSocketConnectFailureException(String msg, Throwable cause) { super(msg, cause); } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java index b369aca0850..5e658feccc6 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java @@ -89,7 +89,7 @@ public class DefaultHandshakeHandler implements HandshakeHandler { @Override public final boolean doHandshake(ServerHttpRequest request, ServerHttpResponse response, - WebSocketHandler webSocketHandler) throws IOException { + WebSocketHandler webSocketHandler) throws IOException, HandshakeFailureException { logger.debug("Starting handshake for " + request.getURI()); @@ -201,14 +201,14 @@ public class DefaultHandshakeHandler implements HandshakeHandler { return null; } - private String getWebSocketKeyHash(String key) { + private String getWebSocketKeyHash(String key) throws HandshakeFailureException { try { - MessageDigest digest = MessageDigest.getInstance("SHA1"); - byte[] bytes = digest.digest((key + GUID).getBytes(Charset.forName("ISO-8859-1"))); - return DatatypeConverter.printBase64Binary(bytes); + MessageDigest digest = MessageDigest.getInstance("SHA1"); + byte[] bytes = digest.digest((key + GUID).getBytes(Charset.forName("ISO-8859-1"))); + return DatatypeConverter.printBase64Binary(bytes); } catch (NoSuchAlgorithmException ex) { - throw new IllegalStateException("Failed to generate value for Sec-WebSocket-Key header", ex); + throw new HandshakeFailureException("Failed to generate value for Sec-WebSocket-Key header", ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeFailureException.java b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeFailureException.java new file mode 100644 index 00000000000..0c17392d39c --- /dev/null +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeFailureException.java @@ -0,0 +1,48 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.websocket.server; + +import org.springframework.core.NestedRuntimeException; + + +/** + * Thrown when handshake processing failed to complete due to an internal, unrecoverable + * error. This implies a server error (HTTP status code 500) as opposed to a failure in + * the handshake negotiation. + * + *

+ * By contrast, when handshake negotiation fails, the response status code will be 200 and + * the response headers and body will have been updated to reflect the cause for the + * failure. A {@link HandshakeHandler} implementation will have protected methods to + * customize updates to the response in those cases. + * + * @author Rossen Stoyanchev + * @since 4.0 + */ +@SuppressWarnings("serial") +public class HandshakeFailureException extends NestedRuntimeException { + + + public HandshakeFailureException(String msg, Throwable cause) { + super(msg, cause); + } + + public HandshakeFailureException(String msg) { + super(msg); + } + +} diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java index fbdf2ca1a60..4a2df4928c0 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java @@ -31,7 +31,21 @@ import org.springframework.websocket.WebSocketHandler; public interface HandshakeHandler { + /** + * + * @param request + * @param response + * @param webSocketHandler + * @return + * + * @throws IOException thrown when accessing or setting the response + * + * @throws HandshakeFailureException thrown when handshake processing failed to + * complete due to an internal, unrecoverable error, i.e. a server error as + * opposed to a failure to successfully negotiate the requirements of the + * handshake request. + */ boolean doHandshake(ServerHttpRequest request, ServerHttpResponse response, WebSocketHandler webSocketHandler) - throws IOException; + throws IOException, HandshakeFailureException; } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java index 70a7653af49..f717ac81e3a 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java @@ -37,12 +37,17 @@ public interface RequestUpgradeStrategy { String[] getSupportedVersions(); /** - * Perform runtime specific steps to complete the upgrade. - * Invoked only if the handshake is successful. + * Perform runtime specific steps to complete the upgrade. Invoked after successful + * negotiation of the handshake request. * * @param webSocketHandler the handler for WebSocket messages + * + * @throws HandshakeFailureException thrown when handshake processing failed to + * complete due to an internal, unrecoverable error, i.e. a server error as + * opposed to a failure to successfully negotiate the requirements of the + * handshake request. */ void upgrade(ServerHttpRequest request, ServerHttpResponse response, String selectedProtocol, - WebSocketHandler webSocketHandler) throws IOException; + WebSocketHandler webSocketHandler) throws IOException, HandshakeFailureException; } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java index a5ca56c5b50..358813af800 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java @@ -48,6 +48,7 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; +import org.springframework.websocket.server.HandshakeFailureException; import org.springframework.websocket.server.endpoint.EndpointRegistration; /** @@ -69,7 +70,7 @@ public class GlassfishRequestUpgradeStrategy extends AbstractEndpointUpgradeStra @Override public void upgradeInternal(ServerHttpRequest request, ServerHttpResponse response, - String selectedProtocol, Endpoint endpoint) throws IOException { + String selectedProtocol, Endpoint endpoint) throws IOException, HandshakeFailureException { Assert.isTrue(request instanceof ServletServerHttpRequest); HttpServletRequest servletRequest = ((ServletServerHttpRequest) request).getServletRequest(); @@ -85,12 +86,12 @@ public class GlassfishRequestUpgradeStrategy extends AbstractEndpointUpgradeStra engine.register(tyrusEndpoint); } catch (DeploymentException ex) { - throw new IllegalStateException("Failed to deploy endpoint in Glassfish", ex); + throw new HandshakeFailureException("Failed to deploy endpoint in Glassfish", ex); } try { if (!performUpgrade(servletRequest, servletResponse, request.getHeaders(), tyrusEndpoint)) { - throw new IllegalStateException("Failed to upgrade HttpServletRequest"); + throw new HandshakeFailureException("Failed to upgrade HttpServletRequest"); } } finally { diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java index ef5674c6bd0..b536138e240 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java @@ -34,6 +34,7 @@ import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.util.Assert; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.adapter.JettyWebSocketListenerAdapter; +import org.springframework.websocket.server.HandshakeFailureException; import org.springframework.websocket.server.RequestUpgradeStrategy; /** @@ -106,7 +107,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy { if (!this.factory.acceptWebSocket(request, response)) { // should never happen - throw new IllegalStateException("WebSocket request not accepted by Jetty"); + throw new HandshakeFailureException("WebSocket request not accepted by Jetty"); } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java index 0dcced9a125..c5a262b3a8d 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java @@ -32,6 +32,7 @@ import org.springframework.http.server.ServerHttpResponse; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; +import org.springframework.websocket.server.HandshakeFailureException; import org.springframework.websocket.server.endpoint.EndpointRegistration; /** @@ -63,7 +64,7 @@ public class TomcatRequestUpgradeStrategy extends AbstractEndpointUpgradeStrateg method.invoke(webSocketRequest); } catch (Exception ex) { - throw new IllegalStateException("Failed to upgrade HttpServletRequest", ex); + throw new HandshakeFailureException("Failed to upgrade HttpServletRequest", ex); } // TODO: use ServletContext attribute when Tomcat is updated diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java index 6bb1bfa171f..0b8b8c2d8b5 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java @@ -28,7 +28,6 @@ import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.util.Assert; import org.springframework.web.HttpRequestHandler; -import org.springframework.web.util.NestedServletException; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.server.DefaultHandshakeHandler; import org.springframework.websocket.server.HandshakeHandler; @@ -79,14 +78,11 @@ public class WebSocketHttpRequestHandler implements HttpRequestHandler { try { this.handshakeHandler.doHandshake(httpRequest, httpResponse, this.webSocketHandler); - } - catch (Exception e) { - // TODO - throw new NestedServletException("HandshakeHandler failure", e); - } - finally { httpResponse.flush(); } + catch (IOException ex) { + throw ex; + } } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java index 8d45636d198..0e82d032494 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java @@ -43,11 +43,11 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato getDelegate().afterConnectionEstablished(session); } catch (Throwable ex) { - tryCloseWithError(session, ex); + tryCloseWithError(session, ex, logger); } } - private void tryCloseWithError(WebSocketSession session, Throwable exception) { + public static void tryCloseWithError(WebSocketSession session, Throwable exception, Log logger) { logger.error("Closing due to exception for " + session, exception); if (session.isOpen()) { try { @@ -65,7 +65,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato getDelegate().handleMessage(session, message); } catch (Throwable ex) { - tryCloseWithError(session,ex); + tryCloseWithError(session, ex, logger); } } @@ -75,7 +75,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato getDelegate().handleTransportError(session, exception); } catch (Throwable ex) { - tryCloseWithError(session, ex); + tryCloseWithError(session, ex, logger); } } @@ -84,8 +84,8 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato try { getDelegate().afterConnectionClosed(session, closeStatus); } - catch (Throwable ex) { - logger.error("Unhandled error for " + this, ex); + catch (Throwable t) { + logger.error("Unhandled error for " + this, t); } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java index 8cb0e0e8c21..6dbad07312f 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java @@ -39,7 +39,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Connection established, " + session + ", uri=" + session.getURI()); } @@ -47,7 +47,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator } @Override - public void handleMessage(WebSocketSession session, WebSocketMessage message) { + public void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { if (logger.isTraceEnabled()) { logger.trace("Received " + message + ", " + session); } @@ -55,7 +55,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Transport error for " + session, exception); } @@ -63,7 +63,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Connection closed for " + session + ", " + closeStatus); } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java b/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java index 350c23bc052..64ed5c4bec9 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java @@ -81,14 +81,14 @@ public class PerConnectionWebSocketHandlerProxy implements WebSocketHandler, Bea } @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { WebSocketHandler handler = this.provider.getHandler(); this.handlers.put(session, handler); handler.afterConnectionEstablished(session); } @Override - public void handleMessage(WebSocketSession session, WebSocketMessage message) { + public void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { getHandler(session).handleMessage(session, message); } @@ -99,12 +99,12 @@ public class PerConnectionWebSocketHandlerProxy implements WebSocketHandler, Bea } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { getHandler(session).handleTransportError(session, exception); } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { try { getHandler(session).afterConnectionClosed(session, closeStatus); } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java index a94fbb8b518..5fb9d6d7d3a 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java @@ -43,22 +43,22 @@ public class WebSocketHandlerDecorator implements WebSocketHandler { } @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { this.delegate.afterConnectionEstablished(session); } @Override - public void handleMessage(WebSocketSession session, WebSocketMessage message) { + public void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { this.delegate.handleMessage(session, message); } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { this.delegate.handleTransportError(session, exception); } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { this.delegate.afterConnectionClosed(session, closeStatus); }