Retry sporadically stuck requests/responses

Sometimes we have sporadically stuck http requests/responses and in those cases the CLI
is just waiting and not doing anything. If we retry the requests, they
will pass. That's why we need to add a retry logic in the http client.

Author: TsvetanMilanov

lib/common/http-client.ts

@@ -9,13 +9,36 @@ import * as request from "request";
 export class HttpClient implements Server.IHttpClient {
 	private defaultUserAgent: string;
 	private static STATUS_CODE_REGEX = /statuscode=(\d+)/i;
+	private static STUCK_REQUEST_ERROR_MESSAGE = "The request can't receive any response.";
+	private static STUCK_RESPONSE_ERROR_MESSAGE = "Can't receive all parts of the response.";
+	private static STUCK_REQUEST_TIMEOUT = 60000;
+	// We receive multiple response packets every ms but we don't need to be very aggressive here.
+	private static STUCK_RESPONSE_CHECK_INTERVAL = 10000;
 
 	constructor(private $config: Config.IConfig,
 		private $logger: ILogger,
 		private $proxyService: IProxyService,
 		private $staticConfig: Config.IStaticConfig) { }
 
-	async httpRequest(options: any, proxySettings?: IProxySettings): Promise<Server.IResponse> {
+	public async httpRequest(options: any, proxySettings?: IProxySettings): Promise<Server.IResponse> {
+		try {
+			const result = await this.httpRequestCore(options, proxySettings);
+			return result;
+		} catch (err) {
+			if (err.message === HttpClient.STUCK_REQUEST_ERROR_MESSAGE || err.message === HttpClient.STUCK_RESPONSE_ERROR_MESSAGE) {
+				// Retry the request immediately because there are at least 10 seconds between the two requests.
+				// We have to retry only once the sporadically stuck requests/responses.
+				// We can add exponential backoff retry here if we decide that we need to workaround bigger network issues on the client side.
+				this.$logger.warn("%s Retrying request to %s...", err.message, options.url || options);
+				const retryResult = await this.httpRequestCore(options, proxySettings);
+				return retryResult;
+			}
+
+			throw err;
+		}
+	}
+
+	private async httpRequestCore(options: any, proxySettings?: IProxySettings): Promise<Server.IResponse> {
 		if (_.isString(options)) {
 			options = {
 				url: options,
@@ -73,6 +96,10 @@ export class HttpClient implements Server.IHttpClient {
 
 		const result = new Promise<Server.IResponse>((resolve, reject) => {
 			let timerId: number;
+			let stuckRequestTimerId: number;
+			let stuckResponseIntervalId: NodeJS.Timer;
+			let hasResponse = false;
+			const timers: number[] = [];
 
 			const promiseActions: IPromiseActions<Server.IResponse> = {
 				resolve,
@@ -82,8 +109,9 @@ export class HttpClient implements Server.IHttpClient {
 
 			if (options.timeout) {
 				timerId = setTimeout(() => {
-					this.setResponseResult(promiseActions, timerId, { err: new Error(`Request to ${unmodifiedOptions.url} timed out.`) }, );
+					this.setResponseResult(promiseActions, timers, stuckResponseIntervalId, { err: new Error(`Request to ${unmodifiedOptions.url} timed out.`) });
 				}, options.timeout);
+				timers.push(timerId);
 
 				delete options.timeout;
 			}
@@ -95,6 +123,16 @@ export class HttpClient implements Server.IHttpClient {
 			this.$logger.trace("httpRequest: %s", util.inspect(options));
 			const requestObj = request(options);
 
+			stuckRequestTimerId = setTimeout(() => {
+				clearTimeout(stuckRequestTimerId);
+				stuckRequestTimerId = null;
+				if (!hasResponse) {
+					requestObj.abort();
+					this.setResponseResult(promiseActions, timers, stuckResponseIntervalId, { err: new Error(HttpClient.STUCK_REQUEST_ERROR_MESSAGE) });
+				}
+			}, options.timeout || HttpClient.STUCK_REQUEST_TIMEOUT);
+			timers.push(stuckRequestTimerId);
+
 			requestObj
 				.on("error", (err: IHttpRequestError) => {
 					this.$logger.trace("An error occurred while sending the request:", err);
@@ -107,15 +145,29 @@ export class HttpClient implements Server.IHttpClient {
 					const errorMessage = this.getErrorMessage(errorMessageStatusCode, null);
 					err.proxyAuthenticationRequired = errorMessageStatusCode === HttpStatusCodes.PROXY_AUTHENTICATION_REQUIRED;
 					err.message = errorMessage || err.message;
-					this.setResponseResult(promiseActions, timerId, { err });
+					this.setResponseResult(promiseActions, timers, stuckResponseIntervalId, { err });
 				})
 				.on("response", (response: Server.IRequestResponseData) => {
+					hasResponse = true;
+					let lastChunkTimestamp = Date.now();
+					stuckResponseIntervalId = setInterval(() => {
+						if (Date.now() - lastChunkTimestamp > HttpClient.STUCK_RESPONSE_CHECK_INTERVAL) {
+							if ((<any>response).destroy) {
+								(<any>response).destroy();
+							}
+
+							this.setResponseResult(promiseActions, timers, stuckResponseIntervalId, { err: new Error(HttpClient.STUCK_RESPONSE_ERROR_MESSAGE) });
+						}
+					}, HttpClient.STUCK_RESPONSE_CHECK_INTERVAL);
 					const successful = helpers.isRequestSuccessful(response);
 					if (!successful) {
 						pipeTo = undefined;
 					}
 
 					let responseStream = response;
+					responseStream.on("data", (chunk: string) => {
+						lastChunkTimestamp = Date.now();
+					});
 					switch (response.headers["content-encoding"]) {
 						case "gzip":
 							responseStream = responseStream.pipe(zlib.createGunzip());
@@ -128,7 +180,7 @@ export class HttpClient implements Server.IHttpClient {
 					if (pipeTo) {
 						pipeTo.on("finish", () => {
 							this.$logger.trace("httpRequest: Piping done. code = %d", response.statusCode.toString());
-							this.setResponseResult(promiseActions, timerId, { response });
+							this.setResponseResult(promiseActions, timers, stuckResponseIntervalId, { response });
 						});
 
 						responseStream.pipe(pipeTo);
@@ -144,13 +196,13 @@ export class HttpClient implements Server.IHttpClient {
 							const responseBody = data.join("");
 
 							if (successful) {
-								this.setResponseResult(promiseActions, timerId, { body: responseBody, response });
+								this.setResponseResult(promiseActions, timers, stuckResponseIntervalId, { body: responseBody, response });
 							} else {
 								const errorMessage = this.getErrorMessage(response.statusCode, responseBody);
 								const err: any = new Error(errorMessage);
 								err.response = response;
 								err.body = responseBody;
-								this.setResponseResult(promiseActions, timerId, { err });
+								this.setResponseResult(promiseActions, timers, stuckResponseIntervalId, { err });
 							}
 						});
 					}
@@ -181,10 +233,17 @@ export class HttpClient implements Server.IHttpClient {
 		return response;
 	}
 
-	private setResponseResult(result: IPromiseActions<Server.IResponse>, timerId: number, resultData: { response?: Server.IRequestResponseData, body?: string, err?: Error }): void {
-		if (timerId) {
-			clearTimeout(timerId);
-			timerId = null;
+	private setResponseResult(result: IPromiseActions<Server.IResponse>, timers: number[], stuckResponseIntervalId: NodeJS.Timer, resultData: { response?: Server.IRequestResponseData, body?: string, err?: Error }): void {
+		timers.forEach(t => {
+			if (t) {
+				clearTimeout(t);
+				t = null;
+			}
+		});
+
+		if (stuckResponseIntervalId) {
+			clearInterval(stuckResponseIntervalId);
+			stuckResponseIntervalId = null;
 		}
 
 		if (!result.isResolved()) {