GODRIVER-1867 Add new Details field on WriteError for server v5.0 (#694)

Author: matthewdale

mongo/collection.go

@@ -376,12 +376,8 @@ func (coll *Collection) InsertMany(ctx context.Context, documents []interface{},
 	bwErrors := make([]BulkWriteError, 0, len(writeException.WriteErrors))
 	for _, we := range writeException.WriteErrors {
 		bwErrors = append(bwErrors, BulkWriteError{
-			WriteError{
-				Index:   we.Index,
-				Code:    we.Code,
-				Message: we.Message,
-			},
-			nil,
+			WriteError: we,
+			Request:    nil,
 		})
 	}
 

mongo/errors.go

@@ -275,31 +275,39 @@ type WriteError struct {
 
 	Code    int
 	Message string
+	Details bson.Raw
 }
 
-func (we WriteError) Error() string { return we.Message }
+func (we WriteError) Error() string {
+	msg := we.Message
+	if len(we.Details) > 0 {
+		msg = fmt.Sprintf("%s: %s", msg, we.Details.String())
+	}
+	return msg
+}
 
 // WriteErrors is a group of write errors that occurred during execution of a write operation.
 type WriteErrors []WriteError
 
 // Error implements the error interface.
 func (we WriteErrors) Error() string {
-	var buf bytes.Buffer
-	fmt.Fprint(&buf, "write errors: [")
-	for idx, err := range we {
-		if idx != 0 {
-			fmt.Fprintf(&buf, ", ")
-		}
-		fmt.Fprintf(&buf, "{%s}", err)
+	errs := make([]error, len(we))
+	for i := 0; i < len(we); i++ {
+		errs[i] = we[i]
 	}
-	fmt.Fprint(&buf, "]")
-	return buf.String()
+	// WriteErrors isn't returned from batch operations, but we can still use the same formatter.
+	return "write errors: " + joinBatchErrors(errs)
 }
 
 func writeErrorsFromDriverWriteErrors(errs driver.WriteErrors) WriteErrors {
 	wes := make(WriteErrors, 0, len(errs))
 	for _, err := range errs {
-		wes = append(wes, WriteError{Index: int(err.Index), Code: int(err.Code), Message: err.Message})
+		wes = append(wes, WriteError{
+			Index:   int(err.Index),
+			Code:    int(err.Code),
+			Message: err.Message,
+			Details: bson.Raw(err.Details),
+		})
 	}
 	return wes
 }
@@ -336,11 +344,21 @@ type WriteException struct {
 
 // Error implements the error interface.
 func (mwe WriteException) Error() string {
-	var buf bytes.Buffer
-	fmt.Fprint(&buf, "multiple write errors: [")
-	fmt.Fprintf(&buf, "{%s}, ", mwe.WriteErrors)
-	fmt.Fprintf(&buf, "{%s}]", mwe.WriteConcernError)
-	return buf.String()
+	causes := make([]string, 0, 2)
+	if mwe.WriteConcernError != nil {
+		causes = append(causes, "write concern error: "+mwe.WriteConcernError.Error())
+	}
+	if len(mwe.WriteErrors) > 0 {
+		// The WriteErrors error message already starts with "write errors:", so don't add it to the
+		// error message again.
+		causes = append(causes, mwe.WriteErrors.Error())
+	}
+
+	message := "write exception: "
+	if len(causes) == 0 {
+		return message + "no causes"
+	}
+	return message + strings.Join(causes, ", ")
 }
 
 // HasErrorCode returns true if the error has the specified code.
@@ -420,9 +438,7 @@ type BulkWriteError struct {
 
 // Error implements the error interface.
 func (bwe BulkWriteError) Error() string {
-	var buf bytes.Buffer
-	fmt.Fprintf(&buf, "{%s}", bwe.WriteError)
-	return buf.String()
+	return bwe.WriteError.Error()
 }
 
 // BulkWriteException is the error type returned by BulkWrite and InsertMany operations.
@@ -439,11 +455,23 @@ type BulkWriteException struct {
 
 // Error implements the error interface.
 func (bwe BulkWriteException) Error() string {
-	var buf bytes.Buffer
-	fmt.Fprint(&buf, "bulk write error: [")
-	fmt.Fprintf(&buf, "{%s}, ", bwe.WriteErrors)
-	fmt.Fprintf(&buf, "{%s}]", bwe.WriteConcernError)
-	return buf.String()
+	causes := make([]string, 0, 2)
+	if bwe.WriteConcernError != nil {
+		causes = append(causes, "write concern error: "+bwe.WriteConcernError.Error())
+	}
+	if len(bwe.WriteErrors) > 0 {
+		errs := make([]error, len(bwe.WriteErrors))
+		for i := 0; i < len(bwe.WriteErrors); i++ {
+			errs[i] = &bwe.WriteErrors[i]
+		}
+		causes = append(causes, "write errors: "+joinBatchErrors(errs))
+	}
+
+	message := "bulk write exception: "
+	if len(causes) == 0 {
+		return message + "no causes"
+	}
+	return "bulk write exception: " + strings.Join(causes, ", ")
 }
 
 // HasErrorCode returns true if any of the errors have the specified code.
@@ -539,3 +567,34 @@ func processWriteError(err error) (returnResult, error) {
 		return rrAll, nil
 	}
 }
+
+// batchErrorsTargetLength is the target length of error messages returned by batch operation
+// error types. Try to limit batch error messages to 2kb to prevent problems when printing error
+// messages from large batch operations.
+const batchErrorsTargetLength = 2000
+
+// joinBatchErrors appends messages from the given errors to a comma-separated string. If the
+// string exceeds 2kb, it stops appending error messages and appends the message "+N more errors..."
+// to the end.
+//
+// Example format:
+//     "[message 1, message 2, +8 more errors...]"
+func joinBatchErrors(errs []error) string {
+	var buf bytes.Buffer
+	fmt.Fprint(&buf, "[")
+	for idx, err := range errs {
+		if idx != 0 {
+			fmt.Fprint(&buf, ", ")
+		}
+		// If the error message has exceeded the target error message length, stop appending errors
+		// to the message and append the number of remaining errors instead.
+		if buf.Len() > batchErrorsTargetLength {
+			fmt.Fprintf(&buf, "+%d more errors...", len(errs)-idx)
+			break
+		}
+		fmt.Fprint(&buf, err.Error())
+	}
+	fmt.Fprint(&buf, "]")
+
+	return buf.String()
+}

mongo/errors_test.go

@@ -0,0 +1,66 @@
+package mongo
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.mongodb.org/mongo-driver/bson"
+)
+
+func TestErrorMessages(t *testing.T) {
+	details, err := bson.Marshal(bson.D{{"details", bson.D{{"operatorName", "$jsonSchema"}}}})
+	require.Nil(t, err, "unexpected error marshaling BSON")
+
+	cases := []struct {
+		desc     string
+		err      error
+		expected string
+	}{
+		{
+			desc: "WriteException error message should contain the WriteError Message and Details",
+			err: WriteException{
+				WriteErrors: WriteErrors{
+					{
+						Message: "test message 1",
+						Details: details,
+					},
+					{
+						Message: "test message 2",
+						Details: details,
+					},
+				},
+			},
+			expected: `write exception: write errors: [test message 1: {"details": {"operatorName": "$jsonSchema"}}, test message 2: {"details": {"operatorName": "$jsonSchema"}}]`,
+		},
+		{
+			desc: "BulkWriteException error message should contain the WriteError Message and Details",
+			err: BulkWriteException{
+				WriteErrors: []BulkWriteError{
+					{
+						WriteError: WriteError{
+							Message: "test message 1",
+							Details: details,
+						},
+					},
+					{
+						WriteError: WriteError{
+							Message: "test message 2",
+							Details: details,
+						},
+					},
+				},
+			},
+			expected: `bulk write exception: write errors: [test message 1: {"details": {"operatorName": "$jsonSchema"}}, test message 2: {"details": {"operatorName": "$jsonSchema"}}]`,
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
+			assert.Equal(t, tc.expected, tc.err.Error())
+		})
+	}
+}