CDRIVER-2706 set error on invalidated server

kevinAlbs · kevinAlbs · commit 72687f63557a · 2018-06-21T12:41:27.000-04:00
diff --git a/src/libmongoc/src/mongoc/mongoc-change-stream.c b/src/libmongoc/src/mongoc/mongoc-change-stream.c
@@ -41,29 +41,29 @@
 static bool
 _is_resumable_error (const bson_t *reply)
 {
-   const char *msg = "";
-   uint32_t code;
+   bson_error_t error;
 
    /* Change Streams Spec resumable criteria: "any error encountered which is
     * not a server error (e.g. a timeout error or network error)" */
    if (bson_empty (reply)) {
       return true;
    }
 
-   if (!_mongoc_parse_error_reply (reply, false /* check_wce */, &code, &msg)) {
+   if (_mongoc_cmd_check_ok (reply, MONGOC_ERROR_API_VERSION_2, &error)) {
       return true;
    }
 
    /* Change Streams Spec resumable criteria: "a server error response with an
     * error message containing the substring 'not master' or 'node is
     * recovering' */
-   if (strstr (msg, "not master") || strstr (msg, "node is recovering")) {
+   if (strstr (error.message, "not master") ||
+       strstr (error.message, "node is recovering")) {
       return true;
    }
 
    /* Change Streams Spec resumable criteria: "any server error response from a
     * getMore command excluding those containing the following error codes" */
-   switch (code) {
+   switch (error.code) {
    case 11601: /* Interrupted */
    case 136:   /* CappedPositionLost */
    case 237:   /* CursorKilled */
diff --git a/src/libmongoc/src/mongoc/mongoc-cluster.c b/src/libmongoc/src/mongoc/mongoc-cluster.c
@@ -412,16 +412,29 @@ typedef enum {
 } reply_error_type_t;
 
 
+/*---------------------------------------------------------------------------
+ *
+ * _check_not_master_or_recovering_error --
+ *
+ *       Checks @reply for a "not master" or "node is recovering" error and
+ *       sets @error.
+ *
+ * Return:
+ *       A reply_error_type_t indicating if @reply contained a "not master"
+ *       or "node is recovering" error.
+ *
+ *--------------------------------------------------------------------------
+ */
 static reply_error_type_t
-_check_not_master_or_recovering_error (const bson_t *reply)
+_check_not_master_or_recovering_error (const mongoc_client_t *client,
+                                       const bson_t *reply,
+                                       bson_error_t *error)
 {
-   const char *msg = "";
-   uint32_t code;
-   if (!_mongoc_parse_error_reply (reply, true /* check_wce */, &code, &msg)) {
+   if (_mongoc_cmd_check_ok_no_wce (reply, client->error_api_version, error)) {
       return MONGOC_REPLY_ERR_TYPE_NONE;
    }
 
-   switch (code) {
+   switch (error->code) {
    case 11600: /* InterruptedAtShutdown */
    case 11602: /* InterruptedDueToReplStateChange */
    case 13436: /* NotMasterOrSecondary */
@@ -432,9 +445,9 @@ _check_not_master_or_recovering_error (const bson_t *reply)
    case 13435: /* NotMasterNoSlaveOk */
       return MONGOC_REPLY_ERR_TYPE_NOT_MASTER;
    default:
-      if (strstr (msg, "not master")) {
+      if (strstr (error->message, "not master")) {
          return MONGOC_REPLY_ERR_TYPE_NOT_MASTER;
-      } else if (strstr (msg, "node is recovering")) {
+      } else if (strstr (error->message, "node is recovering")) {
          return MONGOC_REPLY_ERR_TYPE_NODE_IS_RECOVERING;
       }
       return MONGOC_REPLY_ERR_TYPE_NONE;
@@ -445,19 +458,19 @@ _check_not_master_or_recovering_error (const bson_t *reply)
 static void
 handle_not_master_error (mongoc_cluster_t *cluster,
                          uint32_t server_id,
-                         const bson_error_t *error,
                          const bson_t *reply)
 {
    mongoc_topology_t *topology = cluster->client->topology;
+   bson_error_t error;
    reply_error_type_t error_type =
-      _check_not_master_or_recovering_error (reply);
+      _check_not_master_or_recovering_error (cluster->client, reply, &error);
 
    if (error_type != MONGOC_REPLY_ERR_TYPE_NONE) {
       /* Server Discovery and Monitoring Spec: "When the client sees a 'not
        * master' or 'node is recovering' error it MUST replace the server's
        * description with a default ServerDescription of type Unknown."
        */
-      mongoc_topology_invalidate_server (topology, server_id, error);
+      mongoc_topology_invalidate_server (topology, server_id, &error);
       if (topology->single_threaded) {
          /* SDAM Spec: "For single-threaded clients, in the case of a 'not
           * master' error, the client MUST check the server immediately... For a
@@ -572,7 +585,7 @@ mongoc_cluster_run_command_monitored (mongoc_cluster_t *cluster,
       mongoc_apm_command_failed_cleanup (&failed_event);
    }
 
-   handle_not_master_error (cluster, server_id, error, reply);
+   handle_not_master_error (cluster, server_id, reply);
 
    if (reply == &reply_local) {
       bson_destroy (&reply_local);
@@ -627,7 +640,7 @@ mongoc_cluster_run_command_private (mongoc_cluster_t *cluster,
       retval = mongoc_cluster_run_command_opquery (
          cluster, cmd, cmd->server_stream->stream, -1, reply, error);
    }
-   handle_not_master_error (cluster, server_stream->sd->id, error, reply);
+   handle_not_master_error (cluster, server_stream->sd->id, reply);
    if (reply == &reply_local) {
       bson_destroy (&reply_local);
    }
diff --git a/src/libmongoc/src/mongoc/mongoc-rpc-private.h b/src/libmongoc/src/mongoc/mongoc-rpc-private.h
@@ -166,6 +166,11 @@ _mongoc_cmd_check_ok (const bson_t *doc,
                       int32_t error_api_version,
                       bson_error_t *error);
 
+bool
+_mongoc_cmd_check_ok_no_wce (const bson_t *doc,
+                             int32_t error_api_version,
+                             bson_error_t *error);
+
 bool
 _mongoc_rpc_decompress (mongoc_rpc_t *rpc_le, uint8_t *buf, size_t buflen);
 
@@ -175,12 +180,6 @@ _mongoc_rpc_compress (struct _mongoc_cluster_t *cluster,
                       mongoc_rpc_t *rpc_le,
                       bson_error_t *error);
 
-bool
-_mongoc_parse_error_reply (const bson_t *doc,
-                           bool check_wce,
-                           uint32_t *code,
-                           const char **msg);
-
 BSON_END_DECLS
 
 
diff --git a/src/libmongoc/src/mongoc/mongoc-rpc.c b/src/libmongoc/src/mongoc/mongoc-rpc.c
@@ -1065,6 +1065,67 @@ _mongoc_rpc_prep_command (mongoc_rpc_t *rpc,
 }
 
 
+/* returns true if an error was found. */
+static bool
+_parse_error_reply (const bson_t *doc,
+                    bool check_wce,
+                    uint32_t *code,
+                    const char **msg)
+{
+   bson_iter_t iter;
+   bool found_error = false;
+
+   ENTRY;
+
+   BSON_ASSERT (doc);
+   BSON_ASSERT (code);
+   *code = 0;
+
+   if (bson_iter_init_find (&iter, doc, "code") &&
+       BSON_ITER_HOLDS_INT32 (&iter)) {
+      *code = (uint32_t) bson_iter_int32 (&iter);
+      found_error = true;
+   }
+
+   if (bson_iter_init_find (&iter, doc, "errmsg") &&
+       BSON_ITER_HOLDS_UTF8 (&iter)) {
+      *msg = bson_iter_utf8 (&iter, NULL);
+      found_error = true;
+   } else if (bson_iter_init_find (&iter, doc, "$err") &&
+              BSON_ITER_HOLDS_UTF8 (&iter)) {
+      *msg = bson_iter_utf8 (&iter, NULL);
+      found_error = true;
+   }
+
+   if (found_error) {
+      /* there was a command error */
+      RETURN (true);
+   }
+
+   if (check_wce) {
+      /* check for a write concern error */
+      if (bson_iter_init_find (&iter, doc, "writeConcernError") &&
+          BSON_ITER_HOLDS_DOCUMENT (&iter)) {
+         bson_iter_t child;
+         BSON_ASSERT (bson_iter_recurse (&iter, &child));
+         if (bson_iter_find (&child, "code") &&
+             BSON_ITER_HOLDS_INT32 (&child)) {
+            *code = (uint32_t) bson_iter_int32 (&child);
+            found_error = true;
+         }
+         BSON_ASSERT (bson_iter_recurse (&iter, &child));
+         if (bson_iter_find (&child, "errmsg") &&
+             BSON_ITER_HOLDS_UTF8 (&child)) {
+            *msg = bson_iter_utf8 (&child, NULL);
+            found_error = true;
+         }
+      }
+   }
+
+   RETURN (found_error);
+}
+
+
 /*
  *--------------------------------------------------------------------------
  *
@@ -1104,7 +1165,7 @@ _mongoc_cmd_check_ok (const bson_t *doc,
       RETURN (true);
    }
 
-   if (!_mongoc_parse_error_reply (doc, false /* check_wce */, &code, &msg)) {
+   if (!_parse_error_reply (doc, false /* check_wce */, &code, &msg)) {
       RETURN (true);
    }
 
@@ -1120,66 +1181,57 @@ _mongoc_cmd_check_ok (const bson_t *doc,
    RETURN (false);
 }
 
-/* returns true if an error was found. */
+/*
+ *--------------------------------------------------------------------------
+ *
+ * _mongoc_cmd_check_ok_no_wce --
+ *
+ *       Check if a server reply document is an error message.
+ *       Optionally fill out a bson_error_t from the server error.
+ *       If the response contains a writeConcernError, this is considered
+ *       an error and returns false.
+ *
+ * Returns:
+ *       false if @doc is an error message, true otherwise.
+ *
+ * Side effects:
+ *       If @doc is an error reply and @error is not NULL, set its
+ *       domain, code, and message.
+ *
+ *--------------------------------------------------------------------------
+ */
 bool
-_mongoc_parse_error_reply (const bson_t *doc,
-                           bool check_wce,
-                           uint32_t *code,
-                           const char **msg)
+_mongoc_cmd_check_ok_no_wce (const bson_t *doc,
+                             int32_t error_api_version,
+                             bson_error_t *error)
 {
-   bson_iter_t iter;
-   bool found_error = false;
+   mongoc_error_domain_t domain =
+      error_api_version >= MONGOC_ERROR_API_VERSION_2 ? MONGOC_ERROR_SERVER
+                                                      : MONGOC_ERROR_QUERY;
+   uint32_t code;
+   const char *msg = "Unknown command error";
 
    ENTRY;
 
    BSON_ASSERT (doc);
-   BSON_ASSERT (code);
-   *code = 0;
-
-   if (bson_iter_init_find (&iter, doc, "code") &&
-       BSON_ITER_HOLDS_INT32 (&iter)) {
-      *code = (uint32_t) bson_iter_int32 (&iter);
-      found_error = true;
-   }
-
-   if (bson_iter_init_find (&iter, doc, "errmsg") &&
-       BSON_ITER_HOLDS_UTF8 (&iter)) {
-      *msg = bson_iter_utf8 (&iter, NULL);
-      found_error = true;
-   } else if (bson_iter_init_find (&iter, doc, "$err") &&
-              BSON_ITER_HOLDS_UTF8 (&iter)) {
-      *msg = bson_iter_utf8 (&iter, NULL);
-      found_error = true;
-   }
 
-   if (found_error) {
-      /* there was a command error */
+   if (!_parse_error_reply (doc, true /* check_wce */, &code, &msg)) {
       RETURN (true);
    }
 
-   if (check_wce) {
-      /* check for a write concern error */
-      if (bson_iter_init_find (&iter, doc, "writeConcernError") &&
-          BSON_ITER_HOLDS_DOCUMENT (&iter)) {
-         bson_iter_t child;
-         BSON_ASSERT (bson_iter_recurse (&iter, &child));
-         if (bson_iter_find (&child, "code") &&
-             BSON_ITER_HOLDS_INT32 (&child)) {
-            *code = (uint32_t) bson_iter_int32 (&child);
-            found_error = true;
-         }
-         BSON_ASSERT (bson_iter_recurse (&iter, &child));
-         if (bson_iter_find (&child, "errmsg") &&
-             BSON_ITER_HOLDS_UTF8 (&child)) {
-            *msg = bson_iter_utf8 (&child, NULL);
-            found_error = true;
-         }
-      }
+   if (code == MONGOC_ERROR_PROTOCOL_ERROR || code == 13390) {
+      code = MONGOC_ERROR_QUERY_COMMAND_NOT_FOUND;
+   } else if (code == 0) {
+      code = MONGOC_ERROR_QUERY_FAILURE;
    }
 
-   RETURN (found_error);
+   bson_set_error (error, domain, code, "%s", msg);
+
+   /* there was a command error */
+   RETURN (false);
 }
 
+
 /* helper function to parse error reply document to an OP_QUERY */
 static void
 _mongoc_populate_query_error (const bson_t *doc,
diff --git a/src/libmongoc/src/mongoc/mongoc-server-description.c b/src/libmongoc/src/mongoc/mongoc-server-description.c
@@ -530,12 +530,10 @@ mongoc_server_description_handle_ismaster (mongoc_server_description_t *sd,
       num_keys++;
       if (strcmp ("ok", bson_iter_key (&iter)) == 0) {
          if (!bson_iter_as_bool (&iter)) {
-            uint32_t unused;
-            const char *msg = NULL;
-            _mongoc_parse_error_reply (ismaster_response, false, &unused, &msg);
-            if (msg) {
-               bson_strncpy (sd->error.message, msg, sizeof (error->message));
-            }
+            /* it doesn't really matter what error API we use. the code and
+             * domain will be overwritten. */
+            (void) _mongoc_cmd_check_ok (
+               ismaster_response, MONGOC_ERROR_API_VERSION_2, &sd->error);
             /* ismaster response returned ok: 0. According to auth spec: "If the
              * isMaster of the MongoDB Handshake fails with an error, drivers
              * MUST treat this an an authentication error." */
diff --git a/src/libmongoc/src/mongoc/mongoc-write-command.c b/src/libmongoc/src/mongoc/mongoc-write-command.c
@@ -1442,13 +1442,13 @@ _mongoc_write_result_complete (
 bool
 _mongoc_write_is_retryable_error (const bson_t *reply)
 {
-   const char *msg = "";
-   uint32_t code;
-   if (!_mongoc_parse_error_reply (reply, true /* check_wce */, &code, &msg)) {
+   bson_error_t error;
+   if (_mongoc_cmd_check_ok_no_wce (
+          reply, MONGOC_ERROR_API_VERSION_2, &error)) {
       return false;
    }
 
-   switch (code) {
+   switch (error.code) {
    case 11600: /* InterruptedAtShutdown */
    case 11602: /* InterruptedDueToReplStateChange */
    case 10107: /* NotMaster */
@@ -1463,7 +1463,8 @@ _mongoc_write_is_retryable_error (const bson_t *reply)
    case 9001:  /* SocketException */
       return true;
    default:
-      if (strstr (msg, "not master") || strstr (msg, "node is recovering")) {
+      if (strstr (error.message, "not master") ||
+          strstr (error.message, "node is recovering")) {
          return true;
       }
       return false;
diff --git a/src/libmongoc/tests/test-mongoc-topology.c b/src/libmongoc/tests/test-mongoc-topology.c
