Skip to content

Commit 00c3e7c

Browse files
BrennanConroyBrennanConroy
authored
[SignalR TS] Fix permanent Disconnecting state (#30948) (#31253)
1 parent 9facd9b commit 00c3e7c

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
@@ -51,7 +51,7 @@ export class HttpConnection implements IConnection {
5151
private transport?: ITransport;
5252
private startInternalPromise?: Promise<void>;
5353
private stopPromise?: Promise<void>;
54-
private stopPromiseResolver!: (value?: PromiseLike<void>) => void;
54+
private stopPromiseResolver: (value?: PromiseLike<void>) => void = () => {};
5555
private stopError?: Error;
5656
private accessTokenFactory?: () => string | Promise<string>;
5757
private sendQueue?: TransportSendQueue;
@@ -214,7 +214,6 @@ export class HttpConnection implements IConnection {
214214
this.transport = undefined;
215215
} else {
216216
this.logger.log(LogLevel.Debug, "HttpConnection.transport is undefined in HttpConnection.stop() because start() failed.");
217-
this.stopConnection();
218217
}
219218
}
220219

@@ -294,6 +293,9 @@ export class HttpConnection implements IConnection {
294293
this.logger.log(LogLevel.Error, "Failed to start the connection: " + e);
295294
this.connectionState = ConnectionState.Disconnected;
296295
this.transport = undefined;
296+
297+
// if start fails, any active calls to stop assume that start will complete the stop promise
298+
this.stopPromiseResolver();
297299
return Promise.reject(e);
298300
}
299301
}

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

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

768768
if (this.connectionState !== HubConnectionState.Reconnecting) {
769-
this.logger.log(LogLevel.Debug, "Connection left the reconnecting state during reconnect attempt. Done reconnecting.");
769+
this.logger.log(LogLevel.Debug, `Connection moved to the '${this.connectionState}' from the reconnecting state during reconnect attempt. Done reconnecting.`);
770+
// The TypeScript compiler thinks that connectionState must be Connected here. The TypeScript compiler is wrong.
771+
if (this.connectionState as any === HubConnectionState.Disconnecting) {
772+
this.completeClose();
773+
}
770774
return;
771775
}
772776

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

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

135135
it("can stop a starting connection", async () => {
136136
await VerifyLogger.run(async (logger) => {
137+
const stoppingPromise = new PromiseSource();
138+
const startingPromise = new PromiseSource();
137139
const options: IHttpConnectionOptions = {
138140
...commonOptions,
139141
httpClient: new TestHttpClient()
140142
.on("POST", async () => {
141-
await connection.stop();
143+
startingPromise.resolve();
144+
await stoppingPromise;
142145
return "{}";
143-
})
144-
.on("GET", async () => {
145-
await connection.stop();
146-
return "";
147146
}),
148147
logger,
149148
} as IHttpConnectionOptions;
150149

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

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

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

0 commit comments

Comments
 (0)