[TS] Reject handshake if connection closes before response (#3000)

clients/ts/common/package.json

@@ -10,10 +10,10 @@
   "author": "",
   "license": "ISC",
   "devDependencies": {
-    "@types/jest": "^22.2.3",
+    "@types/jest": "^23.3.2",
     "@types/node": "^8.5.2",
     "@types/webpack": "^4.4.0",
-    "jest": "^22.4.3",
+    "jest": "^23.6.0",
     "jest-junit": "^5.1.0",
     "rimraf": "^2.6.2",
     "ts-jest": "^22.4.6",

clients/ts/signalr/src/HubConnection.ts

@@ -470,6 +470,12 @@ export class HubConnection {
 
         this.connectionState = HubConnectionState.Disconnected;
 
+        // if handshake is in progress start will be waiting for the handshake promise, so we complete it
+        // if it has already completed this should just noop
+        if (this.handshakeRejecter) {
+            this.handshakeRejecter(error);
+        }
+
         Object.keys(callbacks)
             .forEach((key) => {
                 const callback = callbacks[key];

clients/ts/signalr/tests/HttpConnection.test.ts

@@ -57,7 +57,7 @@ describe("HttpConnection", () => {
 
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toThrow("error");
+                .toBe("error");
         },
         "Failed to start the connection: error",
         "Failed to complete negotiation with the server: error");
@@ -118,11 +118,11 @@ describe("HttpConnection", () => {
 
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toThrow("reached negotiate");
+                .toBe("reached negotiate");
 
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toThrow("reached negotiate");
+                .toBe("reached negotiate");
         },
         "Failed to complete negotiation with the server: reached negotiate",
         "Failed to start the connection: reached negotiate");

clients/ts/signalr/tests/HubConnection.test.ts

@@ -273,6 +273,27 @@ describe("HubConnection", () => {
             });
         });
 
+        it.only("start completes if connection closes and handshake not received yet", async () => {
+            await VerifyLogger.run(async (logger) => {
+                const mockProtocol = new TestProtocol(TransferFormat.Text);
+
+                const connection = new TestConnection(false);
+                const hubConnection = createHubConnection(connection, logger, mockProtocol);
+                try {
+                    let startCompleted = false;
+                    const startPromise = hubConnection.start().then(() => startCompleted = true);
+                    expect(startCompleted).toBe(false);
+
+                    await connection.stop();
+                    try {
+                        await startPromise;
+                    } catch { }
+                } finally {
+                    await hubConnection.stop();
+                }
+            });
+        });
+
         it("rejects the promise when an error is received", async () => {
             await VerifyLogger.run(async (logger) => {
                 const connection = new TestConnection();
@@ -549,7 +570,7 @@ describe("HubConnection", () => {
                     }
                     await expect(startPromise)
                         .rejects
-                        .toThrow("Server returned handshake error: Error!");
+                        .toBe("Server returned handshake error: Error!");
 
                     expect(closeError!.message).toEqual("Server returned handshake error: Error!");
                 } finally {

clients/ts/signalr/tests/HubConnectionBuilder.test.ts

@@ -66,14 +66,15 @@ describe("HubConnectionBuilder", () => {
             // Start the connection
             const closed = makeClosedPromise(connection);
 
-            // start waits for handshake before returning, we don't care in this test
+            const startPromise = connection.start();
-            // tslint:disable-next-line:no-floating-promises
-            connection.start();
 
             const pollRequest = await pollSent.promise;
             expect(pollRequest.url).toMatch(/http:\/\/example.com\?id=abc123.*/);
 
             await closed;
+            try {
+                await startPromise;
+            } catch { }
         });
     });
 
@@ -113,14 +114,15 @@ describe("HubConnectionBuilder", () => {
             // Start the connection
             const closed = makeClosedPromise(connection);
 
-            // start waits for handshake before returning, we don't care in this test
+            const startPromise = connection.start();
-            // tslint:disable-next-line:no-floating-promises
-            connection.start();
 
             const negotiateRequest = await negotiateReceived.promise;
             expect(negotiateRequest.content).toBe(`{"protocol":"${protocol.name}","version":1}\x1E`);
 
             await closed;
+            try {
+                await startPromise;
+            } catch { }
         });
     });
 
@@ -131,8 +133,19 @@ describe("HubConnectionBuilder", () => {
                 loggedMessages += 1;
             },
         };
+        const pollSent = new PromiseSource<HttpRequest>();
+        const pollCompleted = new PromiseSource<HttpResponse>();
+        const testClient = createTestClient(pollSent, pollCompleted.promise)
+            .on("POST", "http://example.com?id=abc123", (req) => {
+                // Respond from the poll with the handshake response
+                pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
+                return new HttpResponse(202);
+            });
         const connection = createConnectionBuilder(logger)
-            .withUrl("http://example.com")
+            .withUrl("http://example.com", {
+                ...commonHttpOptions,
+                httpClient: testClient,
+            })
             .build();
 
         try {
@@ -145,9 +158,20 @@ describe("HubConnectionBuilder", () => {
     });
 
     it("uses logger for both HttpConnection and HubConnection", async () => {
+        const pollSent = new PromiseSource<HttpRequest>();
+        const pollCompleted = new PromiseSource<HttpResponse>();
+        const testClient = createTestClient(pollSent, pollCompleted.promise)
+            .on("POST", "http://example.com?id=abc123", (req) => {
+                // Respond from the poll with the handshake response
+                pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
+                return new HttpResponse(202);
+            });
         const logger = new CaptureLogger();
         const connection = createConnectionBuilder(logger)
-            .withUrl("http://example.com")
+            .withUrl("http://example.com", {
+                ...commonHttpOptions,
+                httpClient: testClient,
+            })
             .build();
 
         try {
@@ -164,10 +188,21 @@ describe("HubConnectionBuilder", () => {
     });
 
     it("does not replace HttpConnectionOptions logger if provided", async () => {
+        const pollSent = new PromiseSource<HttpRequest>();
+        const pollCompleted = new PromiseSource<HttpResponse>();
+        const testClient = createTestClient(pollSent, pollCompleted.promise)
+            .on("POST", "http://example.com?id=abc123", (req) => {
+                // Respond from the poll with the handshake response
+                pollCompleted.resolve(new HttpResponse(204, "No Content", "{}"));
+                return new HttpResponse(202);
+            });
         const hubConnectionLogger = new CaptureLogger();
         const httpConnectionLogger = new CaptureLogger();
         const connection = createConnectionBuilder(hubConnectionLogger)
-            .withUrl("http://example.com", { logger: httpConnectionLogger })
+            .withUrl("http://example.com", {
+                httpClient: testClient,
+                logger: httpConnectionLogger,
+            })
             .build();
 
         try {

clients/ts/signalr/tests/WebSocketTransport.test.ts

@@ -132,7 +132,7 @@ describe("WebSocketTransport", () => {
 
             await expect(webSocket.send(""))
                 .rejects
-                .toThrow("WebSocket is not in the OPEN state");
+                .toBe("WebSocket is not in the OPEN state");
         });
     });
 
@@ -159,7 +159,7 @@ describe("WebSocketTransport", () => {
 
             await expect(webSocket.send(""))
                 .rejects
-                .toThrow("WebSocket is not in the OPEN state");
+                .toBe("WebSocket is not in the OPEN state");
         });
     });
 
@@ -182,7 +182,7 @@ describe("WebSocketTransport", () => {
 
             await expect(webSocket.send(""))
                 .rejects
-                .toThrow("WebSocket is not in the OPEN state");
+                .toBe("WebSocket is not in the OPEN state");
         });
     });