From 8ca579c0b8959f0bace48eac34af123368def599 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 13 Oct 2020 14:59:35 -0400 Subject: [PATCH 1/5] allow for client connect after close --- src/mongo_client.ts | 16 +++++++++++++--- src/operations/connect.ts | 5 +++++ test/functional/connection.test.js | 28 ++++++++++++++++++++++++++++ test/functional/sessions.test.js | 4 ++-- 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 2e04576959c..6c413e4632d 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -346,19 +346,29 @@ export class MongoClient extends EventEmitter implements OperationParent { const force = typeof forceOrCallback === 'boolean' ? forceOrCallback : false; return maybePromise(callback, cb => { + const completeClose = (err?: AnyError) => { + // clear out references to old topology + this.topology = undefined; + this.s.dbCache = new Map(); + this.s.sessions = new Set(); + + cb(err); + }; + if (this.topology == null) { - return cb(); + completeClose(); + return; } const topology = this.topology; topology.close({ force }, err => { const autoEncrypter = topology.s.options.autoEncrypter; if (!autoEncrypter) { - cb(err); + completeClose(err); return; } - autoEncrypter.teardown(force, err2 => cb(err || err2)); + autoEncrypter.teardown(force, err2 => completeClose(err || err2)); }); }); } diff --git a/src/operations/connect.ts b/src/operations/connect.ts index 85700deb640..bcff0e6b2a3 100644 --- a/src/operations/connect.ts +++ b/src/operations/connect.ts @@ -197,6 +197,11 @@ export function connect( throw new Error('no callback function provided'); } + // Has a connection already been established? + if (mongoClient.topology && mongoClient.topology.isConnected()) { + throw new Error(`'connect' cannot be called when already connected`); + } + let didRequestAuthentication = false; const logger = new Logger('MongoClient', options); diff --git a/test/functional/connection.test.js b/test/functional/connection.test.js index 59e46c7e9cb..09469e5634b 100644 --- a/test/functional/connection.test.js +++ b/test/functional/connection.test.js @@ -2,6 +2,7 @@ const test = require('./shared').assert, setupDatabase = require('./shared').setupDatabase, expect = require('chai').expect; +const withClient = require('./shared').withClient; describe('Connection', function () { before(function () { @@ -273,4 +274,31 @@ describe('Connection', function () { done(); } }); + + it('should be able to connect again after close', function () { + return withClient.call(this, (client, done) => { + expect(client.isConnected()).to.be.true; + + const collection = () => client.db('testReconnect').collection('test'); + collection().insertOne({ a: 1 }, (err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + + client.close(err => { + expect(err).to.not.exist; + + client.connect(err => { + expect(err).to.not.exist; + + collection().insertOne({ b: 2 }, (err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + expect(client.topology.isDestroyed()).to.be.false; + done(); + }); + }); + }); + }); + }); + }); }); diff --git a/test/functional/sessions.test.js b/test/functional/sessions.test.js index 2751019bfb9..eb25a46ec87 100644 --- a/test/functional/sessions.test.js +++ b/test/functional/sessions.test.js @@ -123,7 +123,7 @@ describe('Sessions', function () { // verify that the `endSessions` command was sent const lastCommand = test.commands.started[test.commands.started.length - 1]; expect(lastCommand.commandName).to.equal('endSessions'); - expect(client.topology.s.sessionPool.sessions).to.have.length(0); + expect(client.topology).to.not.exist; }); }); }); @@ -143,7 +143,7 @@ describe('Sessions', function () { // verify that the `endSessions` command was sent const lastCommand = test.commands.started[test.commands.started.length - 1]; expect(lastCommand.commandName).to.equal('endSessions'); - expect(client.topology.s.sessionPool.sessions).to.have.length(0); + expect(client.topology).to.not.exist; }); }); } From f7ca2ff4cf925aeefca963b981540307c5d8b3a7 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 14 Oct 2020 09:48:52 -0400 Subject: [PATCH 2/5] bind withClient to this --- test/functional/connection.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/connection.test.js b/test/functional/connection.test.js index 09469e5634b..496b01c4654 100644 --- a/test/functional/connection.test.js +++ b/test/functional/connection.test.js @@ -276,7 +276,7 @@ describe('Connection', function () { }); it('should be able to connect again after close', function () { - return withClient.call(this, (client, done) => { + return withClient.bind(this)((client, done) => { expect(client.isConnected()).to.be.true; const collection = () => client.db('testReconnect').collection('test'); From c5e0309365d30915cd528603e2baecc823adb2f5 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 14 Oct 2020 10:44:46 -0400 Subject: [PATCH 3/5] respond to withClient comment --- test/functional/connection.test.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/functional/connection.test.js b/test/functional/connection.test.js index 496b01c4654..691ed3bdd66 100644 --- a/test/functional/connection.test.js +++ b/test/functional/connection.test.js @@ -275,8 +275,9 @@ describe('Connection', function () { } }); - it('should be able to connect again after close', function () { - return withClient.bind(this)((client, done) => { + it( + 'should be able to connect again after close', + withClient(function (client, done) { expect(client.isConnected()).to.be.true; const collection = () => client.db('testReconnect').collection('test'); @@ -299,6 +300,6 @@ describe('Connection', function () { }); }); }); - }); - }); + }) + ); }); From 4fe09116b75539d870f4126f78a0f0113fdc5095 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 14 Oct 2020 11:05:09 -0400 Subject: [PATCH 4/5] add additional connection expectations in test --- test/functional/connection.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/connection.test.js b/test/functional/connection.test.js index 691ed3bdd66..79497eb18d7 100644 --- a/test/functional/connection.test.js +++ b/test/functional/connection.test.js @@ -287,9 +287,11 @@ describe('Connection', function () { client.close(err => { expect(err).to.not.exist; + expect(client.isConnected()).to.be.false; client.connect(err => { expect(err).to.not.exist; + expect(client.isConnected()).to.be.true; collection().insertOne({ b: 2 }, (err, result) => { expect(err).to.not.exist; From 09dde518cd028592e80927f660024d1d686bff11 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 15 Oct 2020 11:04:04 -0400 Subject: [PATCH 5/5] use MongoError instead of plain Error --- src/operations/connect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/operations/connect.ts b/src/operations/connect.ts index bcff0e6b2a3..c99e5c81b08 100644 --- a/src/operations/connect.ts +++ b/src/operations/connect.ts @@ -199,7 +199,7 @@ export function connect( // Has a connection already been established? if (mongoClient.topology && mongoClient.topology.isConnected()) { - throw new Error(`'connect' cannot be called when already connected`); + throw new MongoError(`'connect' cannot be called when already connected`); } let didRequestAuthentication = false;