Skip to content

Commit cceefc7

Browse files
BrennanConroyBrennanConroy
authored
[SignalR TS] Fix permanent Disconnecting state (#30948) (#31251)
1 parent 5e41934 commit cceefc7

File tree

5 files changed

+169
-9
lines changed

5 files changed

+169
-9
lines changed

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export class HttpConnection implements IConnection {
6161
private transport?: ITransport;
6262
private startInternalPromise?: Promise<void>;
6363
private stopPromise?: Promise<void>;
64-
private stopPromiseResolver!: (value?: PromiseLike<void>) => void;
64+
private stopPromiseResolver: (value?: PromiseLike<void>) => void = () => {};
6565
private stopError?: Error;
6666
private accessTokenFactory?: () => string | Promise<string>;
6767
private sendQueue?: TransportSendQueue;
@@ -208,7 +208,6 @@ export class HttpConnection implements IConnection {
208208
this.transport = undefined;
209209
} else {
210210
this.logger.log(LogLevel.Debug, "HttpConnection.transport is undefined in HttpConnection.stop() because start() failed.");
211-
this.stopConnection();
212211
}
213212
}
214213

@@ -288,6 +287,9 @@ export class HttpConnection implements IConnection {
288287
this.logger.log(LogLevel.Error, "Failed to start the connection: " + e);
289288
this.connectionState = ConnectionState.Disconnected;
290289
this.transport = undefined;
290+
291+
// if start fails, any active calls to stop assume that start will complete the stop promise
292+
this.stopPromiseResolver();
291293
return Promise.reject(e);
292294
}
293295
}

‎src/SignalR/clients/ts/signalr/src/HubConnection.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -762,7 +762,11 @@ export class HubConnection {
762762
this.logger.log(LogLevel.Information, `Reconnect attempt failed because of error '${e}'.`);
763763

764764
if (this.connectionState !== HubConnectionState.Reconnecting) {
765-
this.logger.log(LogLevel.Debug, "Connection left the reconnecting state during reconnect attempt. Done reconnecting.");
765+
this.logger.log(LogLevel.Debug, `Connection moved to the '${this.connectionState}' from the reconnecting state during reconnect attempt. Done reconnecting.`);
766+
// The TypeScript compiler thinks that connectionState must be Connected here. The TypeScript compiler is wrong.
767+
if (this.connectionState as any === HubConnectionState.Disconnecting) {
768+
this.completeClose();
769+
}
766770
return;
767771
}
768772

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

+13-6
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,30 @@ describe("HttpConnection", () => {
132132

133133
it("can stop a starting connection", async () => {
134134
await VerifyLogger.run(async (logger) => {
135+
const stoppingPromise = new PromiseSource();
136+
const startingPromise = new PromiseSource();
135137
const options: IHttpConnectionOptions = {
136138
...commonOptions,
137139
httpClient: new TestHttpClient()
138140
.on("POST", async () => {
139-
await connection.stop();
141+
startingPromise.resolve();
142+
await stoppingPromise;
140143
return "{}";
141-
})
142-
.on("GET", async () => {
143-
await connection.stop();
144-
return "";
145144
}),
146145
logger,
147146
} as IHttpConnectionOptions;
148147

149148
const connection = new HttpConnection("http://tempuri.org", options);
150149

151-
await expect(connection.start(TransferFormat.Text))
150+
const startPromise = connection.start(TransferFormat.Text);
151+
152+
await startingPromise;
153+
const stopPromise = connection.stop();
154+
stoppingPromise.resolve();
155+
156+
await stopPromise;
157+
158+
await expect(startPromise)
152159
.rejects
153160
.toThrow("The connection was stopped during negotiation.");
154161
},

‎src/SignalR/clients/ts/signalr/tests/HubConnection.Reconnect.test.ts

+93
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
import { DefaultReconnectPolicy } from "../src/DefaultReconnectPolicy";
5+
import { HttpConnection, INegotiateResponse } from "../src/HttpConnection";
56
import { HubConnection, HubConnectionState } from "../src/HubConnection";
7+
import { IHttpConnectionOptions } from "../src/IHttpConnectionOptions";
68
import { MessageType } from "../src/IHubProtocol";
79
import { RetryContext } from "../src/IRetryPolicy";
810
import { JsonHubProtocol } from "../src/JsonHubProtocol";
911

1012
import { VerifyLogger } from "./Common";
1113
import { TestConnection } from "./TestConnection";
14+
import { TestHttpClient } from "./TestHttpClient";
15+
import { TestEvent, TestMessageEvent, TestWebSocket } from "./TestWebSocket";
1216
import { PromiseSource } from "./Utils";
1317

1418
describe("auto reconnect", () => {
@@ -785,4 +789,93 @@ describe("auto reconnect", () => {
785789
}
786790
});
787791
});
792+
793+
it("can be stopped while restarting the underlying connection and negotiate throws", async () => {
794+
await VerifyLogger.run(async (logger) => {
795+
let onreconnectingCount = 0;
796+
let onreconnectedCount = 0;
797+
let closeCount = 0;
798+
799+
const nextRetryDelayCalledPromise = new PromiseSource();
800+
801+
const defaultConnectionId = "abc123";
802+
const defaultConnectionToken = "123abc";
803+
const defaultNegotiateResponse: INegotiateResponse = {
804+
availableTransports: [
805+
{ transport: "WebSockets", transferFormats: ["Text", "Binary"] },
806+
{ transport: "ServerSentEvents", transferFormats: ["Text"] },
807+
{ transport: "LongPolling", transferFormats: ["Text", "Binary"] },
808+
],
809+
connectionId: defaultConnectionId,
810+
connectionToken: defaultConnectionToken,
811+
negotiateVersion: 1,
812+
};
813+
814+
const startStarted = new PromiseSource();
815+
let negotiateCount = 0;
816+
817+
const options: IHttpConnectionOptions = {
818+
WebSocket: TestWebSocket,
819+
httpClient: new TestHttpClient()
820+
.on("POST", async () => {
821+
++negotiateCount;
822+
if (negotiateCount === 1) {
823+
return defaultNegotiateResponse;
824+
}
825+
startStarted.resolve();
826+
return Promise.reject("Error with negotiate");
827+
})
828+
.on("GET", () => ""),
829+
logger,
830+
} as IHttpConnectionOptions;
831+
832+
const connection = new HttpConnection("http://tempuri.org", options);
833+
const hubConnection = HubConnection.create(connection, logger, new JsonHubProtocol(), {
834+
nextRetryDelayInMilliseconds() {
835+
nextRetryDelayCalledPromise.resolve();
836+
return 0;
837+
},
838+
});
839+
840+
hubConnection.onreconnecting(() => {
841+
onreconnectingCount++;
842+
});
843+
844+
hubConnection.onreconnected(() => {
845+
onreconnectedCount++;
846+
});
847+
848+
hubConnection.onclose(() => {
849+
closeCount++;
850+
});
851+
852+
TestWebSocket.webSocketSet = new PromiseSource();
853+
const startPromise = hubConnection.start();
854+
await TestWebSocket.webSocketSet;
855+
await TestWebSocket.webSocket.openSet;
856+
TestWebSocket.webSocket.onopen(new TestEvent());
857+
TestWebSocket.webSocket.onmessage(new TestMessageEvent("{}\x1e"));
858+
859+
await startPromise;
860+
TestWebSocket.webSocket.close();
861+
TestWebSocket.webSocketSet = new PromiseSource();
862+
863+
await nextRetryDelayCalledPromise;
864+
865+
expect(hubConnection.state).toBe(HubConnectionState.Reconnecting);
866+
expect(onreconnectingCount).toBe(1);
867+
expect(onreconnectedCount).toBe(0);
868+
expect(closeCount).toBe(0);
869+
870+
await startStarted;
871+
await hubConnection.stop();
872+
873+
expect(hubConnection.state).toBe(HubConnectionState.Disconnected);
874+
expect(onreconnectingCount).toBe(1);
875+
expect(onreconnectedCount).toBe(0);
876+
expect(closeCount).toBe(1);
877+
},
878+
"Failed to complete negotiation with the server: Error with negotiate",
879+
"Failed to start the connection: Error with negotiate");
880+
});
788881
});

‎src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts

+54
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,57 @@ export class TestCloseEvent {
219219
public CAPTURING_PHASE: number = 0;
220220
public NONE: number = 0;
221221
}
222+
223+
export class TestMessageEvent implements MessageEvent {
224+
constructor(data: any) {
225+
this.data = data;
226+
}
227+
public data: any;
228+
public lastEventId: string = "";
229+
public origin: string = "";
230+
public ports: MessagePort[] = [];
231+
public source: Window | null = null;
232+
public composed: boolean = false;
233+
public composedPath(): EventTarget[];
234+
public composedPath(): any[] {
235+
throw new Error("Method not implemented.");
236+
}
237+
public code: number = 0;
238+
public reason: string = "";
239+
public wasClean: boolean = false;
240+
public initMessageEvent(typeArg: string, canBubbleArg: boolean, cancelableArg: boolean, data: any, origin: string, lastEventId: string): void {
241+
throw new Error("Method not implemented.");
242+
}
243+
public bubbles: boolean = false;
244+
public cancelBubble: boolean = false;
245+
public cancelable: boolean = false;
246+
public currentTarget!: EventTarget;
247+
public defaultPrevented: boolean = false;
248+
public eventPhase: number = 0;
249+
public isTrusted: boolean = false;
250+
public returnValue: boolean = false;
251+
public scoped: boolean = false;
252+
public srcElement!: Element | null;
253+
public target!: EventTarget;
254+
public timeStamp: number = 0;
255+
public type: string = "";
256+
public deepPath(): EventTarget[] {
257+
throw new Error("Method not implemented.");
258+
}
259+
public initEvent(type: string, bubbles?: boolean | undefined, cancelable?: boolean | undefined): void {
260+
throw new Error("Method not implemented.");
261+
}
262+
public preventDefault(): void {
263+
throw new Error("Method not implemented.");
264+
}
265+
public stopImmediatePropagation(): void {
266+
throw new Error("Method not implemented.");
267+
}
268+
public stopPropagation(): void {
269+
throw new Error("Method not implemented.");
270+
}
271+
public AT_TARGET: number = 0;
272+
public BUBBLING_PHASE: number = 0;
273+
public CAPTURING_PHASE: number = 0;
274+
public NONE: number = 0;
275+
}

0 commit comments

Comments
 (0)