From: "Vaidas Pilkauskas via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Taylor Blau <me@ttaylorr.com>,
Vaidas Pilkauskas <vaidas.pilkauskas@shopify.com>
Subject: [PATCH v2 0/2] http: add support for HTTP 429 rate limit retries
Date: Thu, 18 Dec 2025 14:44:46 +0000 [thread overview]
Message-ID: <pull.2008.v2.git.1766069088.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2008.git.1764160227.gitgitgadget@gmail.com>
Changes since v1:
* removed configuration options from commit message
* marked "rate limited by %s ..." message for translation in http-push.c
* dropped redundant comments from retry configuration variables
* renamed "fwrite_wwwauth" to "fwrite_headers". Due to complexity related
with header continuation handling, I've decide not to split into header
specific functions.
* rewritten the sleep logic into non-blocking handling, so that the rest of
the slots can be processed at the time retry delay is requested
* renamed "http_request_reauth" to "http_request_recoverable" to better
reflect that it does not only reauth , but also other recoverable
handling
* updated test setup code to use setup script to reduce repetition in the
test code
* updated to use test_grep instead of grep
* dropped memory leak fix patch in afvor to rename of the
"show_http_message" to "show_http_message_fatal".
* Adjusted alloc size in "strbuf_reencode" for "strbuf_attach" call, which
seems to solve the leak problem
The implementation includes:
Patch 1: Core HTTP 429 retry logic with support for RFC-compliant
Retry-After headers (both delay-seconds and HTTP-date formats),
comprehensive configuration options, and fail-fast behavior for excessive
delays. Includes extensive test coverage.
Patch 2: Adds trace2 instrumentation to enable monitoring and debugging of
retry operations in production environments.
Vaidas Pilkauskas (2):
http: add support for HTTP 429 rate limit retries
http: add trace2 logging for retry operations
Documentation/config/http.adoc | 24 +++
http-push.c | 8 +
http-walker.c | 5 +
http.c | 321 +++++++++++++++++++++++++++++----
http.h | 4 +
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 | 286 +++++++++++++++++++++++++++++
12 files changed, 750 insertions(+), 57 deletions(-)
create mode 100644 t/lib-httpd/http-429.sh
create mode 100755 t/t5584-http-429-retry.sh
base-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2008%2Fvaidas-shopify%2Fretry-after-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2008/vaidas-shopify/retry-after-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2008
Range-diff vs v1:
1: ae0087cd1c ! 1: d80ce07703 http: add support for HTTP 429 rate limit retries
@@ Commit message
both delay-seconds (integer) and HTTP-date (RFC 2822) formats. If a
past date is provided, Git retries immediately without waiting.
- Retry behavior is controlled by three new configuration options:
-
- * http.maxRetries: Maximum number of retry attempts (default: 0,
- meaning retries are disabled by default). Users must explicitly
- opt-in to retry behavior.
-
- * http.retryAfter: Default delay in seconds when the server doesn't
- provide a Retry-After header (default: -1, meaning fail if no
- header is provided). This serves as a fallback mechanism.
-
- * http.maxRetryTime: Maximum delay in seconds for a single retry
- (default: 300). If the server requests a delay exceeding this
- limit, Git fails immediately rather than waiting. This prevents
- indefinite blocking on unreasonable server requests.
-
- All three options can be overridden via environment variables:
- GIT_HTTP_MAX_RETRIES, GIT_HTTP_RETRY_AFTER, and
- GIT_HTTP_MAX_RETRY_TIME.
+ Retry behavior is controlled by three new configuration options
+ (http.maxRetries, http.retryAfter, and http.maxRetryTime) which are
+ documented in git-config(1).
The retry logic implements a fail-fast approach: if any delay
(whether from server header or configuration) exceeds maxRetryTime,
@@ http-push.c: static int fetch_indices(void)
ret = 0;
break;
+ case HTTP_RATE_LIMITED:
-+ error("rate limited by '%s', please try again later", repo->url);
++ error(_("rate limited by '%s', please try again later"), repo->url);
+ ret = -1;
+ break;
default:
@@ http-push.c: static int remote_exists(const char *path)
ret = 0;
break;
+ case HTTP_RATE_LIMITED:
-+ error("rate limited by '%s', please try again later", url);
++ error(_("rate limited by '%s', please try again later"), url);
+ ret = -1;
+ break;
case HTTP_ERROR:
@@ http.c: static char *cached_accept_language;
static int http_schannel_check_revoke = 1;
+
-+/* Retry configuration */
-+static long http_retry_after = -1; /* Default retry-after in seconds when header is missing (-1 means not set, exit with 128) */
-+static long http_max_retries = 0; /* Maximum number of retry attempts (0 means retries are disabled) */
-+static long http_max_retry_time = 300; /* Maximum time to wait for a single retry (default 5 minutes) */
++static long http_retry_after = -1;
++static long http_max_retries = 0;
++static long http_max_retry_time = 300;
+
-+/* Store retry_after value from 429 responses for retry logic (-1 = not set, 0 = retry immediately, >0 = delay in seconds) */
-+static long last_retry_after = -1;
/*
* With the backend being set to `schannel`, setting sslCAinfo would override
* the Certificate Store in cURL v7.60.0 and later, which is not what we want
@@ 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_wwwauth(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)
{
size_t size = eltsize * nmemb;
struct strvec *values = &http_auth.wwwauth_headers;
@@ 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(¤t_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) {
-+ /* Store the retry_after value for use in retry logic */
-+ last_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)
+ {
+ 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);
}
- curl_easy_setopt(slot->curl, CURLOPT_HEADERFUNCTION, fwrite_wwwauth);
+- curl_easy_setopt(slot->curl, CURLOPT_HEADERFUNCTION, fwrite_wwwauth);
++ curl_easy_setopt(slot->curl, CURLOPT_HEADERFUNCTION, fwrite_headers);
+ curl_easy_setopt(slot->curl, CURLOPT_HEADERDATA, slot);
accept_language = http_get_accept_language_header();
+@@ http.c: static int http_request(const char *url,
+
+ ret = run_one_slot(slot, &results);
+
++ /* Store retry_after from slot results if output parameter provided */
++ if (retry_after_out)
++ *retry_after_out = results.retry_after;
++
+ if (options && options->content_type) {
+ struct strbuf raw = STRBUF_INIT;
+ curlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE, &raw);
@@ http.c: static int update_url_from_redirect(struct strbuf *base,
return 1;
}
+-static int http_request_reauth(const char *url,
+/*
-+ * Sleep for the specified number of seconds before retrying.
++ * 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(long retry_after)
++static void sleep_for_retry(struct active_request_slot *slot, long retry_after)
+{
-+ if (retry_after > 0) {
-+ unsigned int remaining;
++ if (retry_after > 0 && slot) {
+ warning(_("rate limited, waiting %ld seconds before retry"), retry_after);
-+ remaining = sleep(retry_after);
-+ while (remaining > 0) {
-+ /* Sleep was interrupted, continue sleeping */
-+ remaining = sleep(remaining);
++ 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;
++ if (*rate_limit_retries <= 0) {
++ /* Retries are disabled or exhausted */
++ if (http_max_retries > 0) {
++ error(_("too many rate limit retries, giving up"));
++ }
++ 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"),
++ slot_retry_after, http_max_retry_time);
++ 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 */
++ if (http_retry_after > http_max_retry_time) {
++ error(_("configured http.retryAfter (%ld seconds) exceeds "
++ "http.maxRetryTime (%ld seconds)"),
++ http_retry_after, http_max_retry_time);
++ return -1;
++ }
++
++ return http_retry_after;
+ }
+}
+
- static int http_request_reauth(const char *url,
++static int http_request_recoverable(const char *url,
void *result, int target,
struct http_get_options *options)
{
int i = 3;
int ret;
+ int rate_limit_retries = http_max_retries;
++ long slot_retry_after = -1; /* Per-slot retry_after value */
if (always_auth_proactively())
credential_fill(the_repository, &http_auth, 1);
- ret = http_request(url, result, target, options);
+- ret = http_request(url, result, target, options);
++ ret = http_request(url, result, target, options, &slot_retry_after, -1);
- 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)
++ return HTTP_ERROR;
++
if (options && options->effective_url && options->base_url) {
+ if (update_url_from_redirect(options->base_url,
+ url, options->effective_url)) {
@@ http.c: static int http_request_reauth(const char *url,
}
}
- while (ret == HTTP_REAUTH && --i) {
+ while ((ret == HTTP_REAUTH || ret == HTTP_RATE_LIMITED) && --i) {
++ long retry_delay = -1;
/*
* The previous request may have put cruft into our output stream; we
* should clear it out before making our next request.
@@ http.c: static int http_request_reauth(const char *url,
+ default:
BUG("Unknown http_request target");
}
-
-- credential_fill(the_repository, &http_auth, 1);
+ if (ret == HTTP_RATE_LIMITED) {
-+ /* Handle rate limiting with retry logic */
-+ int retry_attempt = http_max_retries - rate_limit_retries + 1;
-+
-+ if (rate_limit_retries <= 0) {
-+ /* Retries are disabled or exhausted */
-+ if (http_max_retries > 0) {
-+ error(_("too many rate limit retries, giving up"));
-+ }
++ retry_delay = handle_rate_limit_retry(&rate_limit_retries, slot_retry_after);
++ if (retry_delay < 0)
+ return HTTP_ERROR;
-+ }
-+
-+ /* Decrement retries counter */
-+ rate_limit_retries--;
-+
-+ /* Use the stored retry_after value or configured default */
-+ if (last_retry_after >= 0) {
-+ /* Check if retry delay exceeds maximum allowed */
-+ if (last_retry_after > http_max_retry_time) {
-+ error(_("rate limited (HTTP 429) requested %ld second delay, "
-+ "exceeds http.maxRetryTime of %ld seconds"),
-+ last_retry_after, http_max_retry_time);
-+ last_retry_after = -1; /* Reset after use */
-+ return HTTP_ERROR;
-+ }
-+ sleep_for_retry(last_retry_after);
-+ last_retry_after = -1; /* Reset after use */
-+ } 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 HTTP_ERROR;
-+ }
-+ /* Check if configured default exceeds maximum allowed */
-+ if (http_retry_after > http_max_retry_time) {
-+ error(_("configured http.retryAfter (%ld seconds) exceeds "
-+ "http.maxRetryTime (%ld seconds)"),
-+ http_retry_after, http_max_retry_time);
-+ return HTTP_ERROR;
-+ }
-+ /* Use configured default retry-after value */
-+ sleep_for_retry(http_retry_after);
-+ }
++ slot_retry_after = -1; /* Reset after use */
+ } else if (ret == HTTP_REAUTH) {
+ credential_fill(the_repository, &http_auth, 1);
+ }
- ret = http_request(url, result, target, options);
+- 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);
}
+ return ret;
+ }
+@@ http.c: int http_get_strbuf(const char *url,
+ struct strbuf *result,
+ struct http_get_options *options)
+ {
+- return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
++ return http_request_recoverable(url, result, HTTP_REQUEST_STRBUF, options);
+ }
+
+ /*
+@@ http.c: int http_get_file(const char *url, const char *filename,
+ goto cleanup;
+ }
+
+- ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
++ ret = http_request_recoverable(url, result, HTTP_REQUEST_FILE, options);
+ fclose(result);
+
+ if (ret == HTTP_OK && finalize_object_file(the_repository, tmpfile.buf, filename))
## http.h ##
@@ http.h: struct slot_results {
@@ 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)
- show_http_message(&type, &charset, &buffer);
- die(_("unable to access '%s' with http.pinnedPubkey configuration: %s"),
- transport_anonymize_url(url.buf), curl_errorstr);
+ 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);
+ case HTTP_RATE_LIMITED:
-+ show_http_message(&type, &charset, &buffer);
-+ die(_("rate limited by '%s', please try again later"),
-+ transport_anonymize_url(url.buf));
++ 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"),
+- 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;
+ }
+
+
+ ## t/lib-httpd.sh ##
+@@ t/lib-httpd.sh: prepare_httpd() {
+ install_script error.sh
+ install_script apply-one-time-script.sh
+ install_script nph-custom-auth.sh
++ install_script http-429.sh
+
+ ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules"
+
+
+ ## t/lib-httpd/apache.conf ##
+@@ t/lib-httpd/apache.conf: SetEnv PERL_PATH ${PERL_PATH}
+ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+ SetEnv GIT_HTTP_EXPORT_ALL
+ </LocationMatch>
++<LocationMatch /http_429/>
++ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
++ SetEnv GIT_HTTP_EXPORT_ALL
++</LocationMatch>
+ <LocationMatch /smart_v0/>
+ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+ SetEnv GIT_HTTP_EXPORT_ALL
+@@ t/lib-httpd/apache.conf: ScriptAlias /broken_smart/ broken-smart-http.sh/
+ ScriptAlias /error_smart/ error-smart-http.sh/
+ ScriptAlias /error/ error.sh/
+ ScriptAliasMatch /one_time_script/(.*) apply-one-time-script.sh/$1
++ScriptAliasMatch /http_429/(.*) http-429.sh/$1
+ ScriptAliasMatch /custom_auth/(.*) nph-custom-auth.sh/$1
+ <Directory ${GIT_EXEC_PATH}>
+ Options FollowSymlinks
+@@ t/lib-httpd/apache.conf: ScriptAliasMatch /custom_auth/(.*) nph-custom-auth.sh/$1
+ <Files apply-one-time-script.sh>
+ Options ExecCGI
+ </Files>
++<Files http-429.sh>
++ Options ExecCGI
++</Files>
+ <Files ${GIT_EXEC_PATH}/git-http-backend>
+ Options ExecCGI
+ </Files>
+
+ ## t/lib-httpd/http-429.sh (new) ##
+@@
++#!/bin/sh
++
++# Script to return HTTP 429 Too Many Requests responses for testing retry logic.
++# Usage: /http_429/<test-context>/<retry-after-value>/<repo-path>
++#
++# The test-context is a unique identifier for each test to isolate state files.
++# The retry-after-value can be:
++# - A number (e.g., "1", "2", "100") - sets Retry-After header to that many seconds
++# - "none" - no Retry-After header
++# - "invalid" - invalid Retry-After format
++# - "permanent" - always return 429 (never succeed)
++# - An HTTP-date string (RFC 2822 format) - sets Retry-After to that date
++#
++# On first call, returns 429. On subsequent calls (after retry), forwards to git-http-backend
++# unless retry-after-value is "permanent".
++
++# Extract test context, retry-after value and repo path from PATH_INFO
++# PATH_INFO format: /<test-context>/<retry-after-value>/<repo-path>
++path_info="${PATH_INFO#/}" # Remove leading slash
++test_context="${path_info%%/*}" # Get first component (test context)
++remaining="${path_info#*/}" # Get rest
++retry_after="${remaining%%/*}" # Get second component (retry-after value)
++repo_path="${remaining#*/}" # Get rest (repo path)
++
++# Extract repository name from repo_path (e.g., "repo.git" from "repo.git/info/refs")
++# The repo name is the first component before any "/"
++repo_name="${repo_path%%/*}"
++
++# Use current directory (HTTPD_ROOT_PATH) for state file
++# Create a safe filename from test_context, retry_after and repo_name
++# This ensures all requests for the same test context share the same state file
++safe_name=$(echo "${test_context}-${retry_after}-${repo_name}" | tr '/' '_' | tr -cd 'a-zA-Z0-9_-')
++state_file="http-429-state-${safe_name}"
++
++# Check if this is the first call (no state file exists)
++if test -f "$state_file"
++then
++ # Already returned 429 once, forward to git-http-backend
++ # Set PATH_INFO to just the repo path (without retry-after value)
++ # Set GIT_PROJECT_ROOT so git-http-backend can find the repository
++ # Use exec to replace this process so git-http-backend gets the updated environment
++ PATH_INFO="/$repo_path"
++ export PATH_INFO
++ # GIT_PROJECT_ROOT points to the document root where repositories are stored
++ # The script runs from HTTPD_ROOT_PATH, and www/ is the document root
++ if test -z "$GIT_PROJECT_ROOT"
++ then
++ # Construct path: current directory (HTTPD_ROOT_PATH) + /www
++ GIT_PROJECT_ROOT="$(pwd)/www"
++ export GIT_PROJECT_ROOT
++ fi
++ exec "$GIT_EXEC_PATH/git-http-backend"
++fi
++
++# Mark that we've returned 429
++touch "$state_file"
++
++# Output HTTP 429 response
++printf "Status: 429 Too Many Requests\r\n"
++
++# Set Retry-After header based on retry_after value
++case "$retry_after" in
++ none)
++ # No Retry-After header
++ ;;
++ invalid)
++ printf "Retry-After: invalid-format-123abc\r\n"
++ ;;
++ permanent)
++ # Always return 429, don't set state file for success
++ rm -f "$state_file"
++ printf "Retry-After: 1\r\n"
++ printf "Content-Type: text/plain\r\n"
++ printf "\r\n"
++ printf "Permanently rate limited\n"
++ exit 0
++ ;;
++ *)
++ # Check if it's a number
++ case "$retry_after" in
++ [0-9]*)
++ # Numeric value
++ printf "Retry-After: %s\r\n" "$retry_after"
++ ;;
++ *)
++ # Assume it's an HTTP-date format (passed as-is, URL decoded)
++ # Apache may URL-encode the path, so decode common URL-encoded characters
++ # %20 = space, %2C = comma, %3A = colon
++ retry_value=$(echo "$retry_after" | sed -e 's/%20/ /g' -e 's/%2C/,/g' -e 's/%3A/:/g')
++ printf "Retry-After: %s\r\n" "$retry_value"
++ ;;
++ esac
++ ;;
++esac
++
++printf "Content-Type: text/plain\r\n"
++printf "\r\n"
++printf "Rate limited\n"
## t/meson.build ##
@@ t/meson.build: integration_tests = [
@@ t/t5584-http-429-retry.sh (new)
+ git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config http.receivepack true
+'
+
-+test_expect_success 'HTTP 429 with retries disabled (maxRetries=0) fails immediately' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 1\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited\n"
-+ cat "$1" >/dev/null
-+ EOF
++# This test suite uses a special HTTP 429 endpoint at /http_429/ that simulates
++# rate limiting. The endpoint format is:
++# /http_429/<test-context>/<retry-after-value>/<repo-path>
++# The http-429.sh script (in t/lib-httpd) returns a 429 response with the
++# specified Retry-After header on the first request for each test context,
++# then forwards subsequent requests to git-http-backend. Each test context
++# is isolated, allowing multiple tests to run independently.
+
++test_expect_success 'HTTP 429 with retries disabled (maxRetries=0) fails immediately' '
+ # Set maxRetries to 0 (disabled)
+ test_config http.maxRetries 0 &&
+ test_config http.retryAfter 1 &&
+
+ # Should fail immediately without any retry attempt
-+ test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err &&
++ test_must_fail git ls-remote "$HTTPD_URL/http_429/retries-disabled/1/repo.git" 2>err &&
+
+ # Verify no retry happened (no "waiting" message in stderr)
-+ ! grep -i "waiting.*retry" err &&
-+
-+ # The one-time script will be consumed on first request (not a retry)
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep ! -i "waiting.*retry" err
+'
+
+test_expect_success 'HTTP 429 permanent should fail after max retries' '
-+ # Install a permanent error script to prove retries are limited
-+ write_script "$HTTPD_ROOT_PATH/http-429-permanent.sh" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 1\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Permanently rate limited\n"
-+ EOF
-+
+ # Enable retries with a limit
+ test_config http.maxRetries 2 &&
+
+ # Git should retry but eventually fail when 429 persists
-+ test_must_fail git ls-remote "$HTTPD_URL/error/http-429-permanent.sh/repo.git" 2>err
++ test_must_fail git ls-remote "$HTTPD_URL/http_429/permanent-fail/permanent/repo.git" 2>err
+'
+
+test_expect_success 'HTTP 429 with Retry-After is retried and succeeds' '
-+ # Create a one-time script that returns 429 with Retry-After header
-+ # on the first request. Subsequent requests will succeed.
-+ # This contrasts with the permanent 429 above - proving retry works
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ # Return HTTP 429 response instead of git response
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 1\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited - please retry after 1 second\n"
-+ # Output something different from input so the script gets removed
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Enable retries
+ test_config http.maxRetries 3 &&
+
+ # Git should retry after receiving 429 and eventually succeed
-+ git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
-+ test_grep "refs/heads/" output &&
-+
-+ # The one-time script should have been consumed (proving retry happened)
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ git ls-remote "$HTTPD_URL/http_429/retry-succeeds/1/repo.git" >output 2>err &&
++ test_grep "refs/heads/" output
+'
+
+test_expect_success 'HTTP 429 without Retry-After uses configured default' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited - no retry info\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Enable retries and configure default delay
+ test_config http.maxRetries 3 &&
+ test_config http.retryAfter 1 &&
+
+ # Git should retry using configured default and succeed
-+ git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
-+ test_grep "refs/heads/" output &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ git ls-remote "$HTTPD_URL/http_429/no-retry-after-header/none/repo.git" >output 2>err &&
++ test_grep "refs/heads/" output
+'
+
+test_expect_success 'HTTP 429 retry delays are respected' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 2\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Enable retries
+ test_config http.maxRetries 3 &&
+
+ # Time the operation - it should take at least 2 seconds due to retry delay
+ start=$(date +%s) &&
-+ git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
++ git ls-remote "$HTTPD_URL/http_429/retry-delays-respected/2/repo.git" >output 2>err &&
+ end=$(date +%s) &&
+ duration=$((end - start)) &&
+
+ # Verify it took at least 2 seconds (allowing some tolerance)
+ test "$duration" -ge 1 &&
-+ test_grep "refs/heads/" output &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "refs/heads/" output
+'
+
+test_expect_success 'HTTP 429 fails immediately if Retry-After exceeds http.maxRetryTime' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 100\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited with long delay\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Configure max retry time to 3 seconds (much less than requested 100)
+ test_config http.maxRetries 3 &&
+ test_config http.maxRetryTime 3 &&
+
+ # Should fail immediately without waiting
+ start=$(date +%s) &&
-+ test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err &&
++ 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)) &&
+
+ # Should fail quickly (less than 2 seconds, no 100 second wait)
+ test "$duration" -lt 2 &&
-+ test_grep "exceeds http.maxRetryTime" err &&
-+
-+ # The one-time script will be consumed on first request
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "exceeds http.maxRetryTime" err
+'
+
+test_expect_success 'HTTP 429 fails if configured http.retryAfter exceeds http.maxRetryTime' '
+ # Test misconfiguration: retryAfter > maxRetryTime
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited without header\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Configure retryAfter larger than maxRetryTime
+ test_config http.maxRetries 3 &&
+ test_config http.retryAfter 100 &&
@@ t/t5584-http-429-retry.sh (new)
+
+ # Should fail immediately with configuration error
+ start=$(date +%s) &&
-+ test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err &&
++ 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)) &&
+
@@ t/t5584-http-429-retry.sh (new)
+ test_done
+ fi &&
+
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF &&
-+ printf "Status: 429 Too Many Requests\\r\\n"
-+ printf "Retry-After: $future_date\\r\\n"
-+ printf "Content-Type: text/plain\\r\\n"
-+ printf "\\r\\n"
-+ printf "Rate limited with HTTP-date\\n"
-+ cat "\$1" >/dev/null
-+ EOF
++ # URL-encode the date (replace spaces with %20)
++ 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) &&
-+ git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
++ git ls-remote "$HTTPD_URL/http_429/http-date-format/$future_date_encoded/repo.git" >output 2>err &&
+ end=$(date +%s) &&
+ duration=$((end - start)) &&
+
+ # Should take at least 1 second (allowing tolerance for processing time)
+ test "$duration" -ge 1 &&
-+ test_grep "refs/heads/" output &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "refs/heads/" output
+'
+
+test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immediately' '
@@ t/t5584-http-429-retry.sh (new)
+ test_done
+ fi &&
+
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF &&
-+ printf "Status: 429 Too Many Requests\\r\\n"
-+ printf "Retry-After: $future_date\\r\\n"
-+ printf "Content-Type: text/plain\\r\\n"
-+ printf "\\r\\n"
-+ printf "Rate limited with long HTTP-date\\n"
-+ cat "\$1" >/dev/null
-+ EOF
++ # URL-encode the date (replace spaces with %20)
++ future_date_encoded=$(echo "$future_date" | sed "s/ /%20/g") &&
+
+ # Configure max retry time much less than the 200 second delay
+ test_config http.maxRetries 3 &&
@@ t/t5584-http-429-retry.sh (new)
+
+ # Should fail immediately without waiting 200 seconds
+ start=$(date +%s) &&
-+ test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err &&
++ 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)) &&
+
+ # Should fail quickly (not wait 200 seconds)
+ test "$duration" -lt 2 &&
-+ test_grep "exceeds http.maxRetryTime" err &&
-+
-+ # The one-time script will be consumed on first request
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "exceeds http.maxRetryTime" err
+'
+
+test_expect_success 'HTTP 429 with past HTTP-date should not wait' '
@@ t/t5584-http-429-retry.sh (new)
+ test_done
+ fi &&
+
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF &&
-+ printf "Status: 429 Too Many Requests\\r\\n"
-+ printf "Retry-After: $past_date\\r\\n"
-+ printf "Content-Type: text/plain\\r\\n"
-+ printf "\\r\\n"
-+ printf "Rate limited with past date\\n"
-+ cat "\$1" >/dev/null
-+ EOF
++ # URL-encode the date (replace spaces with %20)
++ 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) &&
-+ git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
++ git ls-remote "$HTTPD_URL/http_429/past-http-date/$past_date_encoded/repo.git" >output 2>err &&
+ end=$(date +%s) &&
+ duration=$((end - start)) &&
+
+ # Should complete quickly (less than 2 seconds)
+ test "$duration" -lt 2 &&
-+ test_grep "refs/heads/" output &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "refs/heads/" output
+'
+
+test_expect_success 'HTTP 429 with invalid Retry-After format uses configured default' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: invalid-format-123abc\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited with malformed header\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Configure default retry-after
+ test_config http.maxRetries 3 &&
+ test_config http.retryAfter 1 &&
+
+ # Should use configured default (1 second) since header is invalid
+ start=$(date +%s) &&
-+ git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
++ git ls-remote "$HTTPD_URL/http_429/invalid-retry-after-format/invalid/repo.git" >output 2>err &&
+ end=$(date +%s) &&
+ duration=$((end - start)) &&
+
+ # Should take at least 1 second (the configured default)
+ test "$duration" -ge 1 &&
+ test_grep "refs/heads/" output &&
-+ test_grep "waiting.*retry" err &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "waiting.*retry" err
+'
+
+test_expect_success 'HTTP 429 will not be retried without config' '
+ # Default config means http.maxRetries=0 (retries disabled)
+ # When 429 is received, it should fail immediately without retry
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 1\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Do NOT configure anything - use defaults (http.maxRetries defaults to 0)
+
+ # Should fail immediately without retry
-+ test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err &&
++ test_must_fail git ls-remote "$HTTPD_URL/http_429/no-retry-without-config/1/repo.git" 2>err &&
+
+ # Verify no retry happened (no "waiting" message)
-+ ! grep -i "waiting.*retry" err &&
++ test_grep ! -i "waiting.*retry" err &&
+
+ # Should get 429 error
-+ test_grep "429" err &&
-+
-+ # The one-time script should be consumed on first request
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "429" err
+'
+
+test_expect_success 'GIT_HTTP_RETRY_AFTER overrides http.retryAfter config' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited - no Retry-After header\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Configure retryAfter to 10 seconds
+ test_config http.maxRetries 3 &&
+ test_config http.retryAfter 10 &&
+
+ # Override with environment variable to 1 second
+ start=$(date +%s) &&
-+ GIT_HTTP_RETRY_AFTER=1 git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
++ 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)) &&
+
@@ t/t5584-http-429-retry.sh (new)
+ test "$duration" -ge 1 &&
+ test "$duration" -lt 5 &&
+ test_grep "refs/heads/" output &&
-+ test_grep "waiting.*retry" err &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "waiting.*retry" err
+'
+
+test_expect_success 'GIT_HTTP_MAX_RETRIES overrides http.maxRetries config' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 1\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Configure maxRetries to 0 (disabled)
+ test_config http.maxRetries 0 &&
+ test_config http.retryAfter 1 &&
+
+ # Override with environment variable to enable retries
-+ GIT_HTTP_MAX_RETRIES=3 git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err &&
++ GIT_HTTP_MAX_RETRIES=3 git ls-remote "$HTTPD_URL/http_429/env-max-retries-override/1/repo.git" >output 2>err &&
+
+ # Should retry (env var enables it despite config saying disabled)
+ test_grep "refs/heads/" output &&
-+ test_grep "waiting.*retry" err &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "waiting.*retry" err
+'
+
+test_expect_success 'GIT_HTTP_MAX_RETRY_TIME overrides http.maxRetryTime config' '
-+ write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF &&
-+ printf "Status: 429 Too Many Requests\r\n"
-+ printf "Retry-After: 50\r\n"
-+ printf "Content-Type: text/plain\r\n"
-+ printf "\r\n"
-+ printf "Rate limited with long delay\n"
-+ cat "$1" >/dev/null
-+ EOF
-+
+ # Configure maxRetryTime to 100 seconds (would accept 50 second delay)
+ test_config http.maxRetries 3 &&
+ test_config http.maxRetryTime 100 &&
@@ t/t5584-http-429-retry.sh (new)
+ # Override with environment variable to 10 seconds (should reject 50 second delay)
+ start=$(date +%s) &&
+ test_must_fail env GIT_HTTP_MAX_RETRY_TIME=10 \
-+ git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err &&
++ git ls-remote "$HTTPD_URL/http_429/env-max-retry-time-override/50/repo.git" 2>err &&
+ end=$(date +%s) &&
+ duration=$((end - start)) &&
+
+ # Should fail quickly (not wait 50 seconds) because env var limits to 10
+ test "$duration" -lt 5 &&
-+ test_grep "exceeds http.maxRetryTime" err &&
-+ test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script"
++ test_grep "exceeds http.maxRetryTime" err
+'
+
+test_expect_success 'verify normal repository access still works' '
2: 4382237922 < -: ---------- remote-curl: fix memory leak in show_http_message()
3: adbcc0251f ! 2: ad4495fc94 http: add trace2 logging for retry operations
@@ http.c
static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
static int trace_curl_data = 1;
+@@ http.c: void run_active_slot(struct active_request_slot *slot)
+
+ if (waiting_for_delay) {
+ warning(_("rate limited, waiting %ld seconds before retry"), slot->retry_delay_seconds);
++ trace2_data_intmax("http", the_repository, "http/retry-sleep-seconds",
++ slot->retry_delay_seconds);
+ start_time = slot->retry_delay_start;
+ }
+
+@@ http.c: void run_active_slot(struct active_request_slot *slot)
+ }
+
+ if (elapsed_time.tv_sec >= slot->retry_delay_seconds) {
++ trace2_region_leave("http", "retry-sleep", the_repository);
+ slot->retry_delay_seconds = -1;
+ waiting_for_delay = 0;
+
@@ http.c: static int handle_curl_result(struct slot_results *results)
+ return HTTP_REAUTH;
+ }
} else if (results->http_code == 429) {
- /* Store the retry_after value for use in retry logic */
- last_retry_after = results->retry_after;
+ trace2_data_intmax("http", the_repository, "http/429-retry-after",
-+ last_retry_after);
++ results->retry_after);
return HTTP_RATE_LIMITED;
} else {
if (results->http_connectcode == 407)
-@@ http.c: static void sleep_for_retry(long retry_after)
- if (retry_after > 0) {
- unsigned int remaining;
- warning(_("rate limited, waiting %ld seconds before retry"), retry_after);
-+ trace2_region_enter("http", "retry-sleep", the_repository);
-+ trace2_data_intmax("http", the_repository, "http/retry-sleep-seconds",
-+ retry_after);
- remaining = sleep(retry_after);
- while (remaining > 0) {
- /* Sleep was interrupted, continue sleeping */
- remaining = sleep(remaining);
+@@ http.c: static void sleep_for_retry(struct active_request_slot *slot, long retry_after)
+ 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");
}
-+ trace2_region_leave("http", "retry-sleep", the_repository);
+ return -1;
}
- }
-
-@@ http.c: static int http_request_reauth(const char *url,
- /* Handle rate limiting with retry logic */
- 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 HTTP_ERROR;
- }
-@@ http.c: static int http_request_reauth(const char *url,
- error(_("rate limited (HTTP 429) requested %ld second delay, "
- "exceeds http.maxRetryTime of %ld seconds"),
- last_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", last_retry_after);
- last_retry_after = -1; /* Reset after use */
- return HTTP_ERROR;
- }
-@@ http.c: static int http_request_reauth(const char *url,
- /* 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."));
-+ trace2_data_string("http", the_repository,
-+ "http/429-error", "no-retry-after-config");
- return HTTP_ERROR;
- }
-- /* Check if configured default exceeds maximum allowed */
-- if (http_retry_after > http_max_retry_time) {
-- error(_("configured http.retryAfter (%ld seconds) exceeds "
-- "http.maxRetryTime (%ld seconds)"),
-- http_retry_after, http_max_retry_time);
-- return HTTP_ERROR;
-- }
-- /* Use configured default retry-after value */
-- sleep_for_retry(http_retry_after);
-+ /* Check if configured default exceeds maximum allowed */
-+ if (http_retry_after > http_max_retry_time) {
-+ error(_("configured http.retryAfter (%ld seconds) exceeds "
-+ "http.maxRetryTime (%ld seconds)"),
-+ http_retry_after, http_max_retry_time);
-+ trace2_data_string("http", the_repository,
-+ "http/429-error", "config-exceeds-max-retry-time");
-+ return HTTP_ERROR;
-+ }
-+ /* Use configured default retry-after value */
+@@ http.c: static long handle_rate_limit_retry(int *rate_limit_retries, long slot_retry_aft
+ error(_("rate limited (HTTP 429) requested %ld second delay, "
+ "exceeds http.maxRetryTime of %ld seconds"),
+ slot_retry_after, http_max_retry_time);
+ trace2_data_string("http", the_repository,
-+ "http/429-retry-source", "config-default");
-+ sleep_for_retry(http_retry_after);
- }
- } else if (ret == HTTP_REAUTH) {
- credential_fill(the_repository, &http_auth, 1);
++ "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;
+@@ http.c: static long handle_rate_limit_retry(int *rate_limit_retries, long slot_retry_aft
+ /* 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."));
++ trace2_data_string("http", the_repository,
++ "http/429-error", "no-retry-after-config");
+ return -1;
+ }
+ /* Check if configured default exceeds maximum allowed */
+@@ http.c: static long handle_rate_limit_retry(int *rate_limit_retries, long slot_retry_aft
+ error(_("configured http.retryAfter (%ld seconds) exceeds "
+ "http.maxRetryTime (%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;
+ }
+ }
--
gitgitgadget
next prev parent reply other threads:[~2025-12-18 14:44 UTC|newest]
Thread overview: 10+ 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 ` Vaidas Pilkauskas via GitGitGadget [this message]
2025-12-18 14:44 ` [PATCH v2 1/2] http: add support for HTTP 429 rate limit retries Vaidas Pilkauskas via GitGitGadget
2025-12-18 14:44 ` [PATCH v2 2/2] http: add trace2 logging for retry operations Vaidas Pilkauskas via GitGitGadget
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.v2.git.1766069088.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=me@ttaylorr.com \
--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;
as well as URLs for NNTP newsgroup(s).