From 00de43528c67cf365a6e45f9f779aa46d16eebe6 Mon Sep 17 00:00:00 2001
From: Zhengchao An <anzhengchao@gmail.com>
Date: Tue, 18 Aug 2026 09:55:29 +0800
Subject: [PATCH] fix(ecstore): describe peer bucket RPC failures with no
 details (#6190)

heal_bucket, list_bucket, get_bucket_info and delete_bucket returned Error::other("") when a peer answered success=false without an error payload, so operators saw a bare "io error " after quorum reduction. Route all five bucket RPCs through peer_failure_without_details, which names the operation and bucket while staying identical across the peers of one operation so reduce_errs keeps grouping them into a single dominant error.
---
 .../ecstore/src/cluster/rpc/peer_s3_client.rs | 58 ++++++++++++++++---
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/crates/ecstore/src/cluster/rpc/peer_s3_client.rs b/crates/ecstore/src/cluster/rpc/peer_s3_client.rs
index de7f0e595..02c9f75f1 100644
--- a/crates/ecstore/src/cluster/rpc/peer_s3_client.rs
+++ b/crates/ecstore/src/cluster/rpc/peer_s3_client.rs
@@ -214,6 +214,19 @@ fn pool_write_quorum(participant_count: usize) -> usize {
     (participant_count / 2) + 1
 }
 
+/// Error for a peer that reported `success = false` without an error payload.
+///
+/// The message must stay identical across the peers of one operation: `reduce_errs`
+/// buckets `Error::Io` by kind plus rendered message, so any per-peer detail (address,
+/// timing) would split one shared failure into single-count buckets and downgrade a real
+/// dominant error into `ErasureWriteQuorum`.
+fn peer_failure_without_details(op: &str, bucket: Option<&str>) -> Error {
+    match bucket {
+        Some(bucket) => Error::other(format!("{op}({bucket}): peer returned failure without error details")),
+        None => Error::other(format!("{op}: peer returned failure without error details")),
+    }
+}
+
 fn reduce_pool_write_quorum_errs(per_pool_errs: &[Option<Error>]) -> Option<Error> {
     if per_pool_errs.is_empty() {
         return Some(Error::ErasureWriteQuorum);
@@ -1078,7 +1091,7 @@ impl PeerS3Client for RemotePeerS3Client {
                     return if let Some(err) = response.error {
                         Err(err.into())
                     } else {
-                        Err(Error::other(""))
+                        Err(peer_failure_without_details("heal_bucket", Some(bucket)))
                     };
                 }
 
@@ -1105,7 +1118,7 @@ impl PeerS3Client for RemotePeerS3Client {
                     return if let Some(err) = response.error {
                         Err(err.into())
                     } else {
-                        Err(Error::other(""))
+                        Err(peer_failure_without_details("list_bucket", None))
                     };
                 }
                 let bucket_infos = response
@@ -1136,9 +1149,7 @@ impl PeerS3Client for RemotePeerS3Client {
                     return if let Some(err) = response.error {
                         Err(err.into())
                     } else {
-                        Err(Error::other(format!(
-                            "make_bucket({bucket}): peer returned failure without error details"
-                        )))
+                        Err(peer_failure_without_details("make_bucket", Some(bucket)))
                     };
                 }
 
@@ -1162,7 +1173,7 @@ impl PeerS3Client for RemotePeerS3Client {
                     return if let Some(err) = response.error {
                         Err(err.into())
                     } else {
-                        Err(Error::other(""))
+                        Err(peer_failure_without_details("get_bucket_info", Some(bucket)))
                     };
                 }
                 let bucket_info = serde_json::from_str::<BucketInfo>(&response.bucket_info)?;
@@ -1190,7 +1201,7 @@ impl PeerS3Client for RemotePeerS3Client {
                     return if let Some(err) = response.error {
                         Err(err.into())
                     } else {
-                        Err(Error::other(""))
+                        Err(peer_failure_without_details("delete_bucket", Some(bucket)))
                     };
                 }
 
@@ -2314,4 +2325,37 @@ mod tests {
             .collect::<Vec<_>>();
         assert_eq!(calls, vec![1, 1, 0, 0, 0, 0, 0, 0]);
     }
+
+    #[test]
+    fn peer_failure_without_details_names_operation_and_bucket() {
+        for op in ["heal_bucket", "make_bucket", "get_bucket_info", "delete_bucket"] {
+            let message = peer_failure_without_details(op, Some("ops-bucket")).to_string();
+            assert!(message.contains(op), "{op} message must name the operation: {message}");
+            assert!(message.contains("ops-bucket"), "{op} message must name the bucket: {message}");
+        }
+
+        let message = peer_failure_without_details("list_bucket", None).to_string();
+        assert!(message.contains("list_bucket"), "cluster-wide message must name the operation");
+        assert!(!message.trim().is_empty());
+    }
+
+    #[test]
+    fn peer_failure_without_details_keeps_one_reduce_errs_bucket_per_operation() {
+        // reduce_errs groups Io errors by kind plus rendered message: peers failing the
+        // same operation on the same bucket must still reach quorum as one dominant error.
+        let per_pool_errs = vec![
+            Some(peer_failure_without_details("delete_bucket", Some("shared"))),
+            Some(peer_failure_without_details("delete_bucket", Some("shared"))),
+            Some(peer_failure_without_details("delete_bucket", Some("shared"))),
+        ];
+        assert_eq!(
+            reduce_pool_write_quorum_errs(&per_pool_errs),
+            Some(peer_failure_without_details("delete_bucket", Some("shared")))
+        );
+
+        assert_ne!(
+            peer_failure_without_details("delete_bucket", Some("shared")),
+            peer_failure_without_details("get_bucket_info", Some("shared"))
+        );
+    }
 }