From 9b1ada16cce9e530ac6ff6de7e8d894730d0f477 Mon Sep 17 00:00:00 2001 From: emadum Date: Thu, 19 Nov 2020 10:24:56 -0500 Subject: [PATCH 01/14] test: reproduce NODE-2874 --- test/functional/mongo_client_options.test.js | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index 9eb5ff59a6..ef21a120e4 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -110,6 +110,28 @@ describe('MongoClient Options', function() { }); }); + it('NODE-2874: connectTimeoutMS=0 causes monitoring to time out', function(done) { + const heartbeatFrequencyMS = 500; + const client = this.configuration.newClient({ + connectTimeoutMS: 0, // no connect timeout + heartbeatFrequencyMS // fast 500ms heartbeat + }); + client.connect(() => { + // success after 5 heartbeats + const success = setTimeout(() => client.close(done), heartbeatFrequencyMS * 5); + + // fail on first error + const listener = ev => { + if (ev.newDescription.error) { + clearTimeout(success); + client.removeListener('serverDescriptionChanged', listener); + client.close(() => done(ev.newDescription.error)); + } + }; + client.on('serverDescriptionChanged', listener); + }); + }); + /** * @ignore */ From cbd208e7c97735d296426f9c43957e93ea35cff8 Mon Sep 17 00:00:00 2001 From: emadum Date: Thu, 19 Nov 2020 10:39:23 -0500 Subject: [PATCH 02/14] fix: monitor socket timeout should be unlimited when connectTimeoutMS=0 --- lib/core/sdam/monitor.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/core/sdam/monitor.js b/lib/core/sdam/monitor.js index 79bc6d0ac1..e86cba3d08 100644 --- a/lib/core/sdam/monitor.js +++ b/lib/core/sdam/monitor.js @@ -200,16 +200,19 @@ function checkServer(monitor, callback) { const topologyVersion = monitor[kServer].description.topologyVersion; const isAwaitable = topologyVersion != null; - const cmd = isAwaitable - ? { ismaster: true, maxAwaitTimeMS, topologyVersion: makeTopologyVersion(topologyVersion) } - : { ismaster: true }; - - const options = isAwaitable - ? { socketTimeout: connectTimeoutMS + maxAwaitTimeMS, exhaustAllowed: true } - : { socketTimeout: connectTimeoutMS }; - - if (isAwaitable && monitor[kRTTPinger] == null) { - monitor[kRTTPinger] = new RTTPinger(monitor[kCancellationToken], monitor.connectOptions); + const cmd = { ismaster: true }; + const options = { socketTimeout: connectTimeoutMS }; + + if (isAwaitable) { + cmd.maxAwaitTimeMS = maxAwaitTimeMS; + cmd.topologyVersion = makeTopologyVersion(topologyVersion); + if (connectTimeoutMS) { + options.socketTimeout = connectTimeoutMS + maxAwaitTimeMS; + } + options.exhaustAllowed = true; + if (monitor[kRTTPinger] == null) { + monitor[kRTTPinger] = new RTTPinger(monitor[kCancellationToken], monitor.connectOptions); + } } monitor[kConnection].command('admin.$cmd', cmd, options, (err, result) => { From a86b3837bf98728dc7832fa92e145a78ad9112da Mon Sep 17 00:00:00 2001 From: emadum Date: Fri, 20 Nov 2020 11:57:40 -0500 Subject: [PATCH 03/14] potential fix for failing test --- test/unit/sdam/srv_polling.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/sdam/srv_polling.test.js b/test/unit/sdam/srv_polling.test.js index ccf824a197..bc4bdb9790 100644 --- a/test/unit/sdam/srv_polling.test.js +++ b/test/unit/sdam/srv_polling.test.js @@ -123,6 +123,7 @@ describe('Mongos SRV Polling', function() { const poller = new SrvPoller({ srvHost: SRV_HOST }); context.sinon.stub(dns, 'resolveSrv'); + stubPoller(poller); poller._poll(); From fc178e239a5d35870de291dc1b829dd97ee950ac Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Wed, 25 Nov 2020 08:54:21 -0500 Subject: [PATCH 04/14] fix: transition topology state before async calls When the legacy ReplSet and Server topology types are destroyed they transition their internal state after a potentially long async close process has completed. With the recently introduced infinite default socket timeout, we are hitting this race condition more consistently. This patch ensures the types transition their state immediately, reducing the chance that such race conditions can occur. NODE-2916 --- lib/core/topologies/replset.js | 17 +++++---- lib/core/topologies/server.js | 5 +-- .../core/rs_mocks/connection.test.js | 36 +++++++------------ 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/lib/core/topologies/replset.js b/lib/core/topologies/replset.js index deaa60fa7a..0be880f998 100644 --- a/lib/core/topologies/replset.js +++ b/lib/core/topologies/replset.js @@ -541,7 +541,7 @@ var monitorServer = function(host, self, options) { self.s.options.secondaryOnlyConnectionAllowed) || self.s.replicaSetState.hasPrimary()) ) { - self.state = CONNECTED; + stateTransition(self, CONNECTED); // Emit connected sign process.nextTick(function() { @@ -558,7 +558,7 @@ var monitorServer = function(host, self, options) { self.s.options.secondaryOnlyConnectionAllowed) || self.s.replicaSetState.hasPrimary()) ) { - self.state = CONNECTED; + stateTransition(self, CONNECTED); // Rexecute any stalled operation rexecuteOperations(self); @@ -787,7 +787,7 @@ function handleInitialConnectEvent(self, event) { // Do we have a primary or primaryAndSecondary if (shouldTriggerConnect(self)) { // We are connected - self.state = CONNECTED; + stateTransition(self, CONNECTED); // Set initial connect state self.initialConnectState.connect = true; @@ -975,14 +975,19 @@ ReplSet.prototype.destroy = function(options, callback) { // Emit toplogy closing event emitSDAMEvent(this, 'topologyClosed', { topologyId: this.id }); - // Transition state - stateTransition(this, DESTROYED); - if (typeof callback === 'function') { callback(null, null); } }; + if (this.state === DESTROYED) { + if (typeof callback === 'function') callback(null, null); + return; + } + + // Transition state + stateTransition(this, DESTROYED); + // Clear out any monitoring process if (this.haTimeoutId) clearTimeout(this.haTimeoutId); diff --git a/lib/core/topologies/server.js b/lib/core/topologies/server.js index ecf46135ca..c6a0bfa5db 100644 --- a/lib/core/topologies/server.js +++ b/lib/core/topologies/server.js @@ -864,12 +864,14 @@ Server.prototype.destroy = function(options, callback) { } // No pool, return - if (!self.s.pool) { + if (!self.s.pool || this._destroyed) { this._destroyed = true; if (typeof callback === 'function') callback(null, null); return; } + this._destroyed = true; + // Emit close event if (options.emitClose) { self.emit('close', self); @@ -900,7 +902,6 @@ Server.prototype.destroy = function(options, callback) { // Destroy the pool this.s.pool.destroy(options.force, callback); - this._destroyed = true; }; /** diff --git a/test/functional/core/rs_mocks/connection.test.js b/test/functional/core/rs_mocks/connection.test.js index 34c4fb9a10..74a3f962be 100644 --- a/test/functional/core/rs_mocks/connection.test.js +++ b/test/functional/core/rs_mocks/connection.test.js @@ -135,8 +135,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.not.be.null; expect(server.s.replicaSetState.primary.name).to.equal('localhost:32000'); - server.destroy(); - done(); + server.destroy(done); } } }); @@ -251,8 +250,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.not.be.null; expect(server.s.replicaSetState.primary.name).to.equal('localhost:32000'); - server.destroy(); - done(); + server.destroy(done); } } }); @@ -351,8 +349,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.not.be.null; expect(server.s.replicaSetState.primary.name).to.equal('localhost:32000'); - server.destroy(); - done(); + server.destroy(done); } // Joined @@ -428,8 +425,7 @@ describe('ReplSet Connection Tests (mocks)', function() { ); server.on('error', function() { - server.destroy(); - done(); + server.destroy(done); }); server.connect(); @@ -528,8 +524,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.be.null; - server.destroy(); - done(); + server.destroy(done); } }); @@ -652,8 +647,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.not.be.null; expect(server.s.replicaSetState.primary.name).to.equal('localhost:32000'); - server.destroy(); - done(); + server.destroy(done); } } }); @@ -759,8 +753,7 @@ describe('ReplSet Connection Tests (mocks)', function() { ); server.on('error', function() { - server.destroy(); - done(); + server.destroy(done); }); // Gives proxies a chance to boot up @@ -857,8 +850,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.not.be.null; expect(server.s.replicaSetState.primary.name).to.equal('localhost:32000'); - server.destroy(); - done(); + server.destroy(done); } } }); @@ -979,8 +971,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.not.be.null; expect(server.s.replicaSetState.primary.name).to.equal('localhost:32000'); - server.destroy(); - done(); + server.destroy(done); } } }); @@ -1070,8 +1061,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.s.replicaSetState.primary).to.not.be.null; expect(server.s.replicaSetState.primary.name).to.equal('localhost:32000'); - server.destroy(); - done(); + server.destroy(done); } } }); @@ -1182,8 +1172,7 @@ describe('ReplSet Connection Tests (mocks)', function() { expect(server.__connected).to.be.true; expect(server.__fullsetup).to.be.true; - server.destroy(); - done(); + server.destroy(done); }); server.connect(); @@ -1274,8 +1263,7 @@ describe('ReplSet Connection Tests (mocks)', function() { var result = server.lastIsMaster(); expect(result).to.exist; - server.destroy(); - done(); + server.destroy(done); }); server.connect(); From 1fdb4964698bb3e50883896ee3c44bb7904ed876 Mon Sep 17 00:00:00 2001 From: emadum Date: Sun, 29 Nov 2020 16:10:48 -0500 Subject: [PATCH 05/14] cleanup --- test/unit/sdam/srv_polling.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/sdam/srv_polling.test.js b/test/unit/sdam/srv_polling.test.js index bc4bdb9790..ccf824a197 100644 --- a/test/unit/sdam/srv_polling.test.js +++ b/test/unit/sdam/srv_polling.test.js @@ -123,7 +123,6 @@ describe('Mongos SRV Polling', function() { const poller = new SrvPoller({ srvHost: SRV_HOST }); context.sinon.stub(dns, 'resolveSrv'); - stubPoller(poller); poller._poll(); From 14b671f48873d6c163dea425ba75810d644893e5 Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 14:06:53 -0500 Subject: [PATCH 06/14] refactor test to avoid setTimeout --- test/functional/mongo_client_options.test.js | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index ef21a120e4..a3a0bb7322 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -110,25 +110,25 @@ describe('MongoClient Options', function() { }); }); - it('NODE-2874: connectTimeoutMS=0 causes monitoring to time out', function(done) { - const heartbeatFrequencyMS = 500; + it('monitoring should not time out when connectTimeoutMS=0', function(done) { const client = this.configuration.newClient({ connectTimeoutMS: 0, // no connect timeout - heartbeatFrequencyMS // fast 500ms heartbeat + heartbeatFrequencyMS: 500 // fast heartbeat }); + const heartbeats = {}; + let isDone = false; + function _done(err) { + if (isDone) return; + isDone = true; + client.close(closeErr => done(err || closeErr)); + } client.connect(() => { - // success after 5 heartbeats - const success = setTimeout(() => client.close(done), heartbeatFrequencyMS * 5); - - // fail on first error - const listener = ev => { - if (ev.newDescription.error) { - clearTimeout(success); - client.removeListener('serverDescriptionChanged', listener); - client.close(() => done(ev.newDescription.error)); - } - }; - client.on('serverDescriptionChanged', listener); + // success after 3 successful heartbeats on a given connection + client.on('serverHeartbeatSucceeded', ev => { + if (!heartbeats[ev.connectionId]) heartbeats[ev.connectionId] = 0; + if (++heartbeats[ev.connectionId] >= 3) _done(); + }); + client.on('serverHeartbeatFailed', ev => _done(ev.failure)); }); }); From 78468d15fd608d776a11d423dae234a5936f691b Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 14:34:04 -0500 Subject: [PATCH 07/14] cleanup --- test/functional/mongo_client_options.test.js | 43 +++++++++++--------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index a3a0bb7322..b0a0ad605c 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -110,26 +110,31 @@ describe('MongoClient Options', function() { }); }); - it('monitoring should not time out when connectTimeoutMS=0', function(done) { - const client = this.configuration.newClient({ - connectTimeoutMS: 0, // no connect timeout - heartbeatFrequencyMS: 500 // fast heartbeat - }); - const heartbeats = {}; - let isDone = false; - function _done(err) { - if (isDone) return; - isDone = true; - client.close(closeErr => done(err || closeErr)); - } - client.connect(() => { - // success after 3 successful heartbeats on a given connection - client.on('serverHeartbeatSucceeded', ev => { - if (!heartbeats[ev.connectionId]) heartbeats[ev.connectionId] = 0; - if (++heartbeats[ev.connectionId] >= 3) _done(); + it('monitoring should not time out when connectTimeoutMS=0', { + metadata: { requires: { topology: 'replicaset' } }, + test: function(done) { + const client = this.configuration.newClient({ + connectTimeoutMS: 0, // no connect timeout + heartbeatFrequencyMS: 500 // fast heartbeat }); - client.on('serverHeartbeatFailed', ev => _done(ev.failure)); - }); + const heartbeats = {}; + let isDone = false; + function _done(err) { + if (isDone) return; + isDone = true; + client.close(closeErr => done(err || closeErr)); + } + client.connect(() => { + // succeed after 3 successful heartbeats on any connection + client.on('serverHeartbeatSucceeded', ev => { + if (!heartbeats[ev.connectionId]) heartbeats[ev.connectionId] = 0; + if (++heartbeats[ev.connectionId] >= 3) _done(); + }); + + // fail on first failed heartbeat + client.on('serverHeartbeatFailed', ev => _done(ev.failure)); + }); + } }); /** From b80d46dc8ad4a8e54493e2967ac4322c01f60ae3 Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 17:24:22 -0500 Subject: [PATCH 08/14] improve tests --- test/functional/mongo_client_options.test.js | 64 ++++++++++++++------ 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index b0a0ad605c..300ef600c5 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -2,6 +2,9 @@ const test = require('./shared').assert; const setupDatabase = require('./shared').setupDatabase; const expect = require('chai').expect; +const sinon = require('sinon'); +const LegacyConnection = require('../../lib/core/connection/connection'); +const Connection = require('../../lib/cmap/connection').Connection; describe('MongoClient Options', function() { before(function() { @@ -110,30 +113,55 @@ describe('MongoClient Options', function() { }); }); - it('monitoring should not time out when connectTimeoutMS=0', { + it('must respect an infinite connectTimeoutMS for the streaming protocol', { metadata: { requires: { topology: 'replicaset' } }, test: function(done) { + if (!this.configuration.usingUnifiedTopology()) return done(); + const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { + const args = Array.prototype.slice.call(arguments); + const ns = args[0]; + const options = args[2]; + if (ns === 'admin.$cmd' && options.exhaustAllowed) { + expect(options) + .property('socketTimeout') + .to.equal(0); + done(); + } + stub.wrappedMethod.apply(this, args); + }); + this.defer(() => stub.restore()); const client = this.configuration.newClient({ - connectTimeoutMS: 0, // no connect timeout - heartbeatFrequencyMS: 500 // fast heartbeat + connectTimeoutMS: 0, + heartbeatFrequencyMS: 500 }); - const heartbeats = {}; - let isDone = false; - function _done(err) { - if (isDone) return; - isDone = true; - client.close(closeErr => done(err || closeErr)); - } - client.connect(() => { - // succeed after 3 successful heartbeats on any connection - client.on('serverHeartbeatSucceeded', ev => { - if (!heartbeats[ev.connectionId]) heartbeats[ev.connectionId] = 0; - if (++heartbeats[ev.connectionId] >= 3) _done(); - }); + this.defer(() => client.close()); + client.connect(err => expect(err).to.not.exist); + } + }); - // fail on first failed heartbeat - client.on('serverHeartbeatFailed', ev => _done(ev.failure)); + it('must respect a non-infinite connectTimeoutMS for the streaming protocol', { + metadata: { requires: { topology: 'replicaset' } }, + test: function(done) { + if (!this.configuration.usingUnifiedTopology()) return done(); + const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { + const args = Array.prototype.slice.call(arguments); + const ns = args[0]; + const options = args[2]; + if (ns === 'admin.$cmd' && options.exhaustAllowed) { + expect(options) + .property('socketTimeout') + .to.equal(510); + done(); + } + stub.wrappedMethod.apply(this, args); + }); + this.defer(() => stub.restore()); + const client = this.configuration.newClient({ + connectTimeoutMS: 10, + heartbeatFrequencyMS: 500 }); + this.defer(() => client.close()); + client.connect(err => expect(err).to.not.exist); } }); From c840960d9ebda6b15deaafd6b4d49dce7e4fd415 Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 17:27:01 -0500 Subject: [PATCH 09/14] remove unused require --- test/functional/mongo_client_options.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index 300ef600c5..9916e2dc1f 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -3,7 +3,6 @@ const test = require('./shared').assert; const setupDatabase = require('./shared').setupDatabase; const expect = require('chai').expect; const sinon = require('sinon'); -const LegacyConnection = require('../../lib/core/connection/connection'); const Connection = require('../../lib/cmap/connection').Connection; describe('MongoClient Options', function() { From c6b36380866b51feb6261210c5d76ecbec6d749a Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 17:29:50 -0500 Subject: [PATCH 10/14] cleanup --- test/functional/mongo_client_options.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index 9916e2dc1f..9b031080ab 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -119,8 +119,9 @@ describe('MongoClient Options', function() { const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { const args = Array.prototype.slice.call(arguments); const ns = args[0]; + const command = args[1]; const options = args[2]; - if (ns === 'admin.$cmd' && options.exhaustAllowed) { + if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { expect(options) .property('socketTimeout') .to.equal(0); @@ -145,8 +146,9 @@ describe('MongoClient Options', function() { const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { const args = Array.prototype.slice.call(arguments); const ns = args[0]; + const command = args[1]; const options = args[2]; - if (ns === 'admin.$cmd' && options.exhaustAllowed) { + if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { expect(options) .property('socketTimeout') .to.equal(510); From 8e1014380a63ff66d8aba266e4c0e55935a2e9c3 Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 18:04:39 -0500 Subject: [PATCH 11/14] limit streaming tests to mongodb >= 4.4 --- test/functional/mongo_client_options.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index 9b031080ab..a6d3992f51 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -113,7 +113,7 @@ describe('MongoClient Options', function() { }); it('must respect an infinite connectTimeoutMS for the streaming protocol', { - metadata: { requires: { topology: 'replicaset' } }, + metadata: { requires: { topology: 'replicaset', mongodb: '>= 4.4' } }, test: function(done) { if (!this.configuration.usingUnifiedTopology()) return done(); const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { @@ -122,6 +122,7 @@ describe('MongoClient Options', function() { const command = args[1]; const options = args[2]; if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { + stub.restore(); expect(options) .property('socketTimeout') .to.equal(0); @@ -129,7 +130,6 @@ describe('MongoClient Options', function() { } stub.wrappedMethod.apply(this, args); }); - this.defer(() => stub.restore()); const client = this.configuration.newClient({ connectTimeoutMS: 0, heartbeatFrequencyMS: 500 @@ -140,7 +140,7 @@ describe('MongoClient Options', function() { }); it('must respect a non-infinite connectTimeoutMS for the streaming protocol', { - metadata: { requires: { topology: 'replicaset' } }, + metadata: { requires: { topology: 'replicaset', mongodb: '>= 4.4' } }, test: function(done) { if (!this.configuration.usingUnifiedTopology()) return done(); const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { @@ -149,6 +149,7 @@ describe('MongoClient Options', function() { const command = args[1]; const options = args[2]; if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { + stub.restore(); expect(options) .property('socketTimeout') .to.equal(510); @@ -156,7 +157,6 @@ describe('MongoClient Options', function() { } stub.wrappedMethod.apply(this, args); }); - this.defer(() => stub.restore()); const client = this.configuration.newClient({ connectTimeoutMS: 10, heartbeatFrequencyMS: 500 From 3592d70f410234906a1a62b4605f3b7163f26eb1 Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 18:15:50 -0500 Subject: [PATCH 12/14] fix leak --- test/functional/mongo_client_options.test.js | 22 +++++++++----------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index a6d3992f51..85e2f9102d 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -116,6 +116,10 @@ describe('MongoClient Options', function() { metadata: { requires: { topology: 'replicaset', mongodb: '>= 4.4' } }, test: function(done) { if (!this.configuration.usingUnifiedTopology()) return done(); + const client = this.configuration.newClient({ + connectTimeoutMS: 0, + heartbeatFrequencyMS: 500 + }); const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { const args = Array.prototype.slice.call(arguments); const ns = args[0]; @@ -126,15 +130,10 @@ describe('MongoClient Options', function() { expect(options) .property('socketTimeout') .to.equal(0); - done(); + client.close(done); } stub.wrappedMethod.apply(this, args); }); - const client = this.configuration.newClient({ - connectTimeoutMS: 0, - heartbeatFrequencyMS: 500 - }); - this.defer(() => client.close()); client.connect(err => expect(err).to.not.exist); } }); @@ -143,6 +142,10 @@ describe('MongoClient Options', function() { metadata: { requires: { topology: 'replicaset', mongodb: '>= 4.4' } }, test: function(done) { if (!this.configuration.usingUnifiedTopology()) return done(); + const client = this.configuration.newClient({ + connectTimeoutMS: 10, + heartbeatFrequencyMS: 500 + }); const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { const args = Array.prototype.slice.call(arguments); const ns = args[0]; @@ -153,15 +156,10 @@ describe('MongoClient Options', function() { expect(options) .property('socketTimeout') .to.equal(510); - done(); + client.close(done); } stub.wrappedMethod.apply(this, args); }); - const client = this.configuration.newClient({ - connectTimeoutMS: 10, - heartbeatFrequencyMS: 500 - }); - this.defer(() => client.close()); client.connect(err => expect(err).to.not.exist); } }); From 9fdbdc5e84c8fab2657436606e4c5525c8fe211b Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 18:28:02 -0500 Subject: [PATCH 13/14] stub after connect --- test/functional/mongo_client_options.test.js | 60 +++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index 85e2f9102d..951c7247b2 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -120,21 +120,23 @@ describe('MongoClient Options', function() { connectTimeoutMS: 0, heartbeatFrequencyMS: 500 }); - const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { - const args = Array.prototype.slice.call(arguments); - const ns = args[0]; - const command = args[1]; - const options = args[2]; - if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { - stub.restore(); - expect(options) - .property('socketTimeout') - .to.equal(0); - client.close(done); - } - stub.wrappedMethod.apply(this, args); + client.connect(err => { + expect(err).to.not.exist; + const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { + const args = Array.prototype.slice.call(arguments); + const ns = args[0]; + const command = args[1]; + const options = args[2]; + if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { + stub.restore(); + expect(options) + .property('socketTimeout') + .to.equal(0); + client.close(done); + } + stub.wrappedMethod.apply(this, args); + }); }); - client.connect(err => expect(err).to.not.exist); } }); @@ -146,21 +148,23 @@ describe('MongoClient Options', function() { connectTimeoutMS: 10, heartbeatFrequencyMS: 500 }); - const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { - const args = Array.prototype.slice.call(arguments); - const ns = args[0]; - const command = args[1]; - const options = args[2]; - if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { - stub.restore(); - expect(options) - .property('socketTimeout') - .to.equal(510); - client.close(done); - } - stub.wrappedMethod.apply(this, args); + client.connect(err => { + expect(err).to.not.exist; + const stub = sinon.stub(Connection.prototype, 'command').callsFake(function() { + const args = Array.prototype.slice.call(arguments); + const ns = args[0]; + const command = args[1]; + const options = args[2]; + if (ns === 'admin.$cmd' && command.ismaster && options.exhaustAllowed) { + stub.restore(); + expect(options) + .property('socketTimeout') + .to.equal(510); + client.close(done); + } + stub.wrappedMethod.apply(this, args); + }); }); - client.connect(err => expect(err).to.not.exist); } }); From 324f85340ced4c9af34e19ad3eb833a3d25a00b5 Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 30 Nov 2020 20:58:57 -0500 Subject: [PATCH 14/14] non-infinite = finite --- test/functional/mongo_client_options.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/mongo_client_options.test.js b/test/functional/mongo_client_options.test.js index 951c7247b2..50cc791abd 100644 --- a/test/functional/mongo_client_options.test.js +++ b/test/functional/mongo_client_options.test.js @@ -140,7 +140,7 @@ describe('MongoClient Options', function() { } }); - it('must respect a non-infinite connectTimeoutMS for the streaming protocol', { + it('must respect a finite connectTimeoutMS for the streaming protocol', { metadata: { requires: { topology: 'replicaset', mongodb: '>= 4.4' } }, test: function(done) { if (!this.configuration.usingUnifiedTopology()) return done();