AbstractClientSockJsSession.close propagates IOException from disconnect

Issue: SPR-16415

(cherry picked from commit cf100d4)

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)));
 	}
 
-	protected void closeInternal(CloseStatus status) {
+	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) 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) {

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);