public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: "Vaidas Pilkauskas via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Taylor Blau <me@ttaylorr.com>, Jeff King <peff@peff.net>,
	Vaidas Pilkauskas <vaidas.pilkauskas@shopify.com>
Subject: [PATCH v3 0/3] http: add support for HTTP 429 rate limit retries
Date: Tue, 17 Feb 2026 11:08:37 +0000	[thread overview]
Message-ID: <pull.2008.v3.git.1771326521.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2008.v2.git.1766069088.gitgitgadget@gmail.com>

Changes since v2:

 * New preparatory patch: Introduced show_http_message_fatal() helper
   function to reduce code duplication in remote-curl.c (suggested by Taylor
   Blau)

 * Removed specific HTTP_RATE_LIMITED error handling from http-push.c and
   http-walker.c for the obsolete "dumb" protocol, allowing generic error
   handling to take over (suggested by Jeff King)

 * Added support for CURLINFO_RETRY_AFTER on curl >= 7.66.0, falling back to
   manual header parsing on older versions

 * Simplified retry/delay architecture: replaced complex non-blocking
   "delayed slot" mechanism with simple blocking sleep() call in the retry
   loop, removing ~66 lines of timing logic (suggested by Jeff King)

 * Fixed Retry-After: 0 handling to allow immediate retry as specified by
   RFC 9110

 * Changed http.retryAfter default from -1 to 0, so Git will retry
   immediately when encountering HTTP 429 without a Retry-After header,
   rather than failing with a configuration error

 * Improved error messages: shortened to be more concise

 * Fixed coding style issues: removed unnecessary curly braces, changed x ==
   0 to !x (per CodingGuidelines)

 * Improved test portability: replaced non-portable date(1) commands with
   test-tool date, added nanosecond-precision timing with getnanos, replaced
   cut(1) with POSIX shell parameter expansion

 * Split out strbuf.c bugfix into separate preparatory patch (the
   strbuf_reencode alloc size fix is unrelated to HTTP 429 support)

 * Squashed separate trace2 logging patch into main HTTP 429 retry support
   commit

 * Kept header_is_last_match assignment for Retry-After to prevent incorrect
   handling of HTTP header continuation lines

The implementation includes:

 1. A bug fix in strbuf_reencode() that corrects the allocation size passed
    to strbuf_attach(), ensuring proper memory management.

 2. A refactoring in remote-curl.c that introduces a
    show_http_message_fatal() helper to reduce code duplication when
    handling fatal HTTP errors.

 3. The main feature: HTTP 429 retry logic with support for the Retry-After
    header (both delay-seconds and HTTP-date formats), configurable via
    http.maxRetries, http.retryAfter, and http.maxRetryTime options.

Vaidas Pilkauskas (3):
  strbuf: fix incorrect alloc size in strbuf_reencode()
  remote-curl: introduce show_http_message_fatal() helper
  http: add support for HTTP 429 rate limit retries

 Documentation/config/http.adoc |  23 +++
 git-curl-compat.h              |   8 +
 http.c                         | 190 +++++++++++++++++++++--
 http.h                         |   2 +
 remote-curl.c                  |  49 +++---
 strbuf.c                       |   2 +-
 t/lib-httpd.sh                 |   1 +
 t/lib-httpd/apache.conf        |   8 +
 t/lib-httpd/http-429.sh        |  98 ++++++++++++
 t/meson.build                  |   1 +
 t/t5584-http-429-retry.sh      | 266 +++++++++++++++++++++++++++++++++
 11 files changed, 618 insertions(+), 30 deletions(-)
 create mode 100644 t/lib-httpd/http-429.sh
 create mode 100755 t/t5584-http-429-retry.sh


base-commit: 852829b3dd2fe4e7c7fc4d8badde644cf1b66c74
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2008%2Fvaidas-shopify%2Fretry-after-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2008/vaidas-shopify/retry-after-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2008

Range-diff vs v2:

 -:  ---------- > 1:  821043c664 strbuf: fix incorrect alloc size in strbuf_reencode()
 -:  ---------- > 2:  3653067f0e remote-curl: introduce show_http_message_fatal() helper
 1:  d80ce07703 ! 3:  3cece62a63 http: add support for HTTP 429 rate limit retries
     @@ Documentation/config/http.adoc: http.keepAliveCount::
       
      +http.retryAfter::
      +	Default wait time in seconds before retrying when a server returns
     -+	HTTP 429 (Too Many Requests) without a Retry-After header. If set
     -+	to -1 (the default), Git will fail immediately when encountering
     -+	a 429 response without a Retry-After header. When a Retry-After
     -+	header is present, its value takes precedence over this setting.
     -+	Can be overridden by the `GIT_HTTP_RETRY_AFTER` environment variable.
     ++	HTTP 429 (Too Many Requests) without a Retry-After header.
     ++	Defaults to 0 (retry immediately). When a Retry-After header is
     ++	present, its value takes precedence over this setting. Can be
     ++	overridden by the `GIT_HTTP_RETRY_AFTER` environment variable.
      +	See also `http.maxRetries` and `http.maxRetryTime`.
      +
      +http.maxRetries::
     @@ Documentation/config/http.adoc: http.keepAliveCount::
       	A boolean which disables using of EPSV ftp command by curl.
       	This can be helpful with some "poor" ftp servers which don't
      
     - ## http-push.c ##
     -@@ http-push.c: static int fetch_indices(void)
     - 	case HTTP_MISSING_TARGET:
     - 		ret = 0;
     - 		break;
     -+	case HTTP_RATE_LIMITED:
     -+		error(_("rate limited by '%s', please try again later"), repo->url);
     -+		ret = -1;
     -+		break;
     - 	default:
     - 		ret = -1;
     - 	}
     -@@ http-push.c: static int remote_exists(const char *path)
     - 	case HTTP_MISSING_TARGET:
     - 		ret = 0;
     - 		break;
     -+	case HTTP_RATE_LIMITED:
     -+		error(_("rate limited by '%s', please try again later"), url);
     -+		ret = -1;
     -+		break;
     - 	case HTTP_ERROR:
     - 		error("unable to access '%s': %s", url, curl_errorstr);
     - 		/* fallthrough */
     -
     - ## http-walker.c ##
     -@@ http-walker.c: static int fetch_indices(struct walker *walker, struct alt_base *repo)
     - 		repo->got_indices = 1;
     - 		ret = 0;
     - 		break;
     -+	case HTTP_RATE_LIMITED:
     -+		error("rate limited by '%s', please try again later", repo->base);
     -+		repo->got_indices = 0;
     -+		ret = -1;
     -+		break;
     - 	default:
     - 		repo->got_indices = 0;
     - 		ret = -1;
     + ## git-curl-compat.h ##
     +@@
     + #define GIT_CURL_NEED_TRANSFER_ENCODING_HEADER
     + #endif
     + 
     ++/**
     ++ * CURLINFO_RETRY_AFTER was added in 7.66.0, released in September 2019.
     ++ * It allows curl to automatically parse Retry-After headers.
     ++ */
     ++#if LIBCURL_VERSION_NUM >= 0x074200
     ++#define GIT_CURL_HAVE_CURLINFO_RETRY_AFTER 1
     ++#endif
     ++
     + /**
     +  * CURLOPT_PROTOCOLS_STR and CURLOPT_REDIR_PROTOCOLS_STR were added in 7.85.0,
     +  * released in August 2022.
      
       ## http.c ##
      @@
     @@ http.c
       #include "odb.h"
       #include "tempfile.h"
      +#include "date.h"
     ++#include "trace2.h"
       
       static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
       static int trace_curl_data = 1;
     @@ http.c: static char *cached_accept_language;
       
       static int http_schannel_check_revoke = 1;
      +
     -+static long http_retry_after = -1;
     ++static long http_retry_after = 0;
      +static long http_max_retries = 0;
      +static long http_max_retry_time = 300;
      +
     @@ http.c: static inline int is_hdr_continuation(const char *ptr, const size_t size
       }
       
      -static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p UNUSED)
     -+static size_t fwrite_headers(char *ptr, size_t eltsize, size_t nmemb, void *p)
     ++static size_t fwrite_headers(char *ptr, size_t eltsize, size_t nmemb, void *p MAYBE_UNUSED)
       {
       	size_t size = eltsize * nmemb;
       	struct strvec *values = &http_auth.wwwauth_headers;
     - 	struct strbuf buf = STRBUF_INIT;
     - 	const char *val;
     - 	size_t val_len;
     -+	struct active_request_slot *slot = (struct active_request_slot *)p;
     - 
     - 	/*
     - 	 * Header lines may not come NULL-terminated from libcurl so we must
      @@ http.c: static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p UN
       		goto exit;
       	}
       
     -+	/* Parse Retry-After header for rate limiting */
     ++#ifndef GIT_CURL_HAVE_CURLINFO_RETRY_AFTER
     ++	/* Parse Retry-After header for rate limiting (for curl < 7.66.0) */
      +	if (skip_iprefix_mem(ptr, size, "retry-after:", &val, &val_len)) {
     ++		struct active_request_slot *slot = (struct active_request_slot *)p;
     ++
      +		strbuf_add(&buf, val, val_len);
      +		strbuf_trim(&buf);
      +
     @@ http.c: static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, vo
      +			errno = 0;
      +			retry_after = strtol(buf.buf, &endptr, 10);
      +
     -+			/* Check if it's a valid integer (delay-seconds format) */
     -+			if (endptr != buf.buf && *endptr == '\0' &&
     -+			    errno != ERANGE && retry_after > 0) {
     -+				slot->results->retry_after = retry_after;
     -+			} else {
     ++		/* Check if it's a valid integer (delay-seconds format) */
     ++		if (endptr != buf.buf && *endptr == '\0' &&
     ++		    errno != ERANGE && retry_after >= 0) {
     ++			slot->results->retry_after = retry_after;
     ++		} else {
      +				/* Try parsing as HTTP-date format */
      +				timestamp_t timestamp;
      +				int offset;
     @@ http.c: static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, vo
      +			}
      +		}
      +
     -+		http_auth.header_is_last_match = 1;
      +		goto exit;
      +	}
     ++#endif
      +
       	/*
       	 * This line could be a continuation of the previously matched header
       	 * field. If this is the case then we should append this value to the
     +@@ http.c: static void finish_active_slot(struct active_request_slot *slot)
     + 
     + 		curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
     + 			&slot->results->http_connectcode);
     ++
     ++#ifdef GIT_CURL_HAVE_CURLINFO_RETRY_AFTER
     ++		if (slot->results->http_code == 429) {
     ++			curl_off_t retry_after;
     ++			CURLcode res = curl_easy_getinfo(slot->curl,
     ++							  CURLINFO_RETRY_AFTER,
     ++							  &retry_after);
     ++			if (res == CURLE_OK && retry_after > 0)
     ++				slot->results->retry_after = (long)retry_after;
     ++		}
     ++#endif
     + 	}
     + 
     + 	/* Run callback if appropriate */
      @@ http.c: static int http_options(const char *var, const char *value,
       		return 0;
       	}
     @@ http.c: void http_init(struct remote *remote, const char *url, int proactive_aut
       	curl_default = get_curl_handle();
       }
       
     -@@ http.c: struct active_request_slot *get_active_slot(void)
     - 	slot->finished = NULL;
     - 	slot->callback_data = NULL;
     - 	slot->callback_func = NULL;
     -+	slot->retry_delay_seconds = -1;
     -+	memset(&slot->retry_delay_start, 0, sizeof(slot->retry_delay_start));
     - 
     - 	if (curl_cookie_file && !strcmp(curl_cookie_file, "-")) {
     - 		warning(_("refusing to read cookies from http.cookiefile '-'"));
     -@@ http.c: void run_active_slot(struct active_request_slot *slot)
     - 	fd_set excfds;
     - 	int max_fd;
     - 	struct timeval select_timeout;
     -+	long curl_timeout;
     -+	struct timeval start_time = {0}, current_time, elapsed_time = {0};
     -+	long remaining_seconds;
     - 	int finished = 0;
     -+	int slot_not_started = (slot->finished == NULL);
     -+	int waiting_for_delay = (slot->retry_delay_seconds > 0);
     -+
     -+	if (waiting_for_delay) {
     -+		warning(_("rate limited, waiting %ld seconds before retry"), slot->retry_delay_seconds);
     -+		start_time = slot->retry_delay_start;
     -+	}
     - 
     - 	slot->finished = &finished;
     --	while (!finished) {
     -+	while (waiting_for_delay || !finished) {
     -+		if (waiting_for_delay) {
     -+			gettimeofday(&current_time, NULL);
     -+			elapsed_time.tv_sec = current_time.tv_sec - start_time.tv_sec;
     -+			elapsed_time.tv_usec = current_time.tv_usec - start_time.tv_usec;
     -+			if (elapsed_time.tv_usec < 0) {
     -+				elapsed_time.tv_sec--;
     -+				elapsed_time.tv_usec += 1000000;
     -+			}
     -+
     -+			if (elapsed_time.tv_sec >= slot->retry_delay_seconds) {
     -+				slot->retry_delay_seconds = -1;
     -+				waiting_for_delay = 0;
     -+
     -+				if (slot_not_started)
     -+					return;
     -+			}
     -+		}
     -+
     - 		step_active_slots();
     - 
     --		if (slot->in_use) {
     --			long curl_timeout;
     --			curl_multi_timeout(curlm, &curl_timeout);
     --			if (curl_timeout == 0) {
     -+		if (!waiting_for_delay && !slot->in_use)
     -+			continue;
     -+
     -+		curl_multi_timeout(curlm, &curl_timeout);
     -+		if (curl_timeout == 0) {
     -+			if (!waiting_for_delay)
     - 				continue;
     --			} else if (curl_timeout == -1) {
     --				select_timeout.tv_sec  = 0;
     --				select_timeout.tv_usec = 50000;
     -+			select_timeout.tv_sec = 0;
     -+			select_timeout.tv_usec = 50000; /* 50ms */
     -+		} else if (curl_timeout == -1) {
     -+			select_timeout.tv_sec = 0;
     -+			select_timeout.tv_usec = 50000;
     -+		} else {
     -+			long curl_timeout_sec = curl_timeout / 1000;
     -+			long curl_timeout_usec = (curl_timeout % 1000) * 1000;
     -+
     -+			if (waiting_for_delay) {
     -+				remaining_seconds = slot->retry_delay_seconds - elapsed_time.tv_sec;
     -+				if (curl_timeout_sec < remaining_seconds) {
     -+					select_timeout.tv_sec = curl_timeout_sec;
     -+					select_timeout.tv_usec = curl_timeout_usec;
     -+				} else {
     -+					select_timeout.tv_sec = remaining_seconds;
     -+					select_timeout.tv_usec = 0;
     -+				}
     - 			} else {
     --				select_timeout.tv_sec  =  curl_timeout / 1000;
     --				select_timeout.tv_usec = (curl_timeout % 1000) * 1000;
     -+				select_timeout.tv_sec = curl_timeout_sec;
     -+				select_timeout.tv_usec = curl_timeout_usec;
     - 			}
     -+		}
     - 
     --			max_fd = -1;
     --			FD_ZERO(&readfds);
     --			FD_ZERO(&writefds);
     --			FD_ZERO(&excfds);
     --			curl_multi_fdset(curlm, &readfds, &writefds, &excfds, &max_fd);
     -+		max_fd = -1;
     -+		FD_ZERO(&readfds);
     -+		FD_ZERO(&writefds);
     -+		FD_ZERO(&excfds);
     -+		curl_multi_fdset(curlm, &readfds, &writefds, &excfds, &max_fd);
     - 
     --			/*
     --			 * It can happen that curl_multi_timeout returns a pathologically
     --			 * long timeout when curl_multi_fdset returns no file descriptors
     --			 * to read.  See commit message for more details.
     --			 */
     --			if (max_fd < 0 &&
     --			    (select_timeout.tv_sec > 0 ||
     --			     select_timeout.tv_usec > 50000)) {
     --				select_timeout.tv_sec  = 0;
     --				select_timeout.tv_usec = 50000;
     --			}
     -+		/*
     -+		 * It can happen that curl_multi_timeout returns a pathologically
     -+		 * long timeout when curl_multi_fdset returns no file descriptors
     -+		 * to read.  See commit message for more details.
     -+		 */
     -+		if (max_fd < 0 &&
     -+		    (select_timeout.tv_sec > 0 ||
     -+		     select_timeout.tv_usec > 50000)) {
     -+			select_timeout.tv_sec = 0;
     -+			select_timeout.tv_usec = 50000;
     -+		}
     - 
     --			select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
     -+		/*
     -+		 * If curl_multi_fdset returns no file descriptors but we have
     -+		 * a timeout, still use select() to wait for the timeout period.
     -+		 */
     -+		if (max_fd < 0) {
     -+			/* No file descriptors, just wait for timeout */
     -+			select(0, NULL, NULL, NULL, &select_timeout);
     -+		} else {
     -+			select(max_fd + 1, &readfds, &writefds, &excfds, &select_timeout);
     - 		}
     - 	}
     - 
      @@ http.c: static int handle_curl_result(struct slot_results *results)
       			}
       			return HTTP_REAUTH;
       		}
      +	} else if (results->http_code == 429) {
     ++		trace2_data_intmax("http", the_repository, "http/429-retry-after",
     ++			results->retry_after);
      +		return HTTP_RATE_LIMITED;
       	} else {
       		if (results->http_connectcode == 407)
     @@ http.c: int run_one_slot(struct active_request_slot *slot,
       	slot->results = results;
      +	/* Initialize retry_after to -1 (not set) */
      +	results->retry_after = -1;
     -+
     -+	/* If there's a retry delay, wait for it before starting the slot */
     -+	if (slot->retry_delay_seconds > 0) {
     -+		run_active_slot(slot);
     -+	}
      +
       	if (!start_active_slot(slot)) {
       		xsnprintf(curl_errorstr, sizeof(curl_errorstr),
       			  "failed to start HTTP request");
      @@ http.c: static void http_opt_request_remainder(CURL *curl, off_t pos)
     - #define HTTP_REQUEST_STRBUF	0
     - #define HTTP_REQUEST_FILE	1
       
     -+static void sleep_for_retry(struct active_request_slot *slot, long retry_after);
     -+
       static int http_request(const char *url,
       			void *result, int target,
      -			const struct http_get_options *options)
      +			const struct http_get_options *options,
     -+			long *retry_after_out,
     -+			long retry_delay)
     ++			long *retry_after_out)
       {
       	struct active_request_slot *slot;
       	struct slot_results results;
     -@@ http.c: static int http_request(const char *url,
     - 	int ret;
     - 
     - 	slot = get_active_slot();
     -+	/* Mark slot for delay if retry delay is provided */
     -+	if (retry_delay > 0) {
     -+		sleep_for_retry(slot, retry_delay);
     -+	}
     - 	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);
     - 
     - 	if (!result) {
      @@ http.c: static int http_request(const char *url,
       					 fwrite_buffer);
       	}
     @@ http.c: static int update_url_from_redirect(struct strbuf *base,
       
      -static int http_request_reauth(const char *url,
      +/*
     -+ * Mark slot to be delayed for retry. The actual delay will be handled
     -+ * in run_active_slot when the slot is executed.
     -+ */
     -+static void sleep_for_retry(struct active_request_slot *slot, long retry_after)
     -+{
     -+	if (retry_after > 0 && slot) {
     -+		warning(_("rate limited, waiting %ld seconds before retry"), retry_after);
     -+		slot->retry_delay_seconds = retry_after;
     -+		gettimeofday(&slot->retry_delay_start, NULL);
     -+	}
     -+}
     -+
     -+/*
      + * Handle rate limiting retry logic for HTTP 429 responses.
     -+ * Uses slot-specific retry_after value to support concurrent slots.
      + * Returns a negative value if retries are exhausted or configuration is invalid,
      + * otherwise returns the delay value (>= 0) to indicate the retry should proceed.
      + */
      +static long handle_rate_limit_retry(int *rate_limit_retries, long slot_retry_after)
      +{
      +	int retry_attempt = http_max_retries - *rate_limit_retries + 1;
     ++
     ++	trace2_data_intmax("http", the_repository, "http/429-retry-attempt",
     ++		retry_attempt);
     ++
      +	if (*rate_limit_retries <= 0) {
      +		/* Retries are disabled or exhausted */
      +		if (http_max_retries > 0) {
      +			error(_("too many rate limit retries, giving up"));
     ++			trace2_data_string("http", the_repository,
     ++					   "http/429-error", "retries-exhausted");
      +		}
      +		return -1;
      +	}
      +
     -+	/* Decrement retries counter */
      +	(*rate_limit_retries)--;
      +
      +	/* Use the slot-specific retry_after value or configured default */
      +	if (slot_retry_after >= 0) {
      +		/* Check if retry delay exceeds maximum allowed */
      +		if (slot_retry_after > http_max_retry_time) {
     -+			error(_("rate limited (HTTP 429) requested %ld second delay, "
     -+				"exceeds http.maxRetryTime of %ld seconds"),
     ++			error(_("response requested a delay greater than http.maxRetryTime (%ld > %ld seconds)"),
      +			      slot_retry_after, http_max_retry_time);
     ++			trace2_data_string("http", the_repository,
     ++				  "http/429-error", "exceeds-max-retry-time");
     ++			trace2_data_intmax("http", the_repository,
     ++				  "http/429-requested-delay", slot_retry_after);
      +			return -1;
      +		}
      +		return slot_retry_after;
      +	} else {
     -+		/* No Retry-After header provided */
     -+		if (http_retry_after < 0) {
     -+			/* Not configured - exit with error */
     -+			error(_("rate limited (HTTP 429) and no Retry-After header provided. "
     -+				"Configure http.retryAfter or set GIT_HTTP_RETRY_AFTER."));
     -+			return -1;
     -+		}
     -+		/* Check if configured default exceeds maximum allowed */
     ++		/* No Retry-After header provided, use configured default */
      +		if (http_retry_after > http_max_retry_time) {
     -+			error(_("configured http.retryAfter (%ld seconds) exceeds "
     -+				"http.maxRetryTime (%ld seconds)"),
     ++			error(_("configured http.retryAfter exceeds http.maxRetryTime (%ld > %ld seconds)"),
      +			      http_retry_after, http_max_retry_time);
     ++			trace2_data_string("http", the_repository,
     ++					"http/429-error", "config-exceeds-max-retry-time");
      +			return -1;
      +		}
     -+
     ++		trace2_data_string("http", the_repository,
     ++			"http/429-retry-source", "config-default");
      +		return http_retry_after;
      +	}
      +}
     @@ http.c: static int update_url_from_redirect(struct strbuf *base,
       		credential_fill(the_repository, &http_auth, 1);
       
      -	ret = http_request(url, result, target, options);
     -+	ret = http_request(url, result, target, options, &slot_retry_after, -1);
     ++	ret = http_request(url, result, target, options, &slot_retry_after);
       
      -	if (ret != HTTP_OK && ret != HTTP_REAUTH)
      +	if (ret != HTTP_OK && ret != HTTP_REAUTH && ret != HTTP_RATE_LIMITED)
       		return ret;
       
      +	/* If retries are disabled and we got a 429, fail immediately */
     -+	if (ret == HTTP_RATE_LIMITED && http_max_retries == 0)
     ++	if (ret == HTTP_RATE_LIMITED && !http_max_retries)
      +		return HTTP_ERROR;
      +
       	if (options && options->effective_url && options->base_url) {
     @@ http.c: static int http_request_reauth(const char *url,
      +			retry_delay = handle_rate_limit_retry(&rate_limit_retries, slot_retry_after);
      +			if (retry_delay < 0)
      +				return HTTP_ERROR;
     ++
     ++			if (retry_delay > 0) {
     ++				warning(_("rate limited, waiting %ld seconds before retry"), retry_delay);
     ++				trace2_data_intmax("http", the_repository,
     ++						   "http/retry-sleep-seconds", retry_delay);
     ++				sleep(retry_delay);
     ++			}
      +			slot_retry_after = -1; /* Reset after use */
      +		} else if (ret == HTTP_REAUTH) {
      +			credential_fill(the_repository, &http_auth, 1);
     @@ http.c: static int http_request_reauth(const char *url,
      -		credential_fill(the_repository, &http_auth, 1);
      -
      -		ret = http_request(url, result, target, options);
     -+		ret = http_request(url, result, target, options, &slot_retry_after, retry_delay);
     ++		ret = http_request(url, result, target, options, &slot_retry_after);
       	}
       	return ret;
       }
     @@ http.h: struct slot_results {
       };
       
       struct active_request_slot {
     -@@ http.h: struct active_request_slot {
     - 	void *callback_data;
     - 	void (*callback_func)(void *data);
     - 	struct active_request_slot *next;
     -+	long retry_delay_seconds;
     -+	struct timeval retry_delay_start;
     - };
     - 
     - struct buffer {
      @@ http.h: struct http_get_options {
       #define HTTP_REAUTH	4
       #define HTTP_NOAUTH	5
     @@ http.h: struct http_get_options {
        * Requests a URL and stores the result in a strbuf.
      
       ## remote-curl.c ##
     -@@ remote-curl.c: static void free_discovery(struct discovery *d)
     - 	}
     - }
     - 
     --static int show_http_message(struct strbuf *type, struct strbuf *charset,
     --			     struct strbuf *msg)
     -+static NORETURN void show_http_message_fatal(struct strbuf *type, struct strbuf *charset,
     -+				    struct strbuf *msg, const char *fmt, ...)
     - {
     - 	const char *p, *eol;
     -+	va_list ap;
     -+	report_fn die_message_routine = get_die_message_routine();
     - 
     - 	/*
     - 	 * We only show text/plain parts, as other types are likely
     - 	 * to be ugly to look at on the user's terminal.
     - 	 */
     - 	if (strcmp(type->buf, "text/plain"))
     --		return -1;
     -+		goto out;
     - 	if (charset->len)
     - 		strbuf_reencode(msg, charset->buf, get_log_output_encoding());
     - 
     - 	strbuf_trim(msg);
     - 	if (!msg->len)
     --		return -1;
     -+		goto out;
     - 
     - 	p = msg->buf;
     - 	do {
     -@@ remote-curl.c: static int show_http_message(struct strbuf *type, struct strbuf *charset,
     - 		fprintf(stderr, "remote: %.*s\n", (int)(eol - p), p);
     - 		p = eol + 1;
     - 	} while(*eol);
     --	return 0;
     -+
     -+out:
     -+	strbuf_release(type);
     -+	strbuf_release(charset);
     -+	strbuf_release(msg);
     -+
     -+	va_start(ap, fmt);
     -+	die_message_routine(fmt, ap);
     -+	va_end(ap);
     -+	exit(128);
     - }
     - 
     - static int get_protocol_http_header(enum protocol_version version,
      @@ remote-curl.c: static struct discovery *discover_refs(const char *service, int for_push)
     - 	case HTTP_OK:
     - 		break;
     - 	case HTTP_MISSING_TARGET:
     --		show_http_message(&type, &charset, &buffer);
     --		die(_("repository '%s' not found"),
     --		    transport_anonymize_url(url.buf));
     -+		show_http_message_fatal(&type, &charset, &buffer,
     -+					_("repository '%s' not found"),
     -+					transport_anonymize_url(url.buf));
     - 	case HTTP_NOAUTH:
     --		show_http_message(&type, &charset, &buffer);
     --		die(_("Authentication failed for '%s'"),
     --		    transport_anonymize_url(url.buf));
     -+		show_http_message_fatal(&type, &charset, &buffer,
     -+					_("Authentication failed for '%s'"),
     -+					transport_anonymize_url(url.buf));
     - 	case HTTP_NOMATCHPUBLICKEY:
     --		show_http_message(&type, &charset, &buffer);
     --		die(_("unable to access '%s' with http.pinnedPubkey configuration: %s"),
     --		    transport_anonymize_url(url.buf), curl_errorstr);
     -+		show_http_message_fatal(&type, &charset, &buffer,
     -+					_("unable to access '%s' with http.pinnedPubkey configuration: %s"),
     -+					transport_anonymize_url(url.buf), curl_errorstr);
     + 		show_http_message_fatal(&type, &charset, &buffer,
     + 					_("unable to access '%s' with http.pinnedPubkey configuration: %s"),
     + 					transport_anonymize_url(url.buf), curl_errorstr);
      +	case HTTP_RATE_LIMITED:
      +		show_http_message_fatal(&type, &charset, &buffer,
      +					_("rate limited by '%s', please try again later"),
      +					transport_anonymize_url(url.buf));
       	default:
     --		show_http_message(&type, &charset, &buffer);
     --		die(_("unable to access '%s': %s"),
     --		    transport_anonymize_url(url.buf), curl_errorstr);
     -+		show_http_message_fatal(&type, &charset, &buffer,
     -+					_("unable to access '%s': %s"),
     -+					transport_anonymize_url(url.buf), curl_errorstr);
     - 	}
     - 
     - 	if (options.verbosity && !starts_with(refs_url.buf, url.buf)) {
     -
     - ## strbuf.c ##
     -@@ strbuf.c: int strbuf_reencode(struct strbuf *sb, const char *from, const char *to)
     - 	if (!out)
     - 		return -1;
     - 
     --	strbuf_attach(sb, out, len, len);
     -+	strbuf_attach(sb, out, len, len + 1);
     - 	return 0;
     - }
     - 
     + 		show_http_message_fatal(&type, &charset, &buffer,
     + 					_("unable to access '%s': %s"),
      
       ## t/lib-httpd.sh ##
      @@ t/lib-httpd.sh: prepare_httpd() {
     @@ t/t5584-http-429-retry.sh (new)
      +	test_config http.maxRetries 3 &&
      +
      +	# Time the operation - it should take at least 2 seconds due to retry delay
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	git ls-remote "$HTTPD_URL/http_429/retry-delays-respected/2/repo.git" >output 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Verify it took at least 2 seconds (allowing some tolerance)
     -+	test "$duration" -ge 1 &&
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -ge 1 &&
      +	test_grep "refs/heads/" output
      +'
      +
     @@ t/t5584-http-429-retry.sh (new)
      +	test_config http.maxRetryTime 3 &&
      +
      +	# Should fail immediately without waiting
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	test_must_fail git ls-remote "$HTTPD_URL/http_429/retry-after-exceeds-max-time/100/repo.git" 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should fail quickly (less than 2 seconds, no 100 second wait)
     -+	test "$duration" -lt 2 &&
     -+	test_grep "exceeds http.maxRetryTime" err
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -lt 2 &&
     ++	test_grep "greater than http.maxRetryTime" err
      +'
      +
      +test_expect_success 'HTTP 429 fails if configured http.retryAfter exceeds http.maxRetryTime' '
     @@ t/t5584-http-429-retry.sh (new)
      +	test_config http.maxRetryTime 5 &&
      +
      +	# Should fail immediately with configuration error
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	test_must_fail git ls-remote "$HTTPD_URL/http_429/config-retry-after-exceeds-max-time/none/repo.git" 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should fail quickly
     -+	test "$duration" -lt 2 &&
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -lt 2 &&
      +	test_grep "configured http.retryAfter.*exceeds.*http.maxRetryTime" err
      +'
      +
      +test_expect_success 'HTTP 429 with Retry-After HTTP-date format' '
      +	# Test HTTP-date format (RFC 2822) in Retry-After header
     -+	# Generate a date 2 seconds in the future
     -+	future_date=$(TZ=GMT date -d "+2 seconds" "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \
     -+		      TZ=GMT date -v+2S "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \
     -+		      echo "skip") &&
     -+
     -+	if test "$future_date" = "skip"
     -+	then
     -+		skip_all="date command does not support required format" &&
     -+		test_done
     -+	fi &&
     -+
     -+	# URL-encode the date (replace spaces with %20)
     ++	raw=$(test-tool date timestamp now) &&
     ++	now="${raw#* -> }" &&
     ++	future_time=$((now + 2)) &&
     ++	raw=$(test-tool date show:rfc2822 $future_time) &&
     ++	future_date="${raw#* -> }" &&
      +	future_date_encoded=$(echo "$future_date" | sed "s/ /%20/g") &&
      +
      +	# Enable retries
      +	test_config http.maxRetries 3 &&
      +
      +	# Git should parse the HTTP-date and retry after the delay
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	git ls-remote "$HTTPD_URL/http_429/http-date-format/$future_date_encoded/repo.git" >output 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should take at least 1 second (allowing tolerance for processing time)
     -+	test "$duration" -ge 1 &&
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -ge 1 &&
      +	test_grep "refs/heads/" output
      +'
      +
      +test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immediately' '
     -+	# Generate a date 200 seconds in the future
     -+	future_date=$(TZ=GMT date -d "+200 seconds" "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \
     -+		      TZ=GMT date -v+200S "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \
     -+		      echo "skip") &&
     -+
     -+	if test "$future_date" = "skip"
     -+	then
     -+		skip_all="date command does not support required format" &&
     -+		test_done
     -+	fi &&
     -+
     -+	# URL-encode the date (replace spaces with %20)
     ++	raw=$(test-tool date timestamp now) &&
     ++	now="${raw#* -> }" &&
     ++	future_time=$((now + 200)) &&
     ++	raw=$(test-tool date show:rfc2822 $future_time) &&
     ++	future_date="${raw#* -> }" &&
      +	future_date_encoded=$(echo "$future_date" | sed "s/ /%20/g") &&
      +
      +	# Configure max retry time much less than the 200 second delay
     @@ t/t5584-http-429-retry.sh (new)
      +	test_config http.maxRetryTime 10 &&
      +
      +	# Should fail immediately without waiting 200 seconds
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	test_must_fail git ls-remote "$HTTPD_URL/http_429/http-date-exceeds-max-time/$future_date_encoded/repo.git" 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should fail quickly (not wait 200 seconds)
     -+	test "$duration" -lt 2 &&
     -+	test_grep "exceeds http.maxRetryTime" err
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -lt 2 &&
     ++	test_grep "http.maxRetryTime" err
      +'
      +
      +test_expect_success 'HTTP 429 with past HTTP-date should not wait' '
     -+	past_date=$(TZ=GMT date -d "-10 seconds" "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \
     -+		    TZ=GMT date -v-10S "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \
     -+		    echo "skip") &&
     -+
     -+	if test "$past_date" = "skip"
     -+	then
     -+		skip_all="date command does not support required format" &&
     -+		test_done
     -+	fi &&
     -+
     -+	# URL-encode the date (replace spaces with %20)
     ++	raw=$(test-tool date timestamp now) &&
     ++	now="${raw#* -> }" &&
     ++	past_time=$((now - 10)) &&
     ++	raw=$(test-tool date show:rfc2822 $past_time) &&
     ++	past_date="${raw#* -> }" &&
      +	past_date_encoded=$(echo "$past_date" | sed "s/ /%20/g") &&
      +
      +	# Enable retries
      +	test_config http.maxRetries 3 &&
      +
      +	# Git should retry immediately without waiting
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	git ls-remote "$HTTPD_URL/http_429/past-http-date/$past_date_encoded/repo.git" >output 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should complete quickly (less than 2 seconds)
     -+	test "$duration" -lt 2 &&
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -lt 2 &&
      +	test_grep "refs/heads/" output
      +'
      +
     @@ t/t5584-http-429-retry.sh (new)
      +	test_config http.retryAfter 1 &&
      +
      +	# Should use configured default (1 second) since header is invalid
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	git ls-remote "$HTTPD_URL/http_429/invalid-retry-after-format/invalid/repo.git" >output 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should take at least 1 second (the configured default)
     -+	test "$duration" -ge 1 &&
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -ge 1 &&
      +	test_grep "refs/heads/" output &&
      +	test_grep "waiting.*retry" err
      +'
     @@ t/t5584-http-429-retry.sh (new)
      +	test_config http.retryAfter 10 &&
      +
      +	# Override with environment variable to 1 second
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	GIT_HTTP_RETRY_AFTER=1 git ls-remote "$HTTPD_URL/http_429/env-retry-after-override/none/repo.git" >output 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should use env var (1 second), not config (10 seconds)
     -+	test "$duration" -ge 1 &&
     -+	test "$duration" -lt 5 &&
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -ge 1 &&
     ++	test "$duration_int" -lt 5 &&
      +	test_grep "refs/heads/" output &&
      +	test_grep "waiting.*retry" err
      +'
     @@ t/t5584-http-429-retry.sh (new)
      +	test_config http.maxRetryTime 100 &&
      +
      +	# Override with environment variable to 10 seconds (should reject 50 second delay)
     -+	start=$(date +%s) &&
     ++	start=$(test-tool date getnanos) &&
      +	test_must_fail env GIT_HTTP_MAX_RETRY_TIME=10 \
      +		git ls-remote "$HTTPD_URL/http_429/env-max-retry-time-override/50/repo.git" 2>err &&
     -+	end=$(date +%s) &&
     -+	duration=$((end - start)) &&
     ++	duration=$(test-tool date getnanos $start) &&
      +
      +	# Should fail quickly (not wait 50 seconds) because env var limits to 10
     -+	test "$duration" -lt 5 &&
     -+	test_grep "exceeds http.maxRetryTime" err
     ++	duration_int=${duration%.*} &&
     ++	test "$duration_int" -lt 5 &&
     ++	test_grep "greater than http.maxRetryTime" err
      +'
      +
      +test_expect_success 'verify normal repository access still works' '
 2:  ad4495fc94 < -:  ---------- http: add trace2 logging for retry operations

-- 
gitgitgadget

  parent reply	other threads:[~2026-02-17 11:08 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-26 12:30 [PATCH 0/3] http: add support for HTTP 429 rate limit retries Vaidas Pilkauskas via GitGitGadget
2025-11-26 12:30 ` [PATCH 1/3] " Vaidas Pilkauskas via GitGitGadget
2025-12-09 23:15   ` Taylor Blau
2025-12-12 12:36     ` Vaidas Pilkauskas
2025-11-26 12:30 ` [PATCH 2/3] remote-curl: fix memory leak in show_http_message() Vaidas Pilkauskas via GitGitGadget
2025-12-09 23:52   ` Taylor Blau
2025-11-26 12:30 ` [PATCH 3/3] http: add trace2 logging for retry operations Vaidas Pilkauskas via GitGitGadget
2025-12-18 14:44 ` [PATCH v2 0/2] http: add support for HTTP 429 rate limit retries Vaidas Pilkauskas via GitGitGadget
2025-12-18 14:44   ` [PATCH v2 1/2] " Vaidas Pilkauskas via GitGitGadget
2026-02-11  1:05     ` Taylor Blau
2026-02-11  9:13       ` Jeff King
2026-02-13 13:41         ` Vaidas Pilkauskas
2026-02-15  9:13           ` Jeff King
2026-02-13 13:30       ` Vaidas Pilkauskas
2025-12-18 14:44   ` [PATCH v2 2/2] http: add trace2 logging for retry operations Vaidas Pilkauskas via GitGitGadget
2026-02-11  1:06     ` Taylor Blau
2026-02-17 11:08   ` Vaidas Pilkauskas via GitGitGadget [this message]
2026-02-17 11:08     ` [PATCH v3 1/3] strbuf: fix incorrect alloc size in strbuf_reencode() Vaidas Pilkauskas via GitGitGadget
2026-02-17 20:51       ` Junio C Hamano
2026-02-18 13:43         ` Vaidas Pilkauskas
2026-02-17 11:08     ` [PATCH v3 2/3] remote-curl: introduce show_http_message_fatal() helper Vaidas Pilkauskas via GitGitGadget
2026-02-17 11:08     ` [PATCH v3 3/3] http: add support for HTTP 429 rate limit retries Vaidas Pilkauskas via GitGitGadget
2026-02-18 14:09     ` [PATCH v4 0/5] " Vaidas Pilkauskas via GitGitGadget
2026-02-18 14:09       ` [PATCH v4 1/5] strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode() Vaidas Pilkauskas via GitGitGadget
2026-02-18 14:09       ` [PATCH v4 2/5] strbuf_attach: fix all call sites to pass correct alloc Vaidas Pilkauskas via GitGitGadget
2026-02-20 22:55         ` Junio C Hamano
2026-02-23 12:49           ` Vaidas Pilkauskas
2026-02-18 14:09       ` [PATCH v4 3/5] strbuf: replace strbuf_grow() in strbuf_attach() with BUG() check Vaidas Pilkauskas via GitGitGadget
2026-02-18 14:09       ` [PATCH v4 4/5] remote-curl: introduce show_http_message_fatal() helper Vaidas Pilkauskas via GitGitGadget
2026-02-18 14:09       ` [PATCH v4 5/5] http: add support for HTTP 429 rate limit retries Vaidas Pilkauskas via GitGitGadget
2026-02-23 14:20       ` [PATCH v5 0/4] " Vaidas Pilkauskas via GitGitGadget
2026-02-23 14:20         ` [PATCH v5 1/4] strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode() Vaidas Pilkauskas via GitGitGadget
2026-02-23 14:20         ` [PATCH v5 2/4] strbuf_attach: fix call sites to pass correct alloc Vaidas Pilkauskas via GitGitGadget
2026-02-23 14:20         ` [PATCH v5 3/4] remote-curl: introduce show_http_message_fatal() helper Vaidas Pilkauskas via GitGitGadget
2026-03-10 17:44           ` Jeff King
2026-02-23 14:20         ` [PATCH v5 4/4] http: add support for HTTP 429 rate limit retries Vaidas Pilkauskas via GitGitGadget
2026-03-10 19:07           ` Jeff King
2026-02-24  0:07         ` [PATCH v5 0/4] " Junio C Hamano
2026-03-09 23:34           ` Junio C Hamano
2026-03-10 19:10             ` Jeff King
2026-03-10 19:19               ` Junio C Hamano
2026-03-17 13:00         ` [PATCH v6 0/3] " Vaidas Pilkauskas via GitGitGadget
2026-03-17 13:00           ` [PATCH v6 1/3] strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode() Vaidas Pilkauskas via GitGitGadget
2026-03-17 13:00           ` [PATCH v6 2/3] strbuf_attach: fix call sites to pass correct alloc Vaidas Pilkauskas via GitGitGadget
2026-03-17 13:00           ` [PATCH v6 3/3] http: add support for HTTP 429 rate limit retries Vaidas Pilkauskas via GitGitGadget
2026-03-21  3:30             ` Taylor Blau
2026-03-21  3:31           ` [PATCH v6 0/3] " Taylor Blau
2026-03-21  4:57             ` Junio C Hamano
2026-03-23  6:58             ` Vaidas Pilkauskas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.2008.v3.git.1771326521.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=vaidas.pilkauskas@shopify.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox