Fix a couple things in Typescript client (#13863)

Author: BrennanConroy

src/SignalR/clients/ts/signalr/src/HttpConnection.ts

@@ -322,7 +322,7 @@ export class HttpConnection implements IConnection {
             });
 
             if (response.statusCode !== 200) {
-                return Promise.reject(new Error(`Unexpected status code returned from negotiate ${response.statusCode}`));
+                return Promise.reject(new Error(`Unexpected status code returned from negotiate '${response.statusCode}'`));
             }
 
             const negotiateResponse = JSON.parse(response.content as string) as INegotiateResponse;
@@ -475,8 +475,8 @@ export class HttpConnection implements IConnection {
         }
 
         if (this.connectionState === ConnectionState.Connecting) {
-            this.logger.log(LogLevel.Warning, `Call to HttpConnection.stopConnection(${error}) was ignored because the connection hasn't yet left the in the connecting state.`);
-            return;
+            this.logger.log(LogLevel.Warning, `Call to HttpConnection.stopConnection(${error}) was ignored because the connection is still in the connecting state.`);
+            throw new Error(`HttpConnection.stopConnection(${error}) was called while the connection is still in the connecting state.`);
         }
 
         if (this.connectionState === ConnectionState.Disconnecting) {
@@ -626,7 +626,7 @@ export class TransportSendQueue {
             offset += item.byteLength;
         }
 
-        return result;
+        return result.buffer;
     }
 }
 

src/SignalR/clients/ts/signalr/src/WebSocketTransport.ts

@@ -105,7 +105,11 @@ export class WebSocketTransport implements ITransport {
                 }
             };
 
-            webSocket.onclose = (event: CloseEvent) => this.close(event);
+            webSocket.onclose = (event: CloseEvent) => {
+                if (this.webSocket) {
+                    this.close(event);
+                }
+            };
         });
     }
 

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

@@ -8,6 +8,7 @@ import { HttpTransportType, ITransport, TransferFormat } from "../src/ITransport
 import { getUserAgentHeader } from "../src/Utils";
 
 import { HttpError } from "../src/Errors";
+import { ILogger, LogLevel } from "../src/ILogger";
 import { NullLogger } from "../src/Loggers";
 import { EventSourceConstructor, WebSocketConstructor } from "../src/Polyfills";
 
@@ -192,9 +193,9 @@ describe("HttpConnection", () => {
             const connection = new HttpConnection("http://tempuri.org", options);
             await expect(connection.start(TransferFormat.Text))
                 .rejects
-                .toThrow("Unexpected status code returned from negotiate 999");
+                .toThrow("Unexpected status code returned from negotiate '999'");
         },
-        "Failed to start the connection: Error: Unexpected status code returned from negotiate 999");
+        "Failed to start the connection: Error: Unexpected status code returned from negotiate '999'");
     });
 
     it("all transport failure errors get aggregated", async () => {
@@ -1151,6 +1152,53 @@ describe("HttpConnection", () => {
         }, "Failed to start the connection: Error: nope");
     });
 
+    it("logMessageContent displays correctly with binary data", async () => {
+        await VerifyLogger.run(async (logger) => {
+            const availableTransport = { transport: "LongPolling", transferFormats: ["Text", "Binary"] };
+
+            let sentMessage = "";
+            const captureLogger: ILogger = {
+                log: (logLevel: LogLevel, message: string) => {
+                    if (logLevel === LogLevel.Trace && message.search("data of length") > 0) {
+                        sentMessage = message;
+                    }
+
+                    logger.log(logLevel, message);
+                },
+            };
+
+            let httpClientGetCount = 0;
+            const options: IHttpConnectionOptions = {
+                ...commonOptions,
+                httpClient: new TestHttpClient()
+                    .on("POST", () => ({ connectionId: "42", availableTransports: [availableTransport] }))
+                    .on("GET", () => {
+                        httpClientGetCount++;
+                        if (httpClientGetCount === 1) {
+                            // First long polling request must succeed so start completes
+                            return "";
+                        }
+                        return Promise.resolve();
+                    })
+                    .on("DELETE", () => new HttpResponse(202)),
+                logMessageContent: true,
+                logger: captureLogger,
+                transport: HttpTransportType.LongPolling,
+            } as IHttpConnectionOptions;
+
+            const connection = new HttpConnection("http://tempuri.org", options);
+            connection.onreceive = () => null;
+            try {
+                await connection.start(TransferFormat.Binary);
+                await connection.send(new Uint8Array([0x68, 0x69, 0x20, 0x3a, 0x29]));
+            } finally {
+                await connection.stop();
+            }
+
+            expect(sentMessage).toBe("(LongPolling transport) sending data. Binary data of length 5. Content: '0x68 0x69 0x20 0x3a 0x29'.");
+        });
+    });
+
     describe(".constructor", () => {
         it("throws if no Url is provided", async () => {
             // Force TypeScript to let us call the constructor incorrectly :)
@@ -1413,7 +1461,7 @@ describe("TransportSendQueue", () => {
 
         const queue = new TransportSendQueue(transport);
 
-        const first = queue.send(new Uint8Array([4, 5, 6]));
+        const first = queue.send(new Uint8Array([4, 5, 6]).buffer);
         // This should allow first to enter transport.send
         promiseSource1.resolve();
         // Wait until we're inside transport.send
@@ -1428,8 +1476,8 @@ describe("TransportSendQueue", () => {
         await Promise.all([first, second, third]);
 
         expect(sendMock.mock.calls.length).toBe(2);
-        expect(sendMock.mock.calls[0][0]).toEqual(new Uint8Array([4, 5, 6]));
-        expect(sendMock.mock.calls[1][0]).toEqual(new Uint8Array([7, 8, 10, 12, 14]));
+        expect(sendMock.mock.calls[0][0]).toEqual(new Uint8Array([4, 5, 6]).buffer);
+        expect(sendMock.mock.calls[1][0]).toEqual(new Uint8Array([7, 8, 10, 12, 14]).buffer);
 
         await queue.stop();
     });

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

@@ -259,6 +259,36 @@ describe("WebSocketTransport", () => {
                 .toBe("WebSocket is not in the OPEN state");
         });
     });
+
+    it("does not run onclose callback if Transport does not fully connect and exits", async () => {
+        await VerifyLogger.run(async (logger) => {
+            (global as any).ErrorEvent = TestErrorEvent;
+            const webSocket = new WebSocketTransport(new TestHttpClient(), undefined, logger, true, TestWebSocket);
+
+            const connectPromise = webSocket.connect("http://example.com", TransferFormat.Text);
+
+            await TestWebSocket.webSocket.closeSet;
+
+            let closeCalled: boolean = false;
+            let error: Error;
+            webSocket.onclose = (e) => {
+                closeCalled = true;
+                error = e!;
+            };
+
+            const message = new TestCloseEvent();
+            message.wasClean = false;
+            message.code = 1;
+            message.reason = "just cause";
+            TestWebSocket.webSocket.onclose(message);
+
+            expect(closeCalled).toBe(false);
+            expect(error!).toBeUndefined();
+
+            TestWebSocket.webSocket.onerror(new TestEvent());
+            await expect(connectPromise).rejects.toThrow("There was an error with the transport.");
+        });
+    });
 });
 
 async function createAndStartWebSocket(logger: ILogger, url?: string, accessTokenFactory?: (() => string | Promise<string>), format?: TransferFormat): Promise<WebSocketTransport> {