fix(db): only callback with MongoError NODE-1293 (#1652)

mongodb · Jan 29, 2018 · 45bc722 · 45bc722
1 parent a45eb0b
commit 45bc722
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 3 deletions.
diff --git a/lib/db.js b/lib/db.js
@@ -445,8 +445,7 @@ Db.prototype.collection = function(name, options, callback) {
       if (callback) callback(null, collection);
       return collection;
     } catch (err) {
-      // if(err instanceof MongoError && callback) return callback(err);
-      if (callback) return callback(err);
+      if (err instanceof MongoError && callback) return callback(err);
       throw err;
     }
   }

diff --git a/test/functional/db_tests.js b/test/functional/db_tests.js
@@ -110,6 +110,40 @@ describe('Db', function() {
     }
   });
 
+  /**
+   * @ignore
+   */
+  it('should callback with an error only when a MongoError', {
+    metadata: {
+      requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] }
+    },
+
+    test: function(done) {
+      let configuration = this.configuration;
+      let client = configuration.newClient(configuration.writeConcernMax(), {
+        poolSize: 1,
+        auto_reconnect: true
+      });
+
+      client.connect(function(err, client) {
+        let callbackCalled = 0;
+        test.equal(null, err);
+        let db = client.db(configuration.db);
+
+        try {
+          db.collection('collectionCallbackTest', function(e) {
+            callbackCalled++;
+            test.equal(null, e);
+            throw new Error('Erroring on purpose with a non MongoError');
+          });
+        } catch (e) {
+          test.equal(callbackCalled, 1);
+          done();
+        }
+      });
+    }
+  });
+
   /**
    * @ignore
    */

diff --git a/test/functional/index_tests.js b/test/functional/index_tests.js
@@ -1123,7 +1123,8 @@ describe('Indexes', function() {
             db.collection('nonexisting', { strict: true }, function(err) {
               test.ok(err != null);
               db.collection('nonexisting', { strict: false }, function(err) {
-                test.ok(err != null);
+                // When set to false (default) it should not create an error
+                test.ok(err === null);
                 done();
               });
             });