SERVER-58399 Fix serialization logic for 'errInfo' field produced by DocumentValidationError

(cherry picked from commit 74f795cdebd943bf8d6a56005dde4e2189cfb76e)

3 files changed, 60 insertions, 10 deletions

diff --git a/src/mongo/s/write_ops/batched_command_response.cpp b/src/mongo/s/write_ops/batched_command_response.cpp
index 6886422b6a5..cbb9bee253d 100644
--- a/src/mongo/s/write_ops/batched_command_response.cpp
+++ b/src/mongo/s/write_ops/batched_command_response.cpp
@@ -131,17 +131,22 @@ BSONObj BatchedCommandResponse::toBSON() const {
             BSONObjBuilder errDetailsDocument(errDetailsBuilder.subobjStart());
 
             if (writeError->isIndexSet())
-                builder.append(WriteErrorDetail::index(), writeError->getIndex());
+                errDetailsDocument.append(WriteErrorDetail::index(), writeError->getIndex());
 
             auto status = writeError->toStatus();
-            builder.append(WriteErrorDetail::errCode(), status.code());
-            builder.append(WriteErrorDetail::errCodeName(), status.codeString());
-            builder.append(WriteErrorDetail::errMessage(), errorMessage(status.reason()));
-            if (auto extra = _status.extraInfo())
-                extra->serialize(&builder);  // TODO consider extra info size for truncation.
-
-            if (writeError->isErrInfoSet())
-                builder.append(WriteErrorDetail::errInfo(), writeError->getErrInfo());
+
+            errDetailsDocument.append(WriteErrorDetail::errCode(), status.code());
+            errDetailsDocument.append(WriteErrorDetail::errCodeName(), status.codeString());
+            errDetailsDocument.append(WriteErrorDetail::errMessage(),
+                                      errorMessage(status.reason()));
+            if (auto extra = status.extraInfo())
+                extra->serialize(
+                    &errDetailsDocument);  // TODO consider extra info size for truncation.
+
+            // Only set 'errInfo' if it hasn't been added by serializing 'extra'.
+            if (writeError->isErrInfoSet() &&
+                !errDetailsDocument.hasField(WriteErrorDetail::errInfo()))
+                errDetailsDocument.append(WriteErrorDetail::errInfo(), writeError->getErrInfo());
         }
         errDetailsBuilder.done();
     }
diff --git a/src/mongo/s/write_ops/batched_command_response_test.cpp b/src/mongo/s/write_ops/batched_command_response_test.cpp
index cd3fc8bcb32..7088c2fbfe4 100644
--- a/src/mongo/s/write_ops/batched_command_response_test.cpp
+++ b/src/mongo/s/write_ops/batched_command_response_test.cpp
@@ -142,5 +142,49 @@ TEST(BatchedCommandResponse, TooManyBigErrors) {
     }
 }
 
+TEST(BatchedCommandResponse, NoDuplicateErrInfo) {
+    auto verifySingleErrInfo = [](const BSONObj& obj) {
+        size_t errInfo = 0;
+        for (auto elem : obj) {
+            if (elem.fieldNameStringData() == WriteErrorDetail::errInfo()) {
+                ++errInfo;
+            }
+        }
+        ASSERT_EQ(errInfo, 1) << "serialized obj with duplicate errInfo " << obj.toString();
+    };
+
+    // Construct a WriteErrorDetail.
+    Status s(ErrorCodes::DocumentValidationFailure,
+             "Document failed validation",
+             BSON("errInfo" << BSON("detailed"
+                                    << "error message")));
+    BSONObjBuilder b;
+    s.serialize(&b);
+    WriteErrorDetail wed;
+    wed.setIndex(0);
+
+    // Verify it produces a single errInfo.
+    wed.parseBSON(b.obj(), nullptr);
+    BSONObj bsonWed = wed.toBSON();
+    verifySingleErrInfo(bsonWed);
+
+    BSONObjBuilder bcrBuilder;
+    bcrBuilder.append("ok", 1);
+    bcrBuilder.append("writeErrors", BSON_ARRAY(bsonWed));
+
+    // Construct a 'BatchedCommandResponse' using the above 'bsonWed'.
+    BatchedCommandResponse bcr;
+    bcr.parseBSON(bcrBuilder.obj(), nullptr);
+    BSONObj bsonBcr = bcr.toBSON();
+    auto writeErrors = bsonBcr[BatchedCommandResponse::writeErrors()];
+    ASSERT(!writeErrors.eoo());
+    ASSERT_EQ(writeErrors.type(), BSONType::Array);
+
+    // Verify that the entry in the 'writeErrors' array produces one 'errInfo' field.
+    for (auto&& elem : writeErrors.Array()) {
+        ASSERT_EQ(elem.type(), BSONType::Object);
+        verifySingleErrInfo(elem.embeddedObject());
+    }
+}
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/s/write_ops/write_error_detail.cpp b/src/mongo/s/write_ops/write_error_detail.cpp
index d51618fe25b..0c4587b82c0 100644
--- a/src/mongo/s/write_ops/write_error_detail.cpp
+++ b/src/mongo/s/write_ops/write_error_detail.cpp
@@ -82,7 +82,8 @@ BSONObj WriteErrorDetail::toBSON() const {
     if (auto extra = _status.extraInfo())
         extra->serialize(&builder);
 
-    if (_isErrInfoSet)
+    // Only set 'errInfo' if it hasn't been added by serializing 'extra'.
+    if (_isErrInfoSet && !builder.hasField(errInfo()))
         builder.append(errInfo(), _errInfo);
 
     return builder.obj();