From a1e77a2c090e62bc3be3383c34d521449d34e74c Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 26 Jul 2019 21:54:07 -0700 Subject: [PATCH] Set transport handlers earlier in Typescript client (#12524) --- .../clients/ts/signalr/src/HttpConnection.ts | 15 ++-- .../ts/signalr/tests/HttpConnection.test.ts | 78 +++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts index 516d935672..5bd423124b 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts +++ b/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 { + 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) { diff --git a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts index b907276fe9..d720011334 100644 --- a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts +++ b/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 :)