feat(network): use Retry-After header for HTTP 429 responses

Author: arlosi

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

@@ -1057,6 +1057,19 @@ impl HttpServer {
         }
     }
 
+    /// Return too many requests (HTTP 429)
+    pub fn too_many_requests(&self, _req: &Request, delay: std::time::Duration) -> Response {
+        Response {
+            code: 429,
+            headers: vec![format!("Retry-After: {}", delay.as_secs())],
+            body: format!(
+                "too many requests, try again in {} seconds",
+                delay.as_secs()
+            )
+            .into_bytes(),
+        }
+    }
+
     /// Serve the download endpoint
     pub fn dl(&self, req: &Request) -> Response {
         let file = self

src/cargo/util/network/retry.rs

@@ -104,8 +104,8 @@ impl<'a> Retry<'a> {
     pub fn r#try<T>(&mut self, f: impl FnOnce() -> CargoResult<T>) -> RetryResult<T> {
         match f() {
             Err(ref e) if maybe_spurious(e) && self.retries < self.max_retries => {
-                let err_msg = e
-                    .downcast_ref::<HttpNotSuccessful>()
+                let err = e.downcast_ref::<HttpNotSuccessful>();
+                let err_msg = err
                     .map(|http_err| http_err.display_short())
                     .unwrap_or_else(|| e.root_cause().to_string());
                 let left_retries = self.max_retries - self.retries;
@@ -118,7 +118,12 @@ impl<'a> Retry<'a> {
                     return RetryResult::Err(e);
                 }
                 self.retries += 1;
-                RetryResult::Retry(self.next_sleep_ms())
+                let sleep = err
+                    .and_then(|v| Self::parse_retry_after(v, &jiff::Timestamp::now()))
+                    // Limit the Retry-After to a maximum value to avoid waiting too long.
+                    .map(|retry_after| retry_after.min(MAX_RETRY_SLEEP_MS))
+                    .unwrap_or_else(|| self.next_sleep_ms());
+                RetryResult::Retry(sleep)
             }
             Err(e) => RetryResult::Err(e),
             Ok(r) => RetryResult::Success(r),
@@ -141,6 +146,42 @@ impl<'a> Retry<'a> {
             )
         }
     }
+
+    /// Parse the HTTP `Retry-After` header.
+    /// Returns the number of milliseconds to wait before retrying according to the header.
+    fn parse_retry_after(response: &HttpNotSuccessful, now: &jiff::Timestamp) -> Option<u64> {
+        // Only applies to HTTP 429 (too many requests) and 503 (service unavailable).
+        if !matches!(response.code, 429 | 503) {
+            return None;
+        }
+
+        // Extract the Retry-After header value.
+        let retry_after = response
+            .headers
+            .iter()
+            .filter_map(|h| h.split_once(':'))
+            .map(|(k, v)| (k.trim(), v.trim()))
+            .find(|(k, _)| k.eq_ignore_ascii_case("retry-after"))?
+            .1;
+
+        // First option: Retry-After is a positive integer of seconds to wait.
+        if let Ok(delay_secs) = retry_after.parse::<u32>() {
+            return Some(delay_secs as u64 * 1000);
+        }
+
+        // Second option: Retry-After is a future HTTP date string that tells us when to retry.
+        if let Ok(retry_time) = jiff::fmt::rfc2822::parse(retry_after) {
+            let diff_ms = now
+                .until(&retry_time)
+                .unwrap()
+                .total(jiff::Unit::Millisecond)
+                .unwrap();
+            if diff_ms > 0.0 {
+                return Some(diff_ms as u64);
+            }
+        }
+        None
+    }
 }
 
 fn maybe_spurious(err: &Error) -> bool {
@@ -169,7 +210,7 @@ fn maybe_spurious(err: &Error) -> bool {
         }
     }
     if let Some(not_200) = err.downcast_ref::<HttpNotSuccessful>() {
-        if 500 <= not_200.code && not_200.code < 600 {
+        if 500 <= not_200.code && not_200.code < 600 || not_200.code == 429 {
             return true;
         }
     }
@@ -317,3 +358,47 @@ fn curle_http2_stream_is_spurious() {
     let err = curl::Error::new(code);
     assert!(maybe_spurious(&err.into()));
 }
+
+#[test]
+fn retry_after_parsing() {
+    use crate::core::Shell;
+    fn spurious(code: u32, header: &str) -> HttpNotSuccessful {
+        HttpNotSuccessful {
+            code,
+            url: "Uri".to_string(),
+            ip: None,
+            body: Vec::new(),
+            headers: vec![header.to_string()],
+        }
+    }
+
+    // Start of year 2025.
+    let now = jiff::Timestamp::new(1735689600, 0).unwrap();
+    let headers = spurious(429, "Retry-After: 10");
+    assert_eq!(Retry::parse_retry_after(&headers, &now), Some(10_000));
+    let headers = spurious(429, "retry-after: Wed, 01 Jan 2025 00:00:10 GMT");
+    let actual = Retry::parse_retry_after(&headers, &now).unwrap();
+    assert_eq!(10000, actual);
+
+    let headers = spurious(429, "Content-Type: text/html");
+    assert_eq!(Retry::parse_retry_after(&headers, &now), None);
+
+    let headers = spurious(429, "retry-after: Fri, 01 Jan 2000 00:00:00 GMT");
+    assert_eq!(Retry::parse_retry_after(&headers, &now), None);
+
+    let headers = spurious(429, "retry-after: -1");
+    assert_eq!(Retry::parse_retry_after(&headers, &now), None);
+
+    let headers = spurious(400, "retry-after: 1");
+    assert_eq!(Retry::parse_retry_after(&headers, &now), None);
+
+    let gctx = GlobalContext::default().unwrap();
+    *gctx.shell() = Shell::from_write(Box::new(Vec::new()));
+    let mut retry = Retry::new(&gctx).unwrap();
+    match retry
+        .r#try(|| -> CargoResult<()> { Err(anyhow::Error::from(spurious(429, "Retry-After: 7"))) })
+    {
+        RetryResult::Retry(sleep) => assert_eq!(sleep, 7_000),
+        _ => panic!("unexpected non-retry"),
+    }
+}

tests/testsuite/registry.rs

@@ -3881,6 +3881,57 @@ fn dl_retry_multiple() {
         .run();
 }
 
+#[cargo_test]
+fn retry_too_many_requests() {
+    let fail_count = Mutex::new(0);
+    let _registry = RegistryBuilder::new()
+        .http_index()
+        .add_responder("/index/3/b/bar", move |req, server| {
+            let mut fail_count = fail_count.lock().unwrap();
+            if *fail_count < 1 {
+                *fail_count += 1;
+                server.too_many_requests(req, std::time::Duration::from_secs(1))
+            } else {
+                server.index(req)
+            }
+        })
+        .build();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.0.1"
+                edition = "2015"
+                authors = []
+
+                [dependencies]
+                bar = ">= 0.0.0"
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    Package::new("bar", "0.0.1").publish();
+
+    p.cargo("check")
+    .with_stderr_data(str![[r#"
+[UPDATING] `dummy-registry` index
+[WARNING] spurious network error (3 tries remaining): failed to get successful HTTP response from `[..]/index/3/b/bar` ([..]), got 429
+body:
+too many requests, try again in 1 seconds
+[LOCKING] 1 package to latest compatible version
+[DOWNLOADING] crates ...
+[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
+[CHECKING] bar v0.0.1
+[CHECKING] foo v0.0.1 ([ROOT]/foo)
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+
+"#]]).run();
+}
+
 #[cargo_test]
 fn deleted_entry() {
     // Checks the behavior when a package is removed from the index.