Skip to content

Commit a154422

Browse files
authored
feat(network): use Retry-After header for HTTP 429 responses (#15463)
### What does this PR try to resolve? Cargo registries that return HTTP 429 when the service is overloaded expect the client to retry the request automatically after a delay. Cargo currently does not retry for HTTP 429. ### What changed? * Adds HTTP 429 (too many requests) as a spurious HTTP error to enable retries. * Parse the [Retry-After](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Retry-After) HTTP header to determine how long to wait before a retry. In this implementation, the maximum delay is limited to Cargo's existing limit of 10 seconds. We could consider increasing that limit for this case, since the server is explicitly requesting the delay.
2 parents b1bb014 + b72d647 commit a154422

File tree

3 files changed

+153
-4
lines changed

3 files changed

+153
-4
lines changed

crates/cargo-test-support/src/registry.rs

+13
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,19 @@ impl HttpServer {
10571057
}
10581058
}
10591059

1060+
/// Return too many requests (HTTP 429)
1061+
pub fn too_many_requests(&self, _req: &Request, delay: std::time::Duration) -> Response {
1062+
Response {
1063+
code: 429,
1064+
headers: vec![format!("Retry-After: {}", delay.as_secs())],
1065+
body: format!(
1066+
"too many requests, try again in {} seconds",
1067+
delay.as_secs()
1068+
)
1069+
.into_bytes(),
1070+
}
1071+
}
1072+
10601073
/// Serve the download endpoint
10611074
pub fn dl(&self, req: &Request) -> Response {
10621075
let file = self

src/cargo/util/network/retry.rs

+89-4
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ impl<'a> Retry<'a> {
104104
pub fn r#try<T>(&mut self, f: impl FnOnce() -> CargoResult<T>) -> RetryResult<T> {
105105
match f() {
106106
Err(ref e) if maybe_spurious(e) && self.retries < self.max_retries => {
107-
let err_msg = e
108-
.downcast_ref::<HttpNotSuccessful>()
107+
let err = e.downcast_ref::<HttpNotSuccessful>();
108+
let err_msg = err
109109
.map(|http_err| http_err.display_short())
110110
.unwrap_or_else(|| e.root_cause().to_string());
111111
let left_retries = self.max_retries - self.retries;
@@ -118,7 +118,12 @@ impl<'a> Retry<'a> {
118118
return RetryResult::Err(e);
119119
}
120120
self.retries += 1;
121-
RetryResult::Retry(self.next_sleep_ms())
121+
let sleep = err
122+
.and_then(|v| Self::parse_retry_after(v, &jiff::Timestamp::now()))
123+
// Limit the Retry-After to a maximum value to avoid waiting too long.
124+
.map(|retry_after| retry_after.min(MAX_RETRY_SLEEP_MS))
125+
.unwrap_or_else(|| self.next_sleep_ms());
126+
RetryResult::Retry(sleep)
122127
}
123128
Err(e) => RetryResult::Err(e),
124129
Ok(r) => RetryResult::Success(r),
@@ -141,6 +146,42 @@ impl<'a> Retry<'a> {
141146
)
142147
}
143148
}
149+
150+
/// Parse the HTTP `Retry-After` header.
151+
/// Returns the number of milliseconds to wait before retrying according to the header.
152+
fn parse_retry_after(response: &HttpNotSuccessful, now: &jiff::Timestamp) -> Option<u64> {
153+
// Only applies to HTTP 429 (too many requests) and 503 (service unavailable).
154+
if !matches!(response.code, 429 | 503) {
155+
return None;
156+
}
157+
158+
// Extract the Retry-After header value.
159+
let retry_after = response
160+
.headers
161+
.iter()
162+
.filter_map(|h| h.split_once(':'))
163+
.map(|(k, v)| (k.trim(), v.trim()))
164+
.find(|(k, _)| k.eq_ignore_ascii_case("retry-after"))?
165+
.1;
166+
167+
// First option: Retry-After is a positive integer of seconds to wait.
168+
if let Ok(delay_secs) = retry_after.parse::<u32>() {
169+
return Some(delay_secs as u64 * 1000);
170+
}
171+
172+
// Second option: Retry-After is a future HTTP date string that tells us when to retry.
173+
if let Ok(retry_time) = jiff::fmt::rfc2822::parse(retry_after) {
174+
let diff_ms = now
175+
.until(&retry_time)
176+
.unwrap()
177+
.total(jiff::Unit::Millisecond)
178+
.unwrap();
179+
if diff_ms > 0.0 {
180+
return Some(diff_ms as u64);
181+
}
182+
}
183+
None
184+
}
144185
}
145186

146187
fn maybe_spurious(err: &Error) -> bool {
@@ -169,7 +210,7 @@ fn maybe_spurious(err: &Error) -> bool {
169210
}
170211
}
171212
if let Some(not_200) = err.downcast_ref::<HttpNotSuccessful>() {
172-
if 500 <= not_200.code && not_200.code < 600 {
213+
if 500 <= not_200.code && not_200.code < 600 || not_200.code == 429 {
173214
return true;
174215
}
175216
}
@@ -317,3 +358,47 @@ fn curle_http2_stream_is_spurious() {
317358
let err = curl::Error::new(code);
318359
assert!(maybe_spurious(&err.into()));
319360
}
361+
362+
#[test]
363+
fn retry_after_parsing() {
364+
use crate::core::Shell;
365+
fn spurious(code: u32, header: &str) -> HttpNotSuccessful {
366+
HttpNotSuccessful {
367+
code,
368+
url: "Uri".to_string(),
369+
ip: None,
370+
body: Vec::new(),
371+
headers: vec![header.to_string()],
372+
}
373+
}
374+
375+
// Start of year 2025.
376+
let now = jiff::Timestamp::new(1735689600, 0).unwrap();
377+
let headers = spurious(429, "Retry-After: 10");
378+
assert_eq!(Retry::parse_retry_after(&headers, &now), Some(10_000));
379+
let headers = spurious(429, "retry-after: Wed, 01 Jan 2025 00:00:10 GMT");
380+
let actual = Retry::parse_retry_after(&headers, &now).unwrap();
381+
assert_eq!(10000, actual);
382+
383+
let headers = spurious(429, "Content-Type: text/html");
384+
assert_eq!(Retry::parse_retry_after(&headers, &now), None);
385+
386+
let headers = spurious(429, "retry-after: Fri, 01 Jan 2000 00:00:00 GMT");
387+
assert_eq!(Retry::parse_retry_after(&headers, &now), None);
388+
389+
let headers = spurious(429, "retry-after: -1");
390+
assert_eq!(Retry::parse_retry_after(&headers, &now), None);
391+
392+
let headers = spurious(400, "retry-after: 1");
393+
assert_eq!(Retry::parse_retry_after(&headers, &now), None);
394+
395+
let gctx = GlobalContext::default().unwrap();
396+
*gctx.shell() = Shell::from_write(Box::new(Vec::new()));
397+
let mut retry = Retry::new(&gctx).unwrap();
398+
match retry
399+
.r#try(|| -> CargoResult<()> { Err(anyhow::Error::from(spurious(429, "Retry-After: 7"))) })
400+
{
401+
RetryResult::Retry(sleep) => assert_eq!(sleep, 7_000),
402+
_ => panic!("unexpected non-retry"),
403+
}
404+
}

tests/testsuite/registry.rs

+51
Original file line numberDiff line numberDiff line change
@@ -3881,6 +3881,57 @@ fn dl_retry_multiple() {
38813881
.run();
38823882
}
38833883

3884+
#[cargo_test]
3885+
fn retry_too_many_requests() {
3886+
let fail_count = Mutex::new(0);
3887+
let _registry = RegistryBuilder::new()
3888+
.http_index()
3889+
.add_responder("/index/3/b/bar", move |req, server| {
3890+
let mut fail_count = fail_count.lock().unwrap();
3891+
if *fail_count < 1 {
3892+
*fail_count += 1;
3893+
server.too_many_requests(req, std::time::Duration::from_secs(1))
3894+
} else {
3895+
server.index(req)
3896+
}
3897+
})
3898+
.build();
3899+
3900+
let p = project()
3901+
.file(
3902+
"Cargo.toml",
3903+
r#"
3904+
[package]
3905+
name = "foo"
3906+
version = "0.0.1"
3907+
edition = "2015"
3908+
authors = []
3909+
3910+
[dependencies]
3911+
bar = ">= 0.0.0"
3912+
"#,
3913+
)
3914+
.file("src/main.rs", "fn main() {}")
3915+
.build();
3916+
3917+
Package::new("bar", "0.0.1").publish();
3918+
3919+
p.cargo("check")
3920+
.with_stderr_data(str![[r#"
3921+
[UPDATING] `dummy-registry` index
3922+
[WARNING] spurious network error (3 tries remaining): failed to get successful HTTP response from `[..]/index/3/b/bar` ([..]), got 429
3923+
body:
3924+
too many requests, try again in 1 seconds
3925+
[LOCKING] 1 package to latest compatible version
3926+
[DOWNLOADING] crates ...
3927+
[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
3928+
[CHECKING] bar v0.0.1
3929+
[CHECKING] foo v0.0.1 ([ROOT]/foo)
3930+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
3931+
3932+
"#]]).run();
3933+
}
3934+
38843935
#[cargo_test]
38853936
fn deleted_entry() {
38863937
// Checks the behavior when a package is removed from the index.

0 commit comments

Comments
 (0)