From a04176979908e0838f98599b9f1ef9f67dfb3d2b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 25 Jan 2018 12:24:46 +0100 Subject: [PATCH] AbstractClientSockJsSession.close propagates IOException from disconnect Issue: SPR-16415 (cherry picked from commit cf100d4) --- .../client/AbstractClientSockJsSession.java | 48 ++++++++++++------- .../client/ClientSockJsSessionTests.java | 6 ++- 2 files changed, 35 insertions(+), 19 deletions(-) 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 6f9e37bdb7d..73befdeb7a1 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-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -118,7 +118,14 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { return new Runnable() { @Override public void run() { - closeInternal(new CloseStatus(2007, "Transport timed out")); + try { + closeInternal(new CloseStatus(2007, "Transport timed out")); + } + catch (Throwable ex) { + if (logger.isWarnEnabled()) { + logger.warn("Failed to close " + this + " after transport timeout", ex); + } + } } }; } @@ -160,8 +167,10 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { } @Override - public final void close(CloseStatus status) { - Assert.isTrue(status != null && isUserSetStatus(status), "Invalid close status: " + status); + public final void close(CloseStatus status) throws IOException { + if (!isUserSetStatus(status)) { + throw new IllegalArgumentException("Invalid close status: " + status); + } if (logger.isDebugEnabled()) { logger.debug("Closing session with " + status + " in " + this); } @@ -169,10 +178,22 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { } private boolean isUserSetStatus(CloseStatus status) { - return (status.getCode() == 1000 || (status.getCode() >= 3000 && status.getCode() <= 4999)); + return (status != null && (status.getCode() == 1000 || + (status.getCode() >= 3000 && status.getCode() <= 4999))); + } + + private void silentClose(CloseStatus status) { + try { + closeInternal(status); + } + catch (Throwable ex) { + if (logger.isWarnEnabled()) { + logger.warn("Failed to close " + this, ex); + } + } } - protected void closeInternal(CloseStatus status) { + protected void closeInternal(CloseStatus status) throws IOException { if (this.state == null) { logger.warn("Ignoring close since connect() was never invoked"); return; @@ -186,14 +207,7 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { this.state = State.CLOSING; this.closeStatus = status; - try { - disconnect(status); - } - catch (Throwable ex) { - if (logger.isErrorEnabled()) { - logger.error("Failed to close " + this, ex); - } - } + disconnect(status); } protected abstract void disconnect(CloseStatus status) throws IOException; @@ -238,7 +252,7 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { logger.debug("Open frame received in " + getId() + " but we're not connecting (current state " + this.state + "). The server might have been restarted and lost track of the session."); } - closeInternal(new CloseStatus(1006, "Server lost session")); + silentClose(new CloseStatus(1006, "Server lost session")); } } @@ -258,7 +272,7 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { if (logger.isErrorEnabled()) { logger.error("Failed to decode data for SockJS \"message\" frame: " + frame + " in " + this, ex); } - closeInternal(CloseStatus.BAD_DATA); + silentClose(CloseStatus.BAD_DATA); return; } @@ -293,7 +307,7 @@ public abstract class AbstractClientSockJsSession implements WebSocketSession { logger.error("Failed to decode data for " + frame + " in " + this, ex); } } - closeInternal(closeStatus); + silentClose(closeStatus); } public void handleTransportError(Throwable error) { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/ClientSockJsSessionTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/ClientSockJsSessionTests.java index 791aa387af5..3617da5672a 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/ClientSockJsSessionTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/client/ClientSockJsSessionTests.java @@ -129,8 +129,10 @@ public class ClientSockJsSessionTests { @Test public void handleFrameMessageWithWebSocketHandlerException() throws Exception { this.session.handleFrame(SockJsFrame.openFrame().getContent()); - willThrow(new IllegalStateException("Fake error")).given(this.handler).handleMessage(this.session, new TextMessage("foo")); - willThrow(new IllegalStateException("Fake error")).given(this.handler).handleMessage(this.session, new TextMessage("bar")); + willThrow(new IllegalStateException("Fake error")).given(this.handler) + .handleMessage(this.session, new TextMessage("foo")); + willThrow(new IllegalStateException("Fake error")).given(this.handler) + .handleMessage(this.session, new TextMessage("bar")); this.session.handleFrame(SockJsFrame.messageFrame(CODEC, "foo", "bar").getContent()); assertThat(this.session.isOpen(), equalTo(true)); verify(this.handler).afterConnectionEstablished(this.session);