From 0ec7d3e42b8b74fd419d7997319fe724beb6c513 Mon Sep 17 00:00:00 2001
From: ethanstoner <ethanstoner08@gmail.com>
Date: Fri, 18 Sep 2026 10:56:59 -0700
Subject: [PATCH] Retry GCS 408, 502 and client timeouts like S3

gcs::is_retryable treated HTTP 408 and 502 as permanent and ignored the
client's own timeout errors, so a bad gateway on a manifest CAS failed
the push on GCS while the same response on S3 is retried (#18). Use the
status set GCS documents as retryable (408, 429, 500, 502, 503, 504),
leave 501 out, and share the same set with s3::is_transient_status.

(cherry picked from commit 939348a2d40fe59af8dc91c6de050b78b5b9e4a0)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
---
 crates/walgit-store/src/gcs.rs | 34 ++++++++++++++++++++++++++++++++--
 crates/walgit-store/src/s3.rs  |  6 +++---
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/crates/walgit-store/src/gcs.rs b/crates/walgit-store/src/gcs.rs
index fbe4a62c..4dd3b4f7 100644
--- a/crates/walgit-store/src/gcs.rs
+++ b/crates/walgit-store/src/gcs.rs
@@ -1201,9 +1201,13 @@ fn is_retryable(e: &google_cloud_storage::Error) -> bool {
         );
     }
     if let Some(code) = e.http_status_code() {
-        return matches!(code, 503 | 504 | 429 | 500);
+        // The statuses GCS documents as retryable. The client's own policy
+        // retries all of 5xx; 501 is left out because an unimplemented
+        // request will not start working on a second try. Same set as
+        // `s3::is_transient_status`.
+        return matches!(code, 408 | 429 | 500 | 502 | 503 | 504);
     }
-    e.is_connect() || e.is_io() || e.is_transient_and_before_rpc()
+    e.is_connect() || e.is_io() || e.is_timeout() || e.is_transient_and_before_rpc()
 }
 
 fn map_error(key: &str, e: google_cloud_storage::Error) -> StoreError {
@@ -1384,6 +1388,32 @@ mod tests {
         assert!(is_retryable(&e));
     }
 
+    fn http_error(status: u16) -> google_cloud_storage::Error {
+        google_cloud_gax::error::Error::http(status, http::HeaderMap::new(), bytes::Bytes::new())
+    }
+
+    #[test]
+    fn transient_http_statuses_are_retryable() {
+        for status in [408, 429, 500, 502, 503, 504] {
+            assert!(
+                is_retryable(&http_error(status)),
+                "{status} should be retryable"
+            );
+        }
+        for status in [400, 403, 404, 409, 412, 501] {
+            assert!(
+                !is_retryable(&http_error(status)),
+                "{status} should be permanent"
+            );
+        }
+    }
+
+    #[test]
+    fn is_retryable_timeout() {
+        let e = google_cloud_gax::error::Error::timeout("attempt timed out");
+        assert!(is_retryable(&e));
+    }
+
     #[test]
     fn is_not_retryable_not_found() {
         let e = google_cloud_gax::error::Error::service(
diff --git a/crates/walgit-store/src/s3.rs b/crates/walgit-store/src/s3.rs
index 08409d53..2138941b 100644
--- a/crates/walgit-store/src/s3.rs
+++ b/crates/walgit-store/src/s3.rs
@@ -261,7 +261,7 @@ fn is_transient_code(code: &str) -> bool {
 /// HTTP statuses that mean the same, for services whose error codes we do not
 /// recognise (rustfs and other S3-compatible stores do not all use AWS codes).
 fn is_transient_status(status: u16) -> bool {
-    matches!(status, 429 | 500 | 502 | 503 | 504)
+    matches!(status, 408 | 429 | 500 | 502 | 503 | 504)
 }
 
 /// Whether an SDK failure is worth another attempt at walgit's layer.
@@ -1303,10 +1303,10 @@ mod tests {
 
     #[test]
     fn transient_statuses_are_recognised() {
-        for status in [429, 500, 502, 503, 504] {
+        for status in [408, 429, 500, 502, 503, 504] {
             assert!(is_transient_status(status), "{status} should be transient");
         }
-        for status in [400, 403, 404, 409, 412] {
+        for status in [400, 403, 404, 409, 412, 501] {
             assert!(!is_transient_status(status), "{status} should be permanent");
         }
     }