Merge pull request #2765 from WebFreak001/fix-crud-erroring

fix MongoDB CRUD not throwing on error

mongodb/vibe/db/mongo/collection.d

@@ -139,13 +139,13 @@ struct MongoCollection {
 		enforceWireVersionConstraints(options, conn.description.maxWireVersion);
 		foreach (string k, v; serializeToBson(options).byKeyValue)
 			cmd[k] = v;
-		
-		database.runCommandChecked(cmd);
+
+		database.runCommandChecked(cmd).handleWriteResult(res);
 		return res;
 	}
 
 	/// ditto
-	InsertOneResult insertMany(T)(T[] documents, InsertManyOptions options = InsertManyOptions.init)
+	InsertManyResult insertMany(T)(T[] documents, InsertManyOptions options = InsertManyOptions.init)
 	{
 		assert(m_client !is null, "Querying uninitialized MongoCollection.");
 
@@ -168,9 +168,10 @@ struct MongoCollection {
 		enforceWireVersionConstraints(options, conn.description.maxWireVersion);
 		foreach (string k, v; serializeToBson(options).byKeyValue)
 			cmd[k] = v;
-
-		database.runCommandChecked(cmd);
-		return InsertManyResult(insertedIds);
+
+		auto res = InsertManyResult(insertedIds);
+		database.runCommandChecked(cmd).handleWriteResult!"insertedCount"(res);
+		return res;
 	}
 
 	/**
@@ -249,8 +250,9 @@ struct MongoCollection {
 		}
 		cmd["deletes"] = Bson(deletesBson);
 
-		auto n = database.runCommandChecked(cmd)["n"].to!long;
-		return DeleteResult(n);
+		DeleteResult res;
+		database.runCommandChecked(cmd).handleWriteResult!"deletedCount"(res);
+		return res;
 	}
 
 	/**
@@ -394,7 +396,7 @@ struct MongoCollection {
 						if (k.startsWith("$"))
 							anyDollar = true;
 						debug {} // server checks that the rest is consistent (only $ or only non-$ allowed)
-                        else break; // however in debug mode we check the full document, as we can give better error messages to the dev
+						else break; // however in debug mode we check the full document, as we can give better error messages to the dev
 						// also nice side effect: if this is an empty document, this also matches the assert(false) branch.
 					}
 
@@ -419,14 +421,15 @@ struct MongoCollection {
 			res["n"].to!long,
 			res["nModified"].to!long,
 		);
+		res.handleWriteResult(ret);
 		auto upserted = res["upserted"].opt!(Bson[]);
 		if (upserted.length)
 		{
 			ret.upsertedIds.length = upserted.length;
 			foreach (i, upsert; upserted)
-            {
+			{
 				ret.upsertedIds[i] = upsert["_id"].get!BsonObjectID;
-            }
+			}
 		}
 		return ret;
 	}
@@ -747,7 +750,7 @@ struct MongoCollection {
 	/**
 		Returns the count of documents that match the query for a collection or
 		view.
-		
+
 		The method wraps the `$group` aggregation stage with a `$sum` expression
 		to perform the count.
 
@@ -1188,9 +1191,9 @@ struct MongoCollection {
 	}
 
 	/**
-		Returns an array that holds a list of documents that identify and describe the existing indexes on the collection. 
+		Returns an array that holds a list of documents that identify and describe the existing indexes on the collection.
 	*/
-	MongoCursor!R listIndexes(R = Bson)() 
+	MongoCursor!R listIndexes(R = Bson)()
 	@safe {
 		MongoConnection conn = m_client.lockConnection();
 		if (conn.description.satisfiesVersion(WireVersion.v30)) {

mongodb/vibe/db/mongo/connection.d

@@ -85,6 +85,7 @@ class MongoDriverException : MongoException
  * It does not indicate problem with vibe.d driver itself. Most frequently this
  * one is thrown when MongoConnection is in checked mode and getLastError() has something interesting.
  */
+deprecated("Check for MongoException instead - the modern write commands now throw MongoBulkWriteException on error")
 class MongoDBException : MongoException
 {
 @safe:
@@ -958,7 +959,7 @@ final class MongoConnection {
 
 	private int nextMessageId() { return m_msgid++; }
 
-	private void checkForError(string collection_name)
+	deprecated private void checkForError(string collection_name)
 	{
 		auto coll = collection_name.split(".")[0];
 		auto err = getLastError(coll);
@@ -992,7 +993,7 @@ final class MongoConnection {
 	private void authenticate()
 	{
 		scope (failure) disconnect();
-	
+
 		string cn = m_settings.getAuthDatabase;
 
 		auto cmd = Bson(["getnonce": Bson(1)]);

mongodb/vibe/db/mongo/impl/crud.d

@@ -9,6 +9,7 @@ module vibe.db.mongo.impl.crud;
 
 import core.time;
 
+import vibe.db.mongo.connection : MongoException;
 import vibe.db.mongo.collection;
 import vibe.data.bson;
 
@@ -151,7 +152,7 @@ struct FindOptions
 		satisfy a tailable cursor query. This only applies to a TAILABLE_AWAIT
 		cursor. When the cursor is not a TAILABLE_AWAIT cursor, this option is
 		ignored.
-		
+
 		Note: This option is specified as "maxTimeMS" in the getMore command and
 		not provided as part of the initial find command.
 	*/
@@ -475,7 +476,7 @@ struct AggregateOptions
 		satisfy a tailable cursor query. This only applies to a TAILABLE_AWAIT
 		cursor. When the cursor is not a TAILABLE_AWAIT cursor, this option is
 		ignored.
-		
+
 		Note: This option is specified as "maxTimeMS" in the getMore command and
 		not provided as part of the initial find command.
 	*/
@@ -818,43 +819,6 @@ struct DeleteOptions {
 	Nullable!Bson let;
 }
 
-struct BulkWriteResult {
-	/**
-		Number of documents inserted.
-	*/
-	long insertedCount;
-
-	/**
-		The identifiers that were automatically generated, if not set.
-	*/
-	BsonObjectID[size_t] insertedIds;
-
-	/**
-		Number of documents matched for update.
-	*/
-	long matchedCount;
-
-	/**
-		Number of documents modified.
-	*/
-	long modifiedCount;
-
-	/**
-		Number of documents deleted.
-	*/
-	long deletedCount;
-
-	/**
-		Number of documents upserted.
-	*/
-	long upsertedCount;
-
-	/**
-		Map of the index of the operation to the id of the upserted document.
-	*/
-	BsonObjectID[size_t] upsertedIds;
-}
-
 struct InsertOneResult {
 	/**
 		The identifier that was automatically generated, if not set.
@@ -867,6 +831,10 @@ struct InsertManyResult {
 		The identifiers that were automatically generated, if not set.
 	*/
 	BsonObjectID[size_t] insertedIds;
+	/**
+		The number of documents that were inserted.
+	*/
+	long insertedCount;
 }
 
 struct DeleteResult {
@@ -894,3 +862,80 @@ struct UpdateResult {
 	*/
 	BsonObjectID[] upsertedIds;
 }
+
+/**
+	Describes a failed write command result for a single document.
+
+	See_Also: $(LINK https://www.mongodb.com/docs/manual/reference/method/WriteResult/)
+
+	Standards: $(LINK https://github.com/mongodb/specifications/blob/0b6b96b758259edc91c2fc475bcf08eea54e08e2/source/crud/crud.rst#L1745)
+*/
+struct BulkWriteError
+{
+	/**
+	 * An integer value identifying the write error.
+	 */
+	int code;
+	/**
+	 * A document providing more information about the write error (e.g. details
+	 * pertaining to document validation).
+	 */
+	@optional @name("errInfo") Bson details;
+	/**
+	 * A description of the error.
+	 */
+	@optional @name("errmsg") string message;
+	/**
+	 * The index of the request that errored.
+	 */
+	@optional int index;
+}
+
+/**
+ * Exception for partially or fully failed writes from `insert`, `update` or
+ * `delete`.
+ */
+class MongoBulkWriteException : MongoException
+{
+@safe:
+	BulkWriteError[] errors;
+
+	this(BulkWriteError[] errors, string file = __FILE__,
+			size_t line = __LINE__, Throwable next = null)
+	{
+		import std.algorithm : map;
+		import std.string : join;
+
+		super("Failed to write all documents:\n"
+			~ errors.map!(e => "- " ~ e.message).join("\n"), file, line, next);
+
+		this.errors = errors;
+	}
+}
+
+/**
+ * Handles the raw DB response from `insert`, `update` or `delete` operations.
+ *
+ * If the `countField` template argument is set, that field is set on `result`
+ * to `dbResult["n"]`, if it exists.
+ */
+package(vibe.db.mongo) void handleWriteResult(string countField = null, T)(
+	Bson dbResult, ref T result, string file = __FILE__, size_t line = __LINE__)
+{
+	auto dbObject = dbResult.get!(Bson[string]);
+	static if (countField != null)
+	{
+		if (auto n = "n" in dbObject)
+			__traits(getMember, result, countField) = n.to!long;
+	}
+
+	if (auto writeErrors = "writeErrors" in dbObject)
+	{
+		if (writeErrors.type == Bson.Type.array)
+		{
+			auto errors = deserializeBson!(BulkWriteError[])(*writeErrors);
+			if (errors.length > 0)
+				throw new MongoBulkWriteException(errors, file, line);
+		}
+	}
+}

tests/mongodb/insert-remove-aggregate/source/app.d

@@ -11,6 +11,7 @@ import vibe.db.mongo.mongo;
 import std.algorithm : all, canFind, equal, map, sort;
 import std.conv : to;
 import std.encoding : sanitize;
+import std.exception : assertThrown;
 
 struct DBTestEntry
 {
@@ -101,6 +102,48 @@ void runTest(ushort port)
 	auto d = coll.distinct!string("b", ["a": "first"]).array;
 	d.sort!"a<b";
 	assert(d == ["bar", "foo"]);
+
+	testIndexInsert(client);
+}
+
+void testIndexInsert(MongoClient client)
+{
+	MongoCollection coll = client.getCollection("test.indexedinsert");
+	coll.deleteAll();
+	auto n = coll.insertMany([
+		Bson([
+			"_id": Bson(1),
+			"username": Bson("Bob")
+		]),
+		Bson([
+			"_id": Bson(2),
+			"username": Bson("Alice")
+		]),
+	]);
+	assert(n.insertedCount == 2);
+
+	assertThrown(coll.insertOne(Bson([
+		"_id": Bson(2), // duplicate _id
+		"username": Bson("Tom")
+	])));
+
+	IndexOptions indexOptions;
+	indexOptions.unique = true;
+	coll.createIndex(IndexModel.init
+		.withOptions(indexOptions)
+		.add("username", 1));
+
+	coll.insertOne(Bson([
+		"_id": Bson(3),
+		"username": Bson("Charlie")
+	]));
+
+	assertThrown(coll.insertOne(Bson([
+		"_id": Bson(4),
+		"username": Bson("Bob") // duplicate username
+	])));
+
+	assert(coll.estimatedDocumentCount == 3);
 }
 
 int main(string[] args)