fix(bulk): bulk operations should not throw an error on empty batch

Presently the ordered and unordered bulk operation classes will
throw an error when an empty set of operations is provided. This is
invalid behavior beause the execute method takes a callback, or
returns a promise. The error should be properly pushed back up the
chain to the user.

Author: mbroadst

lib/bulk/ordered.js

@@ -488,30 +488,37 @@ var executeCommands = function(self, callback) {
  */
 OrderedBulkOperation.prototype.execute = function(_writeConcern, callback) {
   var self = this;
-  if(this.s.executed) throw new toError("batch cannot be re-executed");
-  if(typeof _writeConcern == 'function') {
+  if (this.s.executed) {
+    var executedError = toError('batch cannot be re-executed');
+    return (typeof callback === 'function') ?
+      callback(executedError, null) : this.s.promiseLibrary.reject(executedError);
+  }
+
+  if (typeof _writeConcern === 'function') {
     callback = _writeConcern;
-	} else if(_writeConcern && typeof _writeConcern == 'object') {
+  } else if (_writeConcern && typeof _writeConcern === 'object') {
     this.s.writeConcern = _writeConcern;
   }
 
   // If we have current batch
-  if(this.s.currentBatch) this.s.batches.push(this.s.currentBatch)
+  if (this.s.currentBatch) this.s.batches.push(this.s.currentBatch)
 
   // If we have no operations in the bulk raise an error
-  if(this.s.batches.length == 0) {
-    throw toError("Invalid Operation, No operations in bulk");
+  if (this.s.batches.length == 0) {
+    var emptyBatchError = toError('Invalid Operation, no operations specified');
+    return (typeof callback === 'function') ?
+      callback(emptyBatchError, null) : this.s.promiseLibrary.reject(emptyBatchError);
   }
 
   // Execute using callback
-  if(typeof callback == 'function') {
-		return executeCommands(this, callback);
-	}
+  if (typeof callback === 'function') {
+    return executeCommands(this, callback);
+  }
 
   // Return a Promise
   return new this.s.promiseLibrary(function(resolve, reject) {
     executeCommands(self, function(err, r) {
-      if(err) return reject(err);
+      if (err) return reject(err);
       resolve(r);
     });
   });

lib/bulk/unordered.js

@@ -490,30 +490,37 @@ var executeBatches = function(self, callback) {
  */
 UnorderedBulkOperation.prototype.execute = function(_writeConcern, callback) {
   var self = this;
-  if(this.s.executed) throw toError("batch cannot be re-executed");
-  if(typeof _writeConcern == 'function') {
+  if (this.s.executed) {
+    var executedError = toError('batch cannot be re-executed');
+    return (typeof callback === 'function') ?
+      callback(executedError, null) : this.s.promiseLibrary.reject(executedError);
+  }
+
+  if (typeof _writeConcern === 'function') {
     callback = _writeConcern;
-  } else if(_writeConcern && typeof _writeConcern == 'object') {
+  } else if (_writeConcern && typeof _writeConcern === 'object') {
     this.s.writeConcern = _writeConcern;
   }
 
   // If we have current batch
-  if(this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
-  if(this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
-  if(this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
+  if (this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
+  if (this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
+  if (this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
 
   // If we have no operations in the bulk raise an error
-  if(this.s.batches.length == 0) {
-    throw toError("Invalid Operation, No operations in bulk");
+  if (this.s.batches.length == 0) {
+    var emptyBatchError = toError('Invalid Operation, no operations specified');
+    return (typeof callback === 'function') ?
+      callback(emptyBatchError, null) : this.s.promiseLibrary.reject(emptyBatchError);
   }
 
   // Execute using callback
-  if(typeof callback == 'function') return executeBatches(this, callback);
+  if (typeof callback === 'function') return executeBatches(this, callback);
 
   // Return a Promise
   return new this.s.promiseLibrary(function(resolve, reject) {
     executeBatches(self, function(err, r) {
-      if(err) return reject(err);
+      if (err) return reject(err);
       resolve(r);
     });
   });

test/functional/bulk_tests.js

@@ -369,7 +369,7 @@ exports['Should correctly perform ordered upsert with custom _id'] = {
   }
 }
 
-exports['Should throw an error when no operations in ordered batch'] = {
+exports['Should return an error when no operations in ordered batch'] = {
   metadata: { requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } },
 
   // The actual test we wish to run
@@ -378,18 +378,15 @@ exports['Should throw an error when no operations in ordered batch'] = {
     db.open(function(err, db) {
       // Get the collection
       var col = db.collection('batch_write_ordered_ops_8');
-      var threw = false;
 
-      try {
-        // Initialize the Ordered Batch
-        col.initializeOrderedBulkOp().execute(function(err, result) {});
-      } catch(err) {
-        threw = true;
-      }
+      // Initialize the Ordered Batch
+      col.initializeOrderedBulkOp().execute(function(err, result) {
+        test.equal(err instanceof Error, true);
+        test.equal(err.message, 'Invalid Operation, no operations specified');
 
-      test.equal(true, threw);
-      db.close();
-      test.done();
+        db.close();
+        test.done();
+      });
     });
   }
 }
@@ -874,7 +871,7 @@ exports['Should prohibit batch finds with no selector'] = {
   }
 }
 
-exports['Should throw an error when no operations in unordered batch'] = {
+exports['Should return an error when no operations in unordered batch'] = {
   metadata: { requires: { topology: ['single', 'replicaset', 'ssl', 'heap', 'wiredtiger'] } },
 
   // The actual test we wish to run
@@ -883,18 +880,15 @@ exports['Should throw an error when no operations in unordered batch'] = {
     db.open(function(err, db) {
       // Get the collection
       var col = db.collection('batch_write_ordered_ops_8');
-      var threw = false;
 
-      try {
-        // Initialize the Ordered Batch
-        col.initializeUnorderedBulkOp().execute(configuration.writeConcernMax(),function(err, result) {});
-      } catch(err) {
-        threw = true;
-      }
+      // Initialize the Ordered Batch
+      col.initializeUnorderedBulkOp().execute(configuration.writeConcernMax(),function(err, result) {
+        test.equal(err instanceof Error, true);
+        test.equal(err.message, 'Invalid Operation, no operations specified');
 
-      test.equal(true, threw);
-      db.close();
-      test.done();
+        db.close();
+        test.done();
+      });
     });
   }
 }
@@ -1228,3 +1222,56 @@ exports['Should correctly handle bulk operation split for unordered bulk operati
     });
   }
 }
+
+exports['Should return an error instead of throwing when no operations are provided for ordered bulk operation execute'] = {
+  metadata: { requires: { mongodb: ">=2.6.0" , topology: 'single', node: ">0.10.0" } },
+  test: function(configure, test) {
+    var db = configure.newDbInstance({ w: 1 }, { poolSize: 1 });
+
+    db.open(function(err, db) {
+      db.collection('doesnt_matter').insertMany([], function(err, r) {
+        test.equal(err instanceof Error, true);
+        test.equal(err.message, 'Invalid Operation, no operations specified');
+        db.close();
+        test.done();
+      });
+    });
+  }
+}
+
+exports['Should return an error instead of throwing when no operations are provided for unordered bulk operation execute'] = {
+  metadata: { requires: { mongodb: ">=2.6.0" , topology: 'single', node: ">0.10.0" } },
+  test: function(configure, test) {
+    var db = configure.newDbInstance({ w: 1 }, { poolSize: 1 });
+
+    db.open(function(err, db) {
+      db.collection('doesnt_matter').insertMany([], { ordered: false }, function(err, r) {
+        test.equal(err instanceof Error, true);
+        test.equal(err.message, 'Invalid Operation, no operations specified');
+        db.close();
+        test.done();
+      });
+    });
+  }
+}
+
+exports['Should return an error instead of throwing when an empty bulk operation is submitted (with promise)'] = {
+  metadata: { requires: { promises: true, node: ">0.12.0" } },
+  test: function(configure, test) {
+    var db = configure.newDbInstance({ w: 1 }, { poolSize: 1 });
+
+    return db.open()
+      .then(function() { return db.collection('doesnt_matter').insertMany([]); })
+      .then(function() {
+        test.equal(false, true); // this should not happen!
+      })
+      .catch(function(err) {
+        test.equal(err instanceof Error, true);
+        test.equal(err.message, 'Invalid Operation, no operations specified');
+      })
+      .then(function() {
+        db.close();
+        test.done();
+      });
+  }
+}