fix(core): Handle GCS Response retriable errors

core/lib/default_da_clients/src/object_store/client.rs

@@ -40,7 +40,7 @@ impl DataAvailabilityClient for ObjectStoreDAClient {
             .await
         {
             return Err(DAError {
-                is_transient: err.is_transient(),
+                is_transient: err.is_retriable(),
                 error: anyhow::Error::from(err),
             });
         }
@@ -66,7 +66,7 @@ impl DataAvailabilityClient for ObjectStoreDAClient {
             }
 
             return Err(DAError {
-                is_transient: err.is_transient(),
+                is_transient: err.is_retriable(),
                 error: anyhow::Error::from(err),
             });
         }

core/lib/object_store/src/file.rs

@@ -10,7 +10,7 @@ impl From<io::Error> for ObjectStoreError {
         match err.kind() {
             io::ErrorKind::NotFound => ObjectStoreError::KeyNotFound(err.into()),
             kind => ObjectStoreError::Other {
-                is_transient: matches!(kind, io::ErrorKind::Interrupted | io::ErrorKind::TimedOut),
+                is_retriable: matches!(kind, io::ErrorKind::Interrupted | io::ErrorKind::TimedOut),
                 source: err.into(),
             },
         }

core/lib/object_store/src/gcs.rs

@@ -85,22 +85,22 @@ impl GoogleCloudStore {
 
 impl From<AuthError> for ObjectStoreError {
     fn from(err: AuthError) -> Self {
-        let is_transient = matches!(
+        let is_retriable = matches!(
             &err,
-            AuthError::HttpError(err) if is_transient_http_error(err)
+            AuthError::HttpError(err) if is_retriable_http_error(err)
         );
         Self::Initialization {
             source: err.into(),
-            is_transient,
+            is_retriable,
         }
     }
 }
 
-fn is_transient_http_error(err: &reqwest::Error) -> bool {
+fn is_retriable_http_error(err: &reqwest::Error) -> bool {
     err.is_timeout()
         || err.is_connect()
         // Not all request errors are logically transient, but a significant part of them are (e.g.,
-        // `hyper` protocol-level errors), and it's safer to consider an error transient.
+        // `hyper` protocol-level errors), and it's safer to consider an error retriable.
         || err.is_request()
         || has_transient_io_source(err)
         || err.status() == Some(StatusCode::BAD_GATEWAY)
@@ -117,8 +117,8 @@ fn get_source<'a, T: StdError + 'static>(mut err: &'a (dyn StdError + 'static))
 }
 
 fn has_transient_io_source(err: &(dyn StdError + 'static)) -> bool {
-    // We treat any I/O errors as transient. This isn't always true, but frequently occurring I/O errors
-    // (e.g., "connection reset by peer") *are* transient, and treating an error as transient is a safer option,
+    // We treat any I/O errors as retriable. This isn't always true, but frequently occurring I/O errors
+    // (e.g., "connection reset by peer") *are* transient, and treating an error as retriable is a safer option,
     // even if it can lead to unnecessary retries.
     get_source::<io::Error>(err).is_some()
 }
@@ -136,19 +136,20 @@ impl From<HttpError> for ObjectStoreError {
         if is_not_found {
             ObjectStoreError::KeyNotFound(err.into())
         } else {
-            let is_transient = match &err {
-                HttpError::HttpClient(err) => is_transient_http_error(err),
+            let is_retriable = match &err {
+                HttpError::HttpClient(err) => is_retriable_http_error(err),
                 HttpError::TokenSource(err) => {
-                    // Token sources are mostly based on the `reqwest` HTTP client, so transient error detection
+                    // Token sources are mostly based on the `reqwest` HTTP client, so retriable error detection
                     // can reuse the same logic.
                     let err = err.as_ref();
                     has_transient_io_source(err)
-                        || get_source::<reqwest::Error>(err).is_some_and(is_transient_http_error)
+                        || get_source::<reqwest::Error>(err).is_some_and(is_retriable_http_error)
                 }
+                HttpError::Response(err) => err.is_retriable(),
                 _ => false,
             };
             ObjectStoreError::Other {
-                is_transient,
+                is_retriable,
                 source: err.into(),
             }
         }

core/lib/object_store/src/raw.rs

@@ -59,7 +59,7 @@ pub enum ObjectStoreError {
     /// Object store initialization failed.
     Initialization {
         source: BoxedError,
-        is_transient: bool,
+        is_retriable: bool,
     },
     /// An object with the specified key is not found.
     KeyNotFound(BoxedError),
@@ -68,16 +68,16 @@ pub enum ObjectStoreError {
     /// Other error has occurred when accessing the store (e.g., a network error).
     Other {
         source: BoxedError,
-        is_transient: bool,
+        is_retriable: bool,
     },
 }
 
 impl ObjectStoreError {
-    /// Gives a best-effort estimate whether this error is transient.
-    pub fn is_transient(&self) -> bool {
+    /// Gives a best-effort estimate whether this error is retriable.
+    pub fn is_retriable(&self) -> bool {
         match self {
-            Self::Initialization { is_transient, .. } | Self::Other { is_transient, .. } => {
-                *is_transient
+            Self::Initialization { is_retriable, .. } | Self::Other { is_retriable, .. } => {
+                *is_retriable
             }
             Self::KeyNotFound(_) | Self::Serialization(_) => false,
         }
@@ -89,9 +89,9 @@ impl fmt::Display for ObjectStoreError {
         match self {
             Self::Initialization {
                 source,
-                is_transient,
+                is_retriable,
             } => {
-                let kind = if *is_transient { "transient" } else { "fatal" };
+                let kind = if *is_retriable { "retriable" } else { "fatal" };
                 write!(
                     formatter,
                     "{kind} error initializing object store: {source}"
@@ -101,9 +101,9 @@ impl fmt::Display for ObjectStoreError {
             Self::Serialization(err) => write!(formatter, "serialization error: {err}"),
             Self::Other {
                 source,
-                is_transient,
+                is_retriable,
             } => {
-                let kind = if *is_transient { "transient" } else { "fatal" };
+                let kind = if *is_retriable { "retriable" } else { "fatal" };
                 write!(formatter, "{kind} error accessing object store: {source}")
             }
         }

core/lib/object_store/src/retries.rs

@@ -39,7 +39,7 @@ impl Request<'_> {
         let result = loop {
             match f().await {
                 Ok(result) => break Ok(result),
-                Err(err) if err.is_transient() => {
+                Err(err) if err.is_retriable() => {
                     if retries > max_retries {
                         tracing::warn!(%err, "Exhausted {max_retries} retries performing request; returning last error");
                         break Err(err);
@@ -142,7 +142,7 @@ mod test {
 
     fn transient_error() -> ObjectStoreError {
         ObjectStoreError::Other {
-            is_transient: true,
+            is_retriable: true,
             source: "oops".into(),
         }
     }

core/lib/snapshots_applier/src/lib.rs

@@ -82,7 +82,7 @@ enum SnapshotsApplierError {
 
 impl SnapshotsApplierError {
     fn object_store(err: ObjectStoreError, context: String) -> Self {
-        if err.is_transient() {
+        if err.is_retriable() {
             Self::Retryable(anyhow::Error::from(err).context(context))
         } else {
             Self::Fatal(anyhow::Error::from(err).context(context))

core/lib/snapshots_applier/src/tests/mod.rs

@@ -61,7 +61,7 @@ async fn snapshots_creator_can_successfully_recover_db(
                 Ok(()) // "recover" after 3 retries
             } else {
                 Err(ObjectStoreError::Other {
-                    is_transient: true,
+                    is_retriable: true,
                     source: "transient error".into(),
                 })
             }
@@ -550,7 +550,7 @@ async fn applier_returns_error_after_too_many_object_store_retries() {
     let (object_store, client) = prepare_clients(&expected_status, &storage_logs).await;
     let object_store = ObjectStoreWithErrors::new(object_store, |_| {
         Err(ObjectStoreError::Other {
-            is_transient: true,
+            is_retriable: true,
             source: "service not available".into(),
         })
     });

core/node/block_reverter/src/tests.rs

@@ -382,7 +382,7 @@ impl ObjectStore for ErroneousStore {
             .unwrap()
             .remove(&(bucket, key.to_owned()));
         Err(ObjectStoreError::Other {
-            is_transient: false,
+            is_retriable: false,
             source: "fatal error".into(),
         })
     }