Changes since v4:
* fix only strbuf_attach() calls which don't need reallocation
* remove patch, which enforces strbuf_attach() contract via BUG()
Changes since v3:
* Clean up of all strbuf_attach() call sites
* Add strbuf_attach() contract enforcement via BUG()
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(), passing len+1 instead of len so that the existing
buffer is reused rather than immediately reallocated. 2. A cleanup of strbuf_attach() call sites that were passing alloc == len,
leaving no room for the NUL terminator. Sites with a
known-NUL-terminated buffer now pass len+1; sites where the source
buffer has no trailing NUL (ll_merge output) are converted to use
strbuf_add() instead. 3. A new show_http_message_fatal() helper in remote-curl.c that combines
the repeated pattern of show_http_message() followed by die() into a
single NORETURN function, reducing boilerplate at existing call sites
and providing a clean hook for the retry logic. 4. 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. If any
computed delay exceeds maxRetryTime the request fails immediately with a
clear diagnostic rather than capping and retrying silently.Vaidas Pilkauskas (4):
strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode()
strbuf_attach: fix call sites to pass correct alloc
remote-curl: introduce show_http_message_fatal() helper
http: add support for HTTP 429 rate limit retries
Documentation/config/http.adoc | 23 +++
builtin/am.c | 2 +-
builtin/fast-import.c | 2 +-
git-curl-compat.h | 8 +
http.c | 190 +++++++++++++++++++++--
http.h | 2 +
mailinfo.c | 2 +-
refs/files-backend.c | 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 +++++++++++++++++++++++++++++++++
trailer.c | 2 +-
16 files changed, 623 insertions(+), 35 deletions(-)
create mode 100644 t/lib-httpd/http-429.sh
create mode 100755 t/t5584-http-429-retry.sh
base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2008%2Fvaidas-shopify%2Fretry-after-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2008/vaidas-shopify/retry-after-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/2008
Range-diff vs v4:
1: a3386f5b56 = 1: 7ec2d66447 strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode()
2: f48b1f07c4 ! 2: 3e0b78cfb6 strbuf_attach: fix all call sites to pass correct alloc
@@ Metadata
Author: Vaidas Pilkauskas <vaidas.pilkauskas@shopify.com>
## Commit message ##
- strbuf_attach: fix all call sites to pass correct alloc
+ strbuf_attach: fix call sites to pass correct alloc
strbuf_attach(sb, buf, len, alloc) requires alloc > len (the buffer
must have at least len+1 bytes to hold the NUL). Several call sites
passed alloc == len, relying on strbuf_grow(sb, 0) inside strbuf_attach
- to reallocate. Prepare for changing that by fixing call sites to pass
- the correct alloc.
-
- - mailinfo, am, refs/files-backend, fast-import, trailer: pass len+1
- when the buffer is a NUL-terminated string (or from strbuf_detach).
- - rerere, apply: ll_merge returns a buffer with exactly result.size
- bytes (no extra NUL). Use strbuf_add() to copy and NUL-terminate
- into the strbuf, then free the merge result, so alloc is correct.
+ to reallocate. Fix these in mailinfo, am, refs/files-backend,
+ fast-import, and trailer by passing len+1 when the buffer is a
+ NUL-terminated string (or from strbuf_detach).
Signed-off-by: Vaidas Pilkauskas <vaidas.pilkauskas@shopify.com>
- ## apply.c ##
-@@ apply.c: static int three_way_merge(struct apply_state *state,
- return -1;
- }
- image_clear(image);
-- strbuf_attach(&image->buf, result.ptr, result.size, result.size);
-+ strbuf_add(&image->buf, result.ptr, result.size);
-+ free(result.ptr);
-
- return status;
- }
-
## builtin/am.c ##
@@ builtin/am.c: static void am_append_signoff(struct am_state *state)
{
@@ refs/files-backend.c: static int commit_ref(struct ref_lock *lock)
/*
* If this fails, commit_lock_file() will also fail
- ## rerere.c ##
-@@ rerere.c: static int handle_cache(struct index_state *istate,
- else
- io.io.output = NULL;
- strbuf_init(&io.input, 0);
-- strbuf_attach(&io.input, result.ptr, result.size, result.size);
-+ strbuf_add(&io.input, result.ptr, result.size);
-+ free(result.ptr);
-
- /*
- * Grab the conflict ID and optionally write the original
-
## trailer.c ##
@@ trailer.c: static struct trailer_block *trailer_block_get(const struct process_trailer_opti
for (ptr = trailer_lines; *ptr; ptr++) {
3: 557fd77444 < -: ---------- strbuf: replace strbuf_grow() in strbuf_attach() with BUG() check
4: 3a39dc9e39 = 3: 973703e9dd remote-curl: introduce show_http_message_fatal() helper
5: 5e0f4a56ef = 4: bfee1f10c0 http: add support for HTTP 429 rate limit retriesVaidas Pilkauskas via GitGitGadget <gitgitgadget@gmail.com>