Set transport handlers earlier in Typescript client (#12524)

Author: BrennanConroy

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

@@ -230,7 +230,7 @@ export class HttpConnection implements IConnection {
                     this.transport = this.constructTransport(HttpTransportType.WebSockets);
                     // We should just call connect directly in this case.
                     // No fallback or negotiate in this case.
-                    await this.transport!.connect(url, transferFormat);
+                    await this.startTransport(url, transferFormat);
                 } else {
                     throw new Error("Negotiation can only be skipped when using the WebSocket transport directly.");
                 }
@@ -281,9 +281,6 @@ export class HttpConnection implements IConnection {
                 this.features.inherentKeepAlive = true;
             }
 
-            this.transport!.onreceive = this.onreceive;
-            this.transport!.onclose = (e) => this.stopConnection(e);
-
             if (this.connectionState === ConnectionState.Connecting) {
                 // Ensure the connection transitions to the connected state prior to completing this.startInternalPromise.
                 // start() will handle the case when stop was called and startInternal exits still in the disconnecting state.
@@ -344,7 +341,7 @@ export class HttpConnection implements IConnection {
         if (this.isITransport(requestedTransport)) {
             this.logger.log(LogLevel.Debug, "Connection was provided an instance of ITransport, using that directly.");
             this.transport = requestedTransport;
-            await this.transport.connect(connectUrl, requestedTransferFormat);
+            await this.startTransport(connectUrl, requestedTransferFormat);
 
             return;
         }
@@ -367,7 +364,7 @@ export class HttpConnection implements IConnection {
                     connectUrl = this.createConnectUrl(url, negotiateResponse.connectionId);
                 }
                 try {
-                    await this.transport!.connect(connectUrl, requestedTransferFormat);
+                    await this.startTransport(connectUrl, requestedTransferFormat);
                     return;
                 } catch (ex) {
                     this.logger.log(LogLevel.Error, `Failed to start the transport '${endpoint.transport}': ${ex}`);
@@ -408,6 +405,12 @@ export class HttpConnection implements IConnection {
         }
     }
 
+    private startTransport(url: string, transferFormat: TransferFormat): Promise<void> {
+        this.transport!.onreceive = this.onreceive;
+        this.transport!.onclose = (e) => this.stopConnection(e);
+        return this.transport!.connect(url, transferFormat);
+    }
+
     private resolveTransportOrError(endpoint: IAvailableTransport, requestedTransport: HttpTransportType | undefined, requestedTransferFormat: TransferFormat): ITransport | Error {
         const transport = HttpTransportType[endpoint.transport];
         if (transport === null || transport === undefined) {

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

@@ -795,6 +795,84 @@ describe("HttpConnection", () => {
         });
     });
 
+    it("transport handlers set before start", async () => {
+        await VerifyLogger.run(async (logger) => {
+            const availableTransport = { transport: "LongPolling", transferFormats: ["Text"] };
+            let handlersSet = false;
+
+            let httpClientGetCount = 0;
+            const options: IHttpConnectionOptions = {
+                ...commonOptions,
+                httpClient: new TestHttpClient()
+                    .on("POST", () => ({ connectionId: "42", availableTransports: [availableTransport] }))
+                    .on("GET", () => {
+                        httpClientGetCount++;
+                        if (httpClientGetCount === 1) {
+                            if ((connection as any).transport.onreceive && (connection as any).transport.onclose) {
+                                handlersSet = true;
+                            }
+                            // First long polling request must succeed so start completes
+                            return "";
+                        }
+                    })
+                    .on("DELETE", () => new HttpResponse(202)),
+                logger,
+            } as IHttpConnectionOptions;
+
+            const connection = new HttpConnection("http://tempuri.org", options);
+            connection.onreceive = () => null;
+            try {
+                await connection.start(TransferFormat.Text);
+            } finally {
+                await connection.stop();
+            }
+
+            expect(handlersSet).toBe(true);
+        });
+    });
+
+    it("transport handlers set before start for custom transports", async () => {
+        await VerifyLogger.run(async (logger) => {
+            const availableTransport = { transport: "Custom", transferFormats: ["Text"] };
+            let handlersSet = false;
+            const transport: ITransport = {
+                connect: (url: string, transferFormat: TransferFormat) => {
+                    if (transport.onreceive && transport.onclose) {
+                        handlersSet = true;
+                    }
+                    return Promise.resolve();
+                },
+                onclose: null,
+                onreceive: null,
+                send: (data: any) => Promise.resolve(),
+                stop: () => {
+                    if (transport.onclose) {
+                        transport.onclose();
+                    }
+                    return Promise.resolve();
+                },
+            };
+
+            const options: IHttpConnectionOptions = {
+                ...commonOptions,
+                httpClient: new TestHttpClient()
+                    .on("POST", () => ({ connectionId: "42", availableTransports: [availableTransport] })),
+                logger,
+                transport,
+            } as IHttpConnectionOptions;
+
+            const connection = new HttpConnection("http://tempuri.org", options);
+            connection.onreceive = () => null;
+            try {
+                await connection.start(TransferFormat.Text);
+            } finally {
+                await connection.stop();
+            }
+
+            expect(handlersSet).toBe(true);
+        });
+    });
+
     describe(".constructor", () => {
         it("throws if no Url is provided", async () => {
             // Force TypeScript to let us call the constructor incorrectly :)