Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SignalR TS] Set keep alive timer in receive loop #31300

Merged
merged 2 commits into from
Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions src/SignalR/clients/ts/signalr/src/HubConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export class HubConnection {
private _connectionStarted: boolean;
private _startPromise?: Promise<void>;
private _stopPromise?: Promise<void>;
private _nextKeepAlive: number = 0;

// The type of these a) doesn't matter and b) varies when building in browser and node contexts
// Since we're building the WebPack bundle directly from the TypeScript, this matters (previously
Expand All @@ -74,6 +75,8 @@ export class HubConnection {
*
* The default value is 15,000 milliseconds (15 seconds).
* Allows the server to detect hard disconnects (like when a client unplugs their computer).
* The ping will happen at most as often as the server pings.
* If the server pings every 5 seconds, a value lower than 5 will ping every 5 seconds.
*/
public keepAliveIntervalInMilliseconds: number;

Expand Down Expand Up @@ -604,24 +607,39 @@ export class HubConnection {
return;
}

// Set the time we want the next keep alive to be sent
// Timer will be setup on next message receive
this._nextKeepAlive = new Date().getTime() + this.keepAliveIntervalInMilliseconds;

this._cleanupPingTimer();
this._pingServerHandle = setTimeout(async () => {
if (this._connectionState === HubConnectionState.Connected) {
try {
await this._sendMessage(this._cachedPingMessage);
} catch {
// We don't care about the error. It should be seen elsewhere in the client.
// The connection is probably in a bad or closed state now, cleanup the timer so it stops triggering
this._cleanupPingTimer();
}
}
}, this.keepAliveIntervalInMilliseconds);
}

private _resetTimeoutPeriod() {
if (!this.connection.features || !this.connection.features.inherentKeepAlive) {
// Set the timeout timer
this._timeoutHandle = setTimeout(() => this.serverTimeout(), this.serverTimeoutInMilliseconds);

// Set keepAlive timer if there isn't one
if (this._pingServerHandle === undefined)
{
let nextPing = this._nextKeepAlive - new Date().getTime();
if (nextPing < 0) {
nextPing = 0;
}

// The timer needs to be set from a networking callback to avoid Chrome timer throttling from causing timers to run once a minute
this._pingServerHandle = setTimeout(async () => {
if (this._connectionState === HubConnectionState.Connected) {
try {
await this._sendMessage(this._cachedPingMessage);
} catch {
// We don't care about the error. It should be seen elsewhere in the client.
// The connection is probably in a bad or closed state now, cleanup the timer so it stops triggering
this._cleanupPingTimer();
}
}
}, nextPing);
}
}
}

Expand Down Expand Up @@ -813,6 +831,7 @@ export class HubConnection {
private _cleanupPingTimer(): void {
if (this._pingServerHandle) {
clearTimeout(this._pingServerHandle);
this._pingServerHandle = undefined;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe("HubConnection", () => {
});

describe("ping", () => {
it("automatically sends multiple pings", async () => {
it("sends pings when receiving pings", async () => {
await VerifyLogger.run(async (logger) => {
const connection = new TestConnection();
const hubConnection = createHubConnection(connection, logger);
Expand All @@ -118,8 +118,15 @@ describe("HubConnection", () => {

try {
await hubConnection.start();

const pingInterval = setInterval(async () => {
await connection.receive({ type: MessageType.Ping });
}, 5);

await delayUntil(500);

clearInterval(pingInterval);

const numPings = connection.sentData.filter((s) => JSON.parse(s).type === MessageType.Ping).length;
expect(numPings).toBeGreaterThanOrEqual(2);
} finally {
Expand Down