Re: [PATCH] Retry HTTP requests on SSL connect failures

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 1, 2012, 21:47 UTC
Message-ID: <7v626tc19t.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1349126586-755-1-git-send-email-spearce@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

Show 6 quoted lines

> Lets assume the site operators (Hi Google!) have a clue and are
> doing everything they already can to ensure secure, successful
> SSL connections from a wide range of HTTP clients. Implementing a
> single level of retry in the client can make it more robust against
> transient failure modes.
> ---

Sign off?

Show 39 quoted lines

>  http.c        | 19 ++++++++++++-------
>  remote-curl.c |  2 ++
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/http.c b/http.c
> index 345c171..953f2e6 100644
> --- a/http.c
> +++ b/http.c
> @@ -784,7 +784,7 @@ static int http_request(const char *url, void *result, int target, int options)
>  	struct slot_results results;
>  	struct curl_slist *headers = NULL;
>  	struct strbuf buf = STRBUF_INIT;
> -	int ret;
> +	int ret, attempts;
>  
>  	slot = get_active_slot();
>  	slot->results = &results;
> @@ -820,12 +820,17 @@ static int http_request(const char *url, void *result, int target, int options)
>  	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
>  	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
>  
> -	if (start_active_slot(slot)) {
> -		run_active_slot(slot);
> -		ret = handle_curl_result(slot);
> -	} else {
> -		error("Unable to start HTTP request for %s", url);
> -		ret = HTTP_START_FAILED;
> +	for (attempts = 0; attempts < 2; attempts++) {
> +		if (start_active_slot(slot)) {
> +			run_active_slot(slot);
> +			if (slot->results->curl_result == CURLE_SSL_CONNECT_ERROR)
> +				continue;
> +			ret = handle_curl_result(slot);
> +		} else {
> +			error("Unable to start HTTP request for %s", url);
> +			ret = HTTP_START_FAILED;
> +		}
> +		break;
>  	}

Two naïve questions, that applies to this and the one in remote-curl.c::run_slot().

 (1) why only twice?
 (2) no need for "wait a bit and then retry"?

Show 13 quoted lines

> diff --git a/remote-curl.c b/remote-curl.c
> index a269608..04a379c 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -353,6 +353,8 @@ static int run_slot(struct active_request_slot *slot)
>  
>  	slot->results = &results;
>  	slot->curl_result = curl_easy_perform(slot->curl);
> +	if (slot->curl_result == CURLE_SSL_CONNECT_ERROR)
> +		slot->curl_result = curl_easy_perform(slot->curl);
>  	finish_active_slot(slot);
>  
>  	err = handle_curl_result(slot);