From 526d89e1e61807faffdb3686e4243ffa4c2831f0 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 26 Nov 2019 12:04:00 +0000 Subject: [PATCH] Refine Throwable handling in spring-websocket This commit lowers the level of Throwable handling in parts of spring-websocket which now handle Exception instead and allow any Error to propagate. Closes gh-24075 --- .../jetty/JettyWebSocketHandlerAdapter.java | 14 +++++++------- .../standard/StandardWebSocketHandlerAdapter.java | 14 +++++++------- .../ExceptionWebSocketHandlerDecorator.java | 10 +++++----- .../server/support/AbstractHandshakeHandler.java | 4 ++-- .../server/support/HandshakeInterceptorChain.java | 2 +- .../support/WebSocketHttpRequestHandler.java | 2 +- .../sockjs/client/AbstractClientSockJsSession.java | 6 +++--- .../sockjs/client/RestTemplateXhrTransport.java | 2 +- .../web/socket/sockjs/client/SockJsClient.java | 4 ++-- .../sockjs/support/SockJsHttpRequestHandler.java | 4 ++-- .../transport/TransportHandlingSockJsService.java | 8 ++++---- .../AbstractHttpReceivingTransportHandler.java | 4 ++-- .../handler/WebSocketTransportHandler.java | 2 +- .../transport/session/AbstractSockJsSession.java | 4 ++-- .../session/WebSocketServerSockJsSession.java | 4 ++-- 15 files changed, 42 insertions(+), 42 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java index e819f60502..0c414a6b11 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -71,7 +71,7 @@ public class JettyWebSocketHandlerAdapter { this.wsSession.initializeNativeSession(session); this.webSocketHandler.afterConnectionEstablished(this.wsSession); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -82,7 +82,7 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleMessage(this.wsSession, message); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -93,7 +93,7 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleMessage(this.wsSession, message); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -106,7 +106,7 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleMessage(this.wsSession, message); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -118,7 +118,7 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.afterConnectionClosed(this.wsSession, closeStatus); } - catch (Throwable ex) { + catch (Exception ex) { if (logger.isWarnEnabled()) { logger.warn("Unhandled exception after connection closed for " + this, ex); } @@ -130,7 +130,7 @@ public class JettyWebSocketHandlerAdapter { try { this.webSocketHandler.handleTransportError(this.wsSession, cause); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java index 787511f18d..c4c11a30d9 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -103,7 +103,7 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.afterConnectionEstablished(this.wsSession); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -113,7 +113,7 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleMessage(this.wsSession, textMessage); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -123,7 +123,7 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleMessage(this.wsSession, binaryMessage); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -133,7 +133,7 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleMessage(this.wsSession, pongMessage); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } @@ -144,7 +144,7 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.afterConnectionClosed(this.wsSession, closeStatus); } - catch (Throwable ex) { + catch (Exception ex) { if (logger.isWarnEnabled()) { logger.warn("Unhandled on-close exception for " + this.wsSession, ex); } @@ -156,7 +156,7 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { try { this.handler.handleTransportError(this.wsSession, exception); } - catch (Throwable ex) { + catch (Exception ex) { ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, ex, logger); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java index 104e70371b..96d7a2d697 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/handler/ExceptionWebSocketHandlerDecorator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -47,7 +47,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato try { getDelegate().afterConnectionEstablished(session); } - catch (Throwable ex) { + catch (Exception ex) { tryCloseWithError(session, ex, logger); } } @@ -57,7 +57,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato try { getDelegate().handleMessage(session, message); } - catch (Throwable ex) { + catch (Exception ex) { tryCloseWithError(session, ex, logger); } } @@ -67,7 +67,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato try { getDelegate().handleTransportError(session, exception); } - catch (Throwable ex) { + catch (Exception ex) { tryCloseWithError(session, ex, logger); } } @@ -77,7 +77,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato try { getDelegate().afterConnectionClosed(session, closeStatus); } - catch (Throwable ex) { + catch (Exception ex) { if (logger.isWarnEnabled()) { logger.warn("Unhandled exception after connection closed for " + this, ex); } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java index b0d8828f8b..47384d84b4 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/AbstractHandshakeHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -157,7 +157,7 @@ public abstract class AbstractHandshakeHandler implements HandshakeHandler, Life Class clazz = ClassUtils.forName(className, AbstractHandshakeHandler.class.getClassLoader()); return (RequestUpgradeStrategy) ReflectionUtils.accessibleConstructor(clazz).newInstance(); } - catch (Throwable ex) { + catch (Exception ex) { throw new IllegalStateException( "Failed to instantiate RequestUpgradeStrategy: " + className, ex); } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/HandshakeInterceptorChain.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/HandshakeInterceptorChain.java index f853aa51f3..5484a52298 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/HandshakeInterceptorChain.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/HandshakeInterceptorChain.java @@ -77,7 +77,7 @@ public class HandshakeInterceptorChain { try { interceptor.afterHandshake(request, response, this.wsHandler, failure); } - catch (Throwable ex) { + catch (Exception ex) { if (logger.isWarnEnabled()) { logger.warn(interceptor + " threw exception in afterHandshake: " + ex); } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/WebSocketHttpRequestHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/WebSocketHttpRequestHandler.java index 279cf8f9f6..d4abeba3ac 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/WebSocketHttpRequestHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/WebSocketHttpRequestHandler.java @@ -171,7 +171,7 @@ public class WebSocketHttpRequestHandler implements HttpRequestHandler, Lifecycl catch (HandshakeFailureException ex) { failure = ex; } - catch (Throwable ex) { + catch (Exception ex) { failure = new HandshakeFailureException("Uncaught failure for request " + request.getURI(), ex); } finally { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/AbstractClientSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/AbstractClientSockJsSession.java index 974ee26804..03338f9bfa 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/AbstractClientSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/AbstractClientSockJsSession.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -244,7 +244,7 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { this.webSocketHandler.afterConnectionEstablished(this); this.connectFuture.set(this); } - catch (Throwable ex) { + catch (Exception ex) { if (logger.isErrorEnabled()) { logger.error("WebSocketHandler.afterConnectionEstablished threw exception in " + this, ex); } @@ -293,7 +293,7 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { try { this.webSocketHandler.handleMessage(this, new TextMessage(message)); } - catch (Throwable ex) { + catch (Exception ex) { logger.error("WebSocketHandler.handleMessage threw an exception on " + frame + " in " + this, ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/RestTemplateXhrTransport.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/RestTemplateXhrTransport.java index b8c7ff3620..ba8deed6e4 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/RestTemplateXhrTransport.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/RestTemplateXhrTransport.java @@ -118,7 +118,7 @@ public class RestTemplateXhrTransport extends AbstractXhrTransport { getRestTemplate().execute(receiveUrl, HttpMethod.POST, requestCallback, responseExtractor); requestCallback = requestCallbackAfterHandshake; } - catch (Throwable ex) { + catch (Exception ex) { if (!connectFuture.isDone()) { connectFuture.setException(ex); } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/SockJsClient.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/SockJsClient.java index f9eac2bd41..88d81e7158 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/SockJsClient.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/SockJsClient.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -260,7 +260,7 @@ public class SockJsClient implements WebSocketClient, Lifecycle { ServerInfo serverInfo = getServerInfo(sockJsUrlInfo, getHttpRequestHeaders(headers)); createRequest(sockJsUrlInfo, headers, serverInfo).connect(handler, connectFuture); } - catch (Throwable exception) { + catch (Exception exception) { if (logger.isErrorEnabled()) { logger.error("Initial SockJS \"Info\" request to server failed, url=" + url, exception); } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/SockJsHttpRequestHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/SockJsHttpRequestHandler.java index c158d3bb50..887864d570 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/SockJsHttpRequestHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/SockJsHttpRequestHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2019 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. @@ -133,7 +133,7 @@ public class SockJsHttpRequestHandler try { this.sockJsService.handleRequest(request, response, getSockJsPath(servletRequest), this.webSocketHandler); } - catch (Throwable ex) { + catch (Exception ex) { throw new SockJsException("Uncaught failure in SockJS request, uri=" + request.getURI(), ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java index fc13a2ef5a..40a1988107 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -215,10 +215,10 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem catch (HandshakeFailureException ex) { failure = ex; } - catch (Throwable ex) { + catch (Exception ex) { failure = new HandshakeFailureException("Uncaught failure for request " + request.getURI(), ex); } - finally { + finally { if (failure != null) { chain.applyAfterHandshake(request, response, failure); throw failure; @@ -316,7 +316,7 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem catch (SockJsException ex) { failure = ex; } - catch (Throwable ex) { + catch (Exception ex) { failure = new SockJsException("Uncaught failure for request " + request.getURI(), sessionId, ex); } finally { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java index 60e3963688..1e3e933dac 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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. @@ -72,7 +72,7 @@ public abstract class AbstractHttpReceivingTransportHandler extends AbstractTran } return; } - catch (Throwable ex) { + catch (Exception ex) { logger.error("Failed to read message", ex); handleReadError(response, "Failed to read message(s)", sockJsSession.getId()); return; diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java index 60423319dc..77e7bbe2f0 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java @@ -124,7 +124,7 @@ public class WebSocketTransportHandler extends AbstractTransportHandler wsHandler = new SockJsWebSocketHandler(getServiceConfig(), wsHandler, sockJsSession); this.handshakeHandler.doHandshake(request, response, wsHandler, sockJsSession.getAttributes()); } - catch (Throwable ex) { + catch (Exception ex) { sockJsSession.tryCloseWithSockJsTransportError(ex, CloseStatus.SERVER_ERROR); throw new SockJsTransportFailureException("WebSocket handshake failure", wsSession.getId(), ex); } 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 9115f64d9b..cc950ea487 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 @@ -324,7 +324,7 @@ public abstract class AbstractSockJsSession implements SockJsSession { try { writeFrameInternal(frame); } - catch (Throwable ex) { + catch (Exception ex) { logWriteFrameFailure(ex); try { // Force disconnect (so we won't try to send close frame) @@ -388,7 +388,7 @@ public abstract class AbstractSockJsSession implements SockJsSession { undelivered.remove(0); } } - catch (Throwable ex) { + catch (Exception ex) { throw new SockJsMessageDeliveryException(this.id, undelivered, ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/WebSocketServerSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/WebSocketServerSockJsSession.java index 35d73332b5..a8ed936327 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/WebSocketServerSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/WebSocketServerSockJsSession.java @@ -166,7 +166,7 @@ public class WebSocketServerSockJsSession extends AbstractSockJsSession implemen scheduleHeartbeat(); this.openFrameSent = true; } - catch (Throwable ex) { + catch (Exception ex) { tryCloseWithSockJsTransportError(ex, CloseStatus.SERVER_ERROR); } } @@ -186,7 +186,7 @@ public class WebSocketServerSockJsSession extends AbstractSockJsSession implemen try { messages = getSockJsServiceConfig().getMessageCodec().decode(payload); } - catch (Throwable ex) { + catch (Exception ex) { logger.error("Broken data received. Terminating WebSocket connection abruptly", ex); tryCloseWithSockJsTransportError(ex, CloseStatus.BAD_DATA); return; -- GitLab