fix(ecstore): stop reduce_errs leaking nil placeholder as dominant error (#4551)

reduce_errs seeds best_err with a synthetic Error::other("nil") placeholder
via unwrap_or. When the error slice is empty or every error is ignored,
err_counts is empty and nil_count is 0, so both nil tie-break conditions are
false and the else branch returned (0, Some(Error::other("nil"))). That
placeholder leaked to build_write_quorum_failure_summary, where dominant_error
became Some("nil"), skipping the nil_dominated early return and falling back to
the "other_error" metric label. Quorum decisions were unaffected (max_count 0
still fails any positive quorum), but write-quorum failure metric labels and
tracing fields were misclassified.

Add an explicit short-circuit: when best_count == 0 and nil_count == 0, return
the (0, None) sentinel so the placeholder never escapes. Add regression tests
for the all-ignored slice, empty slice, and the summary label path, which the
existing test_build_write_quorum_failure_summary (nil_count > 0) did not cover.

Fixes #957

Co-authored-by: heihutu <heihutu@gmail.com>

crates/ecstore/src/disk/error_reduce.rs

@@ -82,6 +82,14 @@ pub fn reduce_errs(errors: &[Option<Error>], ignored_errs: &[Error]) -> (usize,
         .max_by(|(_, c1), (_, c2)| c1.cmp(c2))
         .unwrap_or((nil_error, 0));
 
+    // Guard against the empty/all-ignored case: when there is no real error and
+    // no nil value, the seeded placeholder `nil_error` must not leak out as the
+    // dominant error. Return the `None` sentinel so callers (e.g. write-quorum
+    // failure summaries and their metric labels) treat it as "no dominant error".
+    if best_count == 0 && nil_count == 0 {
+        return (0, None);
+    }
+
     // Compare nil errors with the top non-nil error and prefer the nil error
     if nil_count > best_count || (nil_count == best_count && nil_count > 0) {
         (nil_count, None)
@@ -281,6 +289,45 @@ mod tests {
         assert_eq!(summary.dominant_error_label, "connection_reset");
     }
 
+    #[test]
+    fn test_reduce_errs_all_ignored_returns_none() {
+        // All disks return an ignored error (e.g. every disk offline): err_counts
+        // ends up empty and nil_count == 0. The synthetic "nil" placeholder must
+        // not leak; the function must return the (0, None) sentinel.
+        let errors = vec![
+            Some(Error::DiskNotFound),
+            Some(Error::DiskNotFound),
+            Some(Error::DiskNotFound),
+        ];
+        let (count, err) = reduce_errs(&errors, OBJECT_OP_IGNORED_ERRS);
+        assert_eq!(count, 0);
+        assert_eq!(err, None);
+    }
+
+    #[test]
+    fn test_reduce_errs_empty_slice_returns_none() {
+        // Empty slice: no errors and no nil values -> (0, None), not the placeholder.
+        let errors: Vec<Option<Error>> = vec![];
+        let ignored: Vec<Error> = vec![];
+        let (count, err) = reduce_errs(&errors, &ignored);
+        assert_eq!(count, 0);
+        assert_eq!(err, None);
+    }
+
+    #[test]
+    fn test_build_write_quorum_failure_summary_all_ignored_is_nil_dominated() {
+        // All disks offline (all DiskNotFound). The dominant error must be None and
+        // the label must be "nil_dominated" rather than falling back to "other_error".
+        let errors = vec![
+            Some(Error::DiskNotFound),
+            Some(Error::DiskNotFound),
+            Some(Error::DiskNotFound),
+        ];
+        let summary = build_write_quorum_failure_summary(&errors, OBJECT_OP_IGNORED_ERRS, 2);
+        assert_eq!(summary.dominant_error, None);
+        assert_eq!(summary.dominant_error_label, "nil_dominated");
+    }
+
     #[test]
     fn test_reduce_errs_nil_tiebreak() {
         // Error::Nil and another error have the same count, should prefer Nil