fixed the issue with the way handshake failure was handled in web socket futures

s-arash · s-arash · commit 2cd731b514b3 · 2015-12-07T18:21:24.000+03:30
Fixed the issue where in creating web socket clients, if the handshake
failed, the failure wouldn't be reported to the callback (it would be
called with a null exception parameter).
diff --git a/AndroidAsync/src/com/koushikdutta/async/http/AsyncHttpClient.java b/AndroidAsync/src/com/koushikdutta/async/http/AsyncHttpClient.java
@@ -688,7 +688,8 @@ public void onConnectCompleted(Exception ex, AsyncHttpResponse response) {
                 }
                 WebSocket ws = WebSocketImpl.finishHandshake(req.getHeaders(), response);
                 if (ws == null) {
-                    if (!ret.setComplete(new WebSocketHandshakeException("Unable to complete websocket handshake")))
+                    ex = new WebSocketHandshakeException("Unable to complete websocket handshake");
+                    if (!ret.setComplete(ex))
                         return;
                 }
                 else {
diff --git a/AndroidAsync/test/src/com/koushikdutta/async/test/IssueWithWebSocketFuturesTests.java b/AndroidAsync/test/src/com/koushikdutta/async/test/IssueWithWebSocketFuturesTests.java
@@ -0,0 +1,59 @@
+package com.koushikdutta.async.test;
+
+import com.koushikdutta.async.future.Future;
+import com.koushikdutta.async.http.AsyncHttpClient;
+import com.koushikdutta.async.http.WebSocket;
+import com.koushikdutta.async.http.server.AsyncHttpServer;
+import com.koushikdutta.async.http.server.AsyncHttpServerRequest;
+
+import junit.framework.TestCase;
+
+import java.util.concurrent.CountDownLatch;
+
+
+public class IssueWithWebSocketFuturesTests extends TestCase {
+
+    //testing that websocket callback gets called with the correct parameters.
+    public void testWebSocketFutureWithHandshakeFailureCallback() throws Exception {
+
+        //creating a faulty server!
+        AsyncHttpServer httpServer = new AsyncHttpServer();
+        httpServer.websocket(".*", new AsyncHttpServer.WebSocketRequestCallback() {
+            @Override
+            public void onConnected(WebSocket webSocket, AsyncHttpServerRequest request) {
+
+            }
+        });
+        httpServer.listen(6666);
+
+
+
+        final Exception[] callbackException = {null};
+        final WebSocket[] callbackWs = {null};
+        final CountDownLatch countDownLatch = new CountDownLatch(1);
+
+
+        //for some reason, it fails with a WebSocketHandshakeException.
+        //But in general, if the handshake fails, the callback must be called with an exception.
+        Future<WebSocket> wsFuture = AsyncHttpClient.getDefaultInstance().websocket("ws://127.0.0.1:6666", "ws", new AsyncHttpClient.WebSocketConnectCallback() {
+            @Override
+            public void onCompleted(Exception ex, WebSocket webSocket) {
+                callbackException[0] = ex;
+                callbackWs[0] = webSocket;
+                countDownLatch.countDown();
+            }
+        });
+
+
+        //wait for the future to complete
+        countDownLatch.await();
+
+        //exactly one mut be null
+        assertTrue(callbackWs[0] == null ^ callbackException[0] == null);
+
+        //callback parameters must be the same as the future's result
+        assertEquals(wsFuture.tryGet(), callbackWs[0]);
+        assertEquals(wsFuture.tryGetException(), callbackException[0]);
+
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -688,7 +688,8 @@ public void onConnectCompleted(Exception ex, AsyncHttpResponse response) {`
`688`	`688`	`}`
`689`	`689`	`WebSocket ws = WebSocketImpl.finishHandshake(req.getHeaders(), response);`
`690`	`690`	`if (ws == null) {`
`691`		`- if (!ret.setComplete(new WebSocketHandshakeException("Unable to complete websocket handshake")))`
	`691`	`+ ex = new WebSocketHandshakeException("Unable to complete websocket handshake");`
	`692`	`+ if (!ret.setComplete(ex))`
`692`	`693`	`return;`
`693`	`694`	`}`
`694`	`695`	`else {`