fix(topology): don't early abort server selection on network errors

Network errors encountered during server selection are meant to be
stored on the topology description, in order to eventually convey
the reason for failed selection to the user. We were reporting this
error as soon as it was encountered erroneously. Now, we wait until
selection has failed to report the newly added `reason` property of
the `MongoTimeoutError` returned.

Author: mbroadst

lib/core/error.js

@@ -83,12 +83,17 @@ class MongoParseError extends MongoError {
  * An error signifying a timeout event
  *
  * @param {Error|string|object} message The error message
+ * @param {string|object} [reason] The reason the timeout occured
  * @property {string} message The error message
+ * @property {string} [reason] An optional reason context for the timeout, generally an error saved during flow of monitoring and selecting servers
  */
 class MongoTimeoutError extends MongoError {
-  constructor(message) {
+  constructor(message, reason) {
     super(message);
     this.name = 'MongoTimeoutError';
+    if (reason != null) {
+      this.reason = reason;
+    }
   }
 }
 

lib/core/sdam/topology.js

@@ -836,18 +836,18 @@ function selectServers(topology, selector, timeout, start, callback) {
       clearTimeout(iterationTimer);
       topology.s.iterationTimers.splice(timerIndex, 1);
 
-      if (topology.description.error) {
-        callback(topology.description.error, null);
-        return;
-      }
-
       // topology description has changed due to monitoring, reattempt server selection
       selectServers(topology, selector, timeout, start, callback);
     };
 
     const iterationTimer = setTimeout(() => {
       topology.removeListener('topologyDescriptionChanged', descriptionChangedHandler);
-      callback(new MongoTimeoutError(`Server selection timed out after ${timeout} ms`));
+      callback(
+        new MongoTimeoutError(
+          `Server selection timed out after ${timeout} ms`,
+          topology.description.error
+        )
+      );
     }, timeout - duration);
 
     // track this timer in case we need to clean it up outside this loop

test/core/functional/server_tests.js

@@ -993,12 +993,17 @@ describe('Server tests', function() {
         });
 
         const config = this.configuration;
-        var client = config.newTopology(server.address().host, server.address().port);
+        var client = config.newTopology(server.address().host, server.address().port, {
+          serverSelectionTimeoutMS: 10
+        });
+
         client.on('error', error => {
           let err;
           try {
             expect(error).to.be.an.instanceOf(Error);
-            expect(error.message).to.match(/but this version of the Node.js Driver requires/);
+
+            const errorMessage = error.reason ? error.reason.message : error.message;
+            expect(errorMessage).to.match(/but this version of the Node.js Driver requires/);
           } catch (e) {
             err = e;
           }

test/functional/connection_tests.js

@@ -423,7 +423,9 @@ describe('Connection', function() {
     // The actual test we wish to run
     test: function(done) {
       var configuration = this.configuration;
-      const client = configuration.newClient(configuration.url('slithy', 'toves'));
+      const client = configuration.newClient(configuration.url('slithy', 'toves'), {
+        serverSelectionTimeoutMS: 10
+      });
 
       client.connect(function(err, client) {
         expect(err).to.exist;

test/functional/db_tests.js

@@ -243,7 +243,8 @@ describe('Db', function() {
     test: function(done) {
       var configuration = this.configuration;
       var fs_client = configuration.newClient('mongodb://127.0.0.1:25117/test', {
-        auto_reconnect: false
+        auto_reconnect: false,
+        serverSelectionTimeoutMS: 10
       });
 
       fs_client.connect(function(err) {
@@ -435,7 +436,8 @@ describe('Db', function() {
       var configuration = this.configuration;
       var client = configuration.newClient(`mongodb://127.0.0.1:27088/test`, {
         auto_reconnect: false,
-        poolSize: 4
+        poolSize: 4,
+        serverSelectionTimeoutMS: 10
       });
 
       // Establish connection to db

test/functional/mongo_client_tests.js

@@ -474,7 +474,10 @@ describe('MongoClient', function() {
     // The actual test we wish to run
     test: function(done) {
       var configuration = this.configuration;
-      const client = configuration.newClient('mongodb://localhost:27088/test');
+      const client = configuration.newClient('mongodb://localhost:27088/test', {
+        serverSelectionTimeoutMS: 10
+      });
+
       client.connect(function(err) {
         test.ok(err != null);
 
@@ -508,7 +511,10 @@ describe('MongoClient', function() {
     // The actual test we wish to run
     test: function(done) {
       var configuration = this.configuration;
-      const client = configuration.newClient('mongodb://test.does.not.exist.com:80/test');
+      const client = configuration.newClient('mongodb://test.does.not.exist.com:80/test', {
+        serverSelectionTimeoutMS: 10
+      });
+
       client.connect(function(err) {
         test.ok(err != null);
 
@@ -593,7 +599,10 @@ describe('MongoClient', function() {
     // The actual test we wish to run
     test: function(done) {
       var configuration = this.configuration;
-      const client = configuration.newClient('mongodb://unknownhost:36363/ddddd');
+      const client = configuration.newClient('mongodb://unknownhost:36363/ddddd', {
+        serverSelectionTimeoutMS: 10
+      });
+
       client.connect(function(err) {
         test.ok(err != null);
         done();

test/functional/operation_example_tests.js

@@ -4027,7 +4027,8 @@ describe('Operation Examples', function() {
 
                 const oldClient = secondClient;
                 const thirdClient = configuration.newClient(
-                  'mongodb://user:name@localhost:27017/integration_tests'
+                  'mongodb://user:name@localhost:27017/integration_tests',
+                  { serverSelectionTimeoutMS: 10 }
                 );
 
                 // Authenticate

test/functional/operation_generators_example_tests.js

@@ -2966,7 +2966,10 @@ describe('Operation (Generators)', function() {
 
         try {
           // Authenticate
-          const client = configuration.newClient('mongodb://user:name@localhost:27017/admin');
+          const client = configuration.newClient('mongodb://user:name@localhost:27017/admin', {
+            serverSelectionTimeoutMS: 10
+          });
+
           yield client.connect();
           test.ok(false);
         } catch (err) {} // eslint-disable-line

test/functional/operation_promises_example_tests.js

@@ -3174,7 +3174,8 @@ describe('Operation (Promises)', function() {
 
             // Should error out due to user no longer existing
             const thirdClient = configuration.newClient(
-              'mongodb://user3:name@localhost:27017/integration_tests'
+              'mongodb://user3:name@localhost:27017/integration_tests',
+              { serverSelectionTimeoutMS: 10 }
             );
 
             return thirdClient.connect();

test/functional/scram_sha_256_tests.js

@@ -179,13 +179,17 @@ describe('SCRAM-SHA-256 auth', function() {
           password: userMap.sha256.password
         },
         authSource: this.configuration.db,
-        authMechanism: 'SCRAM-SHA-1'
+        authMechanism: 'SCRAM-SHA-1',
+        serverSelectionTimeoutMS: 10
       };
 
       return withClient(
         this.configuration.newClient({}, options),
         () => Promise.reject(new Error('This request should have failed to authenticate')),
-        err => expect(err).to.match(/Authentication failed/)
+        err => {
+          const errMessage = err.reason ? err.reason.message : err;
+          expect(errMessage).to.match(/Authentication failed/);
+        }
       );
     }
   });
@@ -202,22 +206,27 @@ describe('SCRAM-SHA-256 auth', function() {
           user: 'roth',
           password: 'pencil'
         },
-        authSource: 'admin'
+        authSource: 'admin',
+        serverSelectionTimeoutMS: 1000
       };
 
       const badPasswordOptions = {
         auth: {
           user: 'both',
           password: 'pencil'
         },
-        authSource: 'admin'
+        authSource: 'admin',
+        serverSelectionTimeoutMS: 1000
       };
 
       const getErrorMsg = options =>
         withClient(
           this.configuration.newClient({}, options),
           () => Promise.reject(new Error('This request should have failed to authenticate')),
-          err => expect(err).to.match(/Authentication failed/)
+          err => {
+            const errMessage = err.reason ? err.reason.message : err;
+            expect(errMessage).to.match(/Authentication failed/);
+          }
         );
 
       return Promise.all([getErrorMsg(noUsernameOptions), getErrorMsg(badPasswordOptions)]);