Re: [PATCH 4/4] remote-curl: retry transient HTTP errors in post_rpc()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 3, 2026, 14:47 UTC
- Message-ID
- <xmqqh5hyas5b.fsf@gitster.g>
- In-Reply-To
- <52819accc894e760a0fd06730d20f46cc8da9914.1793462224.git.gitgitgadget@gmail.com>
"Ravi Mistry via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 35 quoted lines
> diff --git a/http.c b/http.c
> index 950e2bbffa..36b559e596 100644
> --- a/http.c
> +++ b/http.c
> @@ -2476,6 +2476,34 @@ static long handle_rate_limit_retry(long slot_retry_after, int attempt)
> }
> }
>
> +int http_retry_wait(long http_code, long slot_retry_after, int attempt)
> +{
> + long retry_delay;
> +
> + if (http_max_retries <= 0)
> + return -1;
> + if (attempt > http_max_retries) {
> + trace2_data_string("http", the_repository,
> + "http/429-error", "retries-exhausted");
> + return -1;
> + }
> + retry_delay = handle_rate_limit_retry(slot_retry_after, attempt);
> + if (retry_delay < 0)
> + return -1;
> + if (retry_delay > 0) {
> + if (http_code == 429)
> + warning(_("rate limited, waiting %ld seconds before retry"),
> + retry_delay);
> + else
> + warning(_("HTTP %ld, waiting %ld seconds before retry"),
> + http_code, retry_delay);
> + trace2_data_intmax("http", the_repository,
> + "http/retry-sleep-seconds", retry_delay);
> + sleep(retry_delay);
> + }
> + return 0;
> +}This code looks familiar. Beautifully extracted from ...
Show 47 quoted lines
> static int http_request_recoverable(const char *url,
> void *result, int target,
> struct http_get_options *options)
> @@ -2483,7 +2511,6 @@ static int http_request_recoverable(const char *url,
> static struct http_get_options empty_opts;
> int i = 3;
> int ret;
> - int rate_limit_retries = http_max_retries;
> int attempt = 0;
>
> if (!options)
> @@ -2506,9 +2533,8 @@ static int http_request_recoverable(const char *url,
> }
>
> while ((ret == HTTP_REAUTH && --i) ||
> - (ret == HTTP_RATE_LIMITED && http_max_retries > 0 &&
> - rate_limit_retries-- > 0)) {
> - long retry_delay = -1;
> + (ret == HTTP_RATE_LIMITED &&
> + !http_retry_wait(options->http_code, options->retry_after, ++attempt))) {
> /*
> * The previous request may have put cruft into our output stream; we
> * should clear it out before making our next request.
> @@ -2533,35 +2559,15 @@ static int http_request_recoverable(const char *url,
> default:
> BUG("Unknown http_request target");
> }
> - if (ret == HTTP_RATE_LIMITED) {
> - retry_delay = handle_rate_limit_retry(options->retry_after, ++attempt);
> - if (retry_delay < 0)
> - return HTTP_ERROR;
> -
> - if (retry_delay > 0) {
> - if (options->http_code == 429)
> - warning(_("rate limited, waiting %ld seconds before retry"),
> - retry_delay);
> - else
> - warning(_("HTTP %ld, waiting %ld seconds before retry"),
> - options->http_code, retry_delay);
> - trace2_data_intmax("http", the_repository,
> - "http/retry-sleep-seconds", retry_delay);
> - sleep(retry_delay);
> - }
> - } else if (ret == HTTP_REAUTH) {
> + if (ret == HTTP_REAUTH)
> http_reauth_prepare(1);
> - }... existing code around here. It always feels good to see that an introduction of a new function is accompanied by replacing many lines of existsing code with a call to the new function.
Thanks.