The errors returned by the server must always contain the $err key (#169)

* The errors returned by the server must always contain the $err key

* Audit all places the server sends back 'errmsg' and added '$err' along with it

Author: dongxinEric

src/ExtCmd.actor.cpp

@@ -230,7 +230,8 @@ struct GetLogCmd {
 		if (query->ns.first != "admin") {
 			// clang-format off
 			reply->addDocument((bob << "ok" << 0.0 <<
-			                           "errmsg" << "access denied; use admin db").obj());
+			                           "errmsg" << "access denied; use admin db" <<
+			                           "$err" << "access denied; use admin db").obj());
 			// clang-format on
 			return reply;
 		}
@@ -286,6 +287,7 @@ struct ReplSetGetStatusCmd {
 		// FIXME: what do we really want to report here?
 		bob.append("ok", 0.0);
 		bob.append("errmsg", "not really talking to mongodb");
+		bob.append("$err", "not really talking to mongodb");
 
 		reply->addDocument(bob.obj());
 
@@ -536,7 +538,12 @@ ACTOR static Future<Reference<ExtMsgReply>> doFindAndModify(Reference<ExtConnect
 		bson::BSONObj replyObj = bob.obj().getOwned();
 		reply->addDocument(replyObj);
 	} catch (Error& e) {
-		reply->addDocument(BSON("errmsg" << e.what() << "code" << e.code() << "ok" << 1.0));
+		// clang-format off
+		reply->addDocument(BSON("errmsg" << e.what() <<
+		                        "$err" << e.what() <<
+		                        "code" << e.code() <<
+		                        "ok" << 1.0));
+		// clang-format on
 	}
 
 	return reply;
@@ -662,7 +669,12 @@ ACTOR static Future<Reference<ExtMsgReply>> doCreateIndexes(Reference<ExtConnect
 		Void _ = wait(waitForAll(f));
 		reply->addDocument(BSON("ok" << 1.0));
 	} catch (Error& e) {
-		reply->addDocument(BSON("ok" << 0.0 << "errmsg" << e.what() << "code" << e.code()));
+		// clang-format off
+		reply->addDocument(BSON("ok" << 0.0 <<
+		                        "$err" << e.what() <<
+		                        "errmsg" << e.what() <<
+		                        "code" << e.code()));
+		// clang-format on
 	}
 	return reply;
 }
@@ -954,7 +966,12 @@ ACTOR static Future<Reference<ExtMsgReply>> insertAndReply(Reference<ExtConnecti
 		// one FDB transaction.
 		bson::BSONArrayBuilder arrayBuilder;
 		for (int i = 0; i < docs.size(); i++) {
-			arrayBuilder << BSON("index" << i << "code" << e.code() << "errmsg" << e.what());
+			// clang-format off
+			arrayBuilder << BSON("index" << i <<
+			                     "code" << e.code() <<
+			                     "$err" << e.what() <<
+			                     "errmsg" << e.what());
+			// clang-format on
 		}
 		reply->addDocument(BSON("ok" << 1 << "n" << 0 << "writeErrors" << arrayBuilder.arr()));
 	}

src/ExtMsg.actor.cpp

@@ -249,8 +249,10 @@ ACTOR Future<Void> runCommand(Reference<ExtConnection> nmc,
 
 		if (e.code() == error_code_bad_dispatch) {
 			bob.append("errmsg", format("no such cmd: %s", cmd.c_str()));
+			bob.append("$err", format("no such cmd: %s", cmd.c_str()));
 		} else {
 			bob.append("errmsg", format("command [%s] failed with err: %s", cmd.c_str(), e.what()));
+			bob.append("$err", format("command [%s] failed with err: %s", cmd.c_str(), e.what()));
 		}
 		bob.append("bad cmd", query->query.toString());
 		bob.append("ok", 0);
@@ -1094,7 +1096,12 @@ ACTOR Future<WriteCmdResult> doUpdateCmd(Namespace ns,
 			}
 		} catch (Error& e) {
 			TraceEvent(SevError, "ExtMsgUpdateFailure").error(e);
-			cmdResult.writeErrors.push_back(BSON("index" << idx << "code" << e.code() << "errmsg" << e.what()));
+			// clang-format off
+			cmdResult.writeErrors.push_back(BSON("index" << idx <<
+			                                     "code" << e.code() <<
+			                                     "$err" << e.what() <<
+			                                     "errmsg" << e.what()));
+			// clang-format on
 			if (ordered)
 				break;
 		}
@@ -1238,7 +1245,12 @@ ACTOR Future<WriteCmdResult> doDeleteCmd(Namespace ns,
 				nrDeletedRecords += deletedRecords;
 			} catch (Error& e) {
 				TraceEvent(SevError, "ExtMsgDeleteFailure").error(e);
-				writeErrors.push_back(BSON("index" << idx << "code" << e.code() << "errmsg" << e.what()));
+				// clang-format off
+				writeErrors.push_back(BSON("index" << idx <<
+				                           "code" << e.code() <<
+				                           "$err" << e.what() <<
+				                           "errmsg" << e.what()));
+				// clang-format on
 				if (ordered)
 					break;
 			}

src/ExtStructs.h

@@ -61,7 +61,7 @@ struct WriteCmdResult {
 	std::vector<Standalone<StringRef>> upsertedOIDList;
 
 	/**
-	 * Array of write errors. Each entry has fields - 'index', 'code' and 'errmsg'. 'index'
+	 * Array of write errors. Each entry has fields - 'index', 'code' '$err' and 'errmsg'. 'index'
 	 * points to write command in the request. So, separate write error for each write failed.
 	 */
 	std::vector<bson::BSONObj> writeErrors;

test/correctness/smoke/test_errors.py

@@ -0,0 +1,32 @@
+#!/usr/bin/python
+#
+# test_errors.py
+#
+# This source file is part of the FoundationDB open source project
+#
+# Copyright 2013-2019 Apple Inc. and the FoundationDB project authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# MongoDB is a registered trademark of MongoDB, Inc.
+#
+
+from pymongo.errors import OperationFailure
+
+def test_error_reply(fixture_db):
+    try:
+        fixture_db.command("someTotallyRandomCmd")
+    except OperationFailure as e:
+        serverErrObj = e.details
+        assert serverErrObj['$err'] != None
+        assert "no such cmd: sometotallyrandomcmd" in serverErrObj['$err']