CDRIVER-901 update bson_error_t from write concern err

Author: ajdavis

src/mongoc/mongoc-error.h

@@ -44,6 +44,7 @@ typedef enum
    MONGOC_ERROR_GRIDFS,
    MONGOC_ERROR_SCRAM,
    MONGOC_ERROR_SERVER_SELECTION,
+   MONGOC_ERROR_WRITE_CONCERN,
 } mongoc_error_domain_t;
 
 

src/mongoc/mongoc-write-command.c

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <bson.h>
 
 #include "mongoc-client-private.h"
 #include "mongoc-error.h"
@@ -1330,26 +1331,86 @@ _mongoc_write_result_merge (mongoc_write_result_t  *result,  /* IN */
 }
 
 
+/*
+ * If error is not set, set code from first document in array like
+ * [{"code": 64, "errmsg": "duplicate"}, ...]. Format the error message
+ * from all errors in array.
+*/
+static void
+_set_error_from_response (bson_t *bson_array,
+                          mongoc_error_domain_t domain,
+                          const char *error_type,
+                          bson_error_t *error /* OUT */)
+{
+   bson_iter_t array_iter;
+   bson_iter_t doc_iter;
+   bson_string_t *compound_err;
+   const char *errmsg = NULL;
+   int32_t code = 0;
+   uint32_t n_keys, i;
+
+   compound_err = bson_string_new (NULL);
+   n_keys = bson_count_keys (bson_array);
+   if (n_keys > 1) {
+      bson_string_append_printf (compound_err,
+                                 "Multiple %s errors: ",
+                                 error_type);
+   }
+
+   if (!bson_empty0 (bson_array) && bson_iter_init (&array_iter, bson_array)) {
+
+      /* get first code and all error messages */
+      i = 0;
+
+      while (bson_iter_next (&array_iter)) {
+         if (BSON_ITER_HOLDS_DOCUMENT (&array_iter) &&
+             bson_iter_recurse (&array_iter, &doc_iter)) {
+
+            /* parse doc, which is like {"code": 64, "errmsg": "duplicate"} */
+            while (bson_iter_next (&doc_iter)) {
+
+               /* use the first error code we find */
+               if (BSON_ITER_IS_KEY (&doc_iter, "code") && code == 0) {
+                  code = bson_iter_int32 (&doc_iter);
+               } else if (BSON_ITER_IS_KEY (&doc_iter, "errmsg")) {
+                  errmsg = bson_iter_utf8 (&doc_iter, NULL);
+
+                  /* build message like 'Multiple write errors: "foo", "bar"' */
+                  if (n_keys > 1) {
+                     bson_string_append_printf (compound_err, "\"%s\"", errmsg);
+                     if (i < n_keys - 1) {
+                        bson_string_append (compound_err, ", ");
+                     }
+                  } else {
+                     /* single error message */
+                     bson_string_append (compound_err, errmsg);
+                  }
+               }
+            }
+
+            i++;
+         }
+      }
+
+      if (code && compound_err->len) {
+         bson_set_error (error, domain, (uint32_t) code,
+                         "%s", compound_err->str);
+      }
+   }
+
+   bson_string_free (compound_err, true);
+}
+
+
 bool
 _mongoc_write_result_complete (mongoc_write_result_t *result,
                                bson_t                *bson,
                                bson_error_t          *error)
 {
-   bson_iter_t iter;
-   bson_iter_t citer;
-   const char *err = NULL;
-   uint32_t code = 0;
-   bool ret;
-
    ENTRY;
 
    BSON_ASSERT (result);
 
-   ret = (!result->failed &&
-          /* TODO: not to spec */
-          bson_empty0 (&result->writeConcernErrors) &&
-          bson_empty0 (&result->writeErrors));
-
    if (bson) {
       BSON_APPEND_INT32 (bson, "nInserted", result->nInserted);
       BSON_APPEND_INT32 (bson, "nMatched", result->nMatched);
@@ -1368,27 +1429,22 @@ _mongoc_write_result_complete (mongoc_write_result_t *result,
       }
    }
 
-   if (error) {
-      memcpy (error, &result->error, sizeof *error);
+   /* set bson_error_t from first write error or write concern error */
+   _set_error_from_response (&result->writeErrors,
+                             MONGOC_ERROR_COMMAND,
+                             "write",
+                             &result->error);
+
+   if (!result->error.code) {
+      _set_error_from_response (&result->writeConcernErrors,
+                                MONGOC_ERROR_WRITE_CONCERN,
+                                "write concern",
+                                &result->error);
    }
 
-   if (!ret &&
-       !bson_empty0 (&result->writeErrors) &&
-       bson_iter_init (&iter, &result->writeErrors) &&
-       bson_iter_next (&iter) &&
-       BSON_ITER_HOLDS_DOCUMENT (&iter) &&
-       bson_iter_recurse (&iter, &citer)) {
-      while (bson_iter_next (&citer)) {
-         if (BSON_ITER_IS_KEY (&citer, "errmsg")) {
-            err = bson_iter_utf8 (&citer, NULL);
-         } else if (BSON_ITER_IS_KEY (&citer, "code")) {
-            code = bson_iter_int32 (&citer);
-         }
-      }
-      if (err && code) {
-         bson_set_error (error, MONGOC_ERROR_COMMAND, code, "%s", err);
-      }
+   if (error) {
+      memcpy (error, &result->error, sizeof *error);
    }
 
-   RETURN (ret);
+   RETURN (!result->failed && result->error.code == 0);
 }

tests/test-bulk.c

@@ -1528,7 +1528,7 @@ test_single_error_unordered_bulk ()
 
 
 static void
-_test_write_concern (bool has_write_commands, bool ordered)
+_test_write_concern (bool has_write_commands, bool ordered, bool multi_err)
 {
    mock_server_t *mock_server;
    mongoc_client_t *client;
@@ -1539,6 +1539,8 @@ _test_write_concern (bool has_write_commands, bool ordered)
    bson_error_t error;
    future_t *future;
    request_t *request;
+   int32_t first_err;
+   int32_t second_err;
 
    /* set wire protocol version for legacy writes or write commands */
    mock_server = mock_server_with_autoismaster (has_write_commands ? 3 : 0);
@@ -1550,6 +1552,7 @@ _test_write_concern (bool has_write_commands, bool ordered)
    mongoc_write_concern_set_wtimeout (wc, 100);
    bulk = mongoc_collection_create_bulk_operation (collection, ordered, wc);
    mongoc_bulk_operation_insert (bulk, tmp_bson ("{'_id': 1}"));
+   mongoc_bulk_operation_remove (bulk, tmp_bson ("{'_id': 2}"));
 
    future = future_bulk_operation_execute (bulk, &reply, &error);
 
@@ -1565,43 +1568,109 @@ _test_write_concern (bool has_write_commands, bool ordered)
          ordered ? "true" : "false");
 
       assert (request);
-      mock_server_replies (
-         request, 0, 0, 0, 1,
+      mock_server_replies_simple (
+         request,
          "{'ok': 1.0, 'n': 1, "
          " 'writeConcernError': {'code': 17, 'errmsg': 'foo'}}");
+
+      request_destroy (request);
+      request = mock_server_receives_command (
+         mock_server,
+         "test",
+         MONGOC_QUERY_NONE,
+         "{'delete': 'test',"
+            " 'writeConcern': {'w': 2, 'wtimeout': 100},"
+            " 'ordered': %s,"
+            " 'deletes': [{'q': {'_id': 2}, 'limit': 0}]}",
+         ordered ? "true" : "false");
+
+      if (multi_err) {
+         mock_server_replies_simple (
+            request,
+            "{'ok': 1.0, 'n': 1, "
+            " 'writeConcernError': {'code': 42, 'errmsg': 'bar'}}");
+      } else {
+         mock_server_replies_simple (request, "{'ok': 1.0, 'n': 1}");
+      }
+
+      request_destroy (request);
+
+      /* server fictionally returns 17 and 42; expect driver to use first one */
+      first_err = 17;
+      second_err = 42;
    } else {
       request = mock_server_receives_insert (
          mock_server, "test.test", MONGOC_INSERT_NONE, "{'_id': 1}");
 
       request_destroy (request);
-
       request = mock_server_receives_command (
          mock_server,
          "test",
          MONGOC_QUERY_NONE,
          "{'getLastError': 1, 'w': 2, 'wtimeout': 100}");
 
       assert (request);
-      mock_server_replies (
-         request, 0, 0, 0, 1,
-         "{'ok': 1.0, 'n': 0, 'err': 'foo', 'wtimeout': true}");
+      mock_server_replies_simple (
+         request, "{'ok': 1.0, 'n': 0, 'err': 'foo', 'wtimeout': true}");
+
+      request = mock_server_receives_delete (
+         mock_server, "test.test", MONGOC_REMOVE_NONE, "{'_id': 1}");
+
+      request_destroy (request);
+      request = mock_server_receives_command (
+         mock_server,
+         "test",
+         MONGOC_QUERY_NONE,
+         "{'getLastError': 1, 'w': 2, 'wtimeout': 100}");
+
+      if (multi_err) {
+         mock_server_replies_simple (
+            request, "{'ok': 1.0, 'n': 0, 'err': 'bar', 'wtimeout': true}");
+      } else {
+         mock_server_replies_simple (request, "{'ok': 1.0, 'n': 1}");
+      }
+
+      request_destroy (request);
+
+      /* The client makes up the error code for legacy writes */
+      first_err = second_err = 64;
    }
 
    /* join thread, assert mongoc_bulk_operation_execute () returned 0 */
    assert (!future_get_uint32_t (future));
 
-   ASSERT_MATCH (&reply, "{'nInserted': 1,"
-                         " 'nMatched': 0,"
-                         " 'nRemoved': 0,"
-                         " 'nUpserted': 0,"
-                         " 'writeErrors': [],"
-                         " 'writeConcernErrors': ["
-                         "     {'code': %d, 'errmsg': 'foo'}]}",
-                         has_write_commands ? 17 : 64);
+   if (multi_err) {
+      ASSERT_MATCH (&reply,
+                    "{'nInserted': 1,"
+                    " 'nMatched': 0,"
+                    " 'nRemoved': 1,"
+                    " 'nUpserted': 0,"
+                    " 'writeErrors': [],"
+                    " 'writeConcernErrors': ["
+                    "     {'code': %d, 'errmsg': 'foo'},"
+                    "     {'code': %d, 'errmsg': 'bar'}]}",
+                    first_err, second_err);
+
+      ASSERT_CMPSTR ("Multiple write concern errors: \"foo\", \"bar\"",
+                     error.message);
+   } else {
+      ASSERT_MATCH (&reply,
+                    "{'nInserted': 1,"
+                    " 'nMatched': 0,"
+                    " 'nRemoved': 1,"
+                    " 'nUpserted': 0,"
+                    " 'writeErrors': [],"
+                    " 'writeConcernErrors': ["
+                    "     {'code': %d, 'errmsg': 'foo'}]}",
+                    first_err);
+      ASSERT_CMPSTR ("foo", error.message);
+   }
 
    check_n_modified (has_write_commands, &reply, 0);
 
-   request_destroy (request);
+   ASSERT_CMPINT (MONGOC_ERROR_WRITE_CONCERN, ==, error.domain);
+   ASSERT_CMPINT (first_err, ==, error.code);
+
    future_destroy (future);
    bson_destroy (&reply);
    mongoc_bulk_operation_destroy (bulk);
@@ -1630,14 +1699,28 @@ test_write_concern_legacy_unordered (void)
 static void
 test_write_concern_write_command_ordered (void)
 {
-   _test_write_concern (true, true);
+   _test_write_concern (true, true, false);
+}
+
+
+static void
+test_write_concern_write_command_ordered_multi_err (void)
+{
+   _test_write_concern (true, true, true);
 }
 
 
 static void
 test_write_concern_write_command_unordered (void)
 {
-   _test_write_concern (true, false);
+   _test_write_concern (true, false, false);
+}
+
+
+static void
+test_write_concern_write_command_unordered_multi_err (void)
+{
+   _test_write_concern (true, false, true);
 }
 
 
@@ -2549,8 +2632,12 @@ test_bulk_install (TestSuite *suite)
 #endif
    TestSuite_Add (suite, "/BulkOperation/write_concern/write_command/ordered",
                   test_write_concern_write_command_ordered);
+   TestSuite_Add (suite, "/BulkOperation/write_concern/write_command/ordered/multi_err",
+                  test_write_concern_write_command_ordered_multi_err);
    TestSuite_Add (suite, "/BulkOperation/write_concern/write_command/unordered",
                   test_write_concern_write_command_unordered);
+   TestSuite_Add (suite, "/BulkOperation/write_concern/write_command/unordered/multi_err",
+                  test_write_concern_write_command_unordered_multi_err);
    TestSuite_Add (suite, "/BulkOperation/multiple_error_unordered_bulk",
                   test_multiple_error_unordered_bulk);
 #ifdef TODO_CDRIVER_707