Fix #2169 by correcting shutdown timeout (#2170)

This commit is contained in:
Andrew Stanton-Nurse 2018-05-01 16:37:22 -07:00 committed by GitHub
parent d711916ad6
commit ae329edd2a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 74 additions and 10 deletions

View File

@ -577,13 +577,8 @@ describe("hubConnection", () => {
const hubConnection = getConnectionBuilder(transportType).build();
hubConnection.serverTimeoutInMilliseconds = 100;
const timeout = setTimeout(200, () => {
fail("Server timeout did not fire within expected interval");
});
hubConnection.start().then(() => {
hubConnection.onclose((error) => {
clearTimeout(timeout);
expect(error).toEqual(new Error("Server timeout elapsed without receiving a message from the server."));
done();
});

View File

@ -0,0 +1,67 @@
import { HttpResponse } from "../src/HttpClient";
import { LogLevel } from "../src/ILogger";
import { TransferFormat } from "../src/ITransport";
import { NullLogger } from "../src/Loggers";
import { LongPollingTransport } from "../src/LongPollingTransport";
import { ConsoleLogger } from "../src/Utils";
import { TestHttpClient } from "./TestHttpClient";
import { asyncit as it, PromiseSource } from "./Utils";
describe("LongPollingTransport", () => {
it("shuts down poll after timeout even if server doesn't shut it down on receiving the DELETE", async () => {
let firstPoll = true;
const pollCompleted = new PromiseSource();
const client = new TestHttpClient()
.on("GET", async (r) => {
if (firstPoll) {
firstPoll = false;
return new HttpResponse(200);
} else {
// Turn 'onabort' into a promise.
const abort = new Promise((resolve, reject) => r.abortSignal.onabort = resolve);
await abort;
// Signal that the poll has completed.
pollCompleted.resolve();
return new HttpResponse(204);
}
})
.on("DELETE", (r) => new HttpResponse(202));
const transport = new LongPollingTransport(client, null, NullLogger.instance, false, 100);
await transport.connect("http://example.com", TransferFormat.Text);
await transport.stop();
// This should complete within the shutdown timeout
await pollCompleted.promise;
});
it("sends DELETE request on stop", async () => {
let firstPoll = true;
const deleteReceived = new PromiseSource();
const pollCompleted = new PromiseSource();
const client = new TestHttpClient()
.on("GET", async (r) => {
if (firstPoll) {
firstPoll = false;
return new HttpResponse(200);
} else {
await deleteReceived.promise;
pollCompleted.resolve();
return new HttpResponse(204);
}
})
.on("DELETE", (r) => {
deleteReceived.resolve();
return new HttpResponse(202);
});
const transport = new LongPollingTransport(client, null, NullLogger.instance, false);
await transport.connect("http://example.com", TransferFormat.Text);
await transport.stop();
// This should complete, because the DELETE request triggers it to stop.
await pollCompleted.promise;
});
});

View File

@ -19,15 +19,17 @@ export class LongPollingTransport implements ITransport {
private url: string;
private pollXhr: XMLHttpRequest;
private pollAbort: AbortController;
private shutdownTimer: any; // We use 'any' because this is an object in NodeJS. But it still gets passed to clearTimeout, so it doesn't really matter
private shutdownTimeout: number;
private running: boolean;
constructor(httpClient: HttpClient, accessTokenFactory: () => string | Promise<string>, logger: ILogger, logMessageContent: boolean) {
constructor(httpClient: HttpClient, accessTokenFactory: () => string | Promise<string>, logger: ILogger, logMessageContent: boolean, shutdownTimeout?: number) {
this.httpClient = httpClient;
this.accessTokenFactory = accessTokenFactory || (() => null);
this.logger = logger;
this.pollAbort = new AbortController();
this.logMessageContent = logMessageContent;
this.shutdownTimeout = shutdownTimeout || SHUTDOWN_TIMEOUT;
}
public async connect(url: string, transferFormat: TransferFormat): Promise<void> {
@ -107,7 +109,7 @@ export class LongPollingTransport implements ITransport {
this.logger.log(LogLevel.Information, "(LongPolling transport) Poll terminated by server");
// If we were on a timeout waiting for shutdown, unregister it.
clearTimeout(this.shutdownTimeout);
clearTimeout(this.shutdownTimer);
this.running = false;
} else if (response.statusCode !== 200) {
@ -179,10 +181,10 @@ export class LongPollingTransport implements ITransport {
} finally {
// Abort the poll after 5 seconds if the server doesn't stop it.
if (!this.pollAbort.aborted) {
this.shutdownTimeout = setTimeout(SHUTDOWN_TIMEOUT, () => {
this.logger.log(LogLevel.Warning, "(LongPolling transport) server did not terminate within 5 seconds after DELETE request, cancelling poll.");
this.shutdownTimer = setTimeout(() => {
this.logger.log(LogLevel.Warning, "(LongPolling transport) server did not terminate after DELETE request, canceling poll.");
this.pollAbort.abort();
});
}, this.shutdownTimeout);
}
}
}