refactor(NODE-3273): remove undefined error param from topology open event (#3033)

* refactor(NODE-3273): remove undefined error param from topology open event

* rename test

* Update test/functional/connection.test.js

Change vars to consts

Co-authored-by: Neal Beeken <[email protected]>

* Fix up test based on code review

* Update tests in connection.test.js to use afterEach

* Fix tests after code review

* Fix hanging test issue

Co-authored-by: Neal Beeken <[email protected]>

Author: ljhaywar

src/sdam/topology.ts

@@ -182,8 +182,8 @@ export type TopologyEvents = {
   topologyOpening(event: TopologyOpeningEvent): void;
   topologyDescriptionChanged(event: TopologyDescriptionChangedEvent): void;
   error(error: Error): void;
-  /** TODO(NODE-3273) - remove error @internal */
-  open(error: undefined, topology: Topology): void;
+  /** @internal */
+  open(topology: Topology): void;
   close(): void;
   timeout(): void;
 } & Omit<ServerEvents, 'connect'> &
@@ -456,8 +456,7 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
           }
 
           stateTransition(this, STATE_CONNECTED);
-          // TODO(NODE-3273) - remove err
-          this.emit(Topology.OPEN, err, this);
+          this.emit(Topology.OPEN, this);
           this.emit(Topology.CONNECT, this);
 
           if (typeof callback === 'function') callback(undefined, this);
@@ -467,8 +466,7 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
       }
 
       stateTransition(this, STATE_CONNECTED);
-      // TODO(NODE-3273) - remove err
-      this.emit(Topology.OPEN, err, this);
+      this.emit(Topology.OPEN, this);
       this.emit(Topology.CONNECT, this);
 
       if (typeof callback === 'function') callback(undefined, this);

test/functional/connection.test.js

@@ -3,17 +3,42 @@ const { withClient, setupDatabase } = require('./shared');
 const test = require('./shared').assert;
 const { expect } = require('chai');
 const { ServerHeartbeatStartedEvent, MongoClient } = require('../../src');
+const { Topology } = require('../../src/sdam/topology');
 
 describe('Connection - functional', function () {
+  let client;
+  let testClient;
+
   before(function () {
     return setupDatabase(this.configuration);
   });
 
+  afterEach(async () => {
+    let savedError;
+    if (client) {
+      try {
+        await client.close();
+      } catch (err) {
+        savedError = err;
+      }
+    }
+    if (testClient) {
+      try {
+        await testClient.close();
+      } catch (err) {
+        savedError = err;
+      }
+    }
+    if (savedError) {
+      throw savedError;
+    }
+  });
+
   it('should correctly start monitoring for single server connection', {
     metadata: { requires: { topology: 'single', os: '!win32' } },
-    test() {
+    test: function (done) {
       var configuration = this.configuration;
-      var client = configuration.newClient(
+      client = configuration.newClient(
         `mongodb://${encodeURIComponent('/tmp/mongodb-27017.sock')}?w=1`,
         {
           maxPoolSize: 1,
@@ -27,12 +52,10 @@ describe('Connection - functional', function () {
         isMonitoring = event instanceof ServerHeartbeatStartedEvent;
       });
 
-      return client
-        .connect()
-        .then(() => {
-          expect(isMonitoring);
-        })
-        .finally(() => client.close());
+      client.connect().then(() => {
+        expect(isMonitoring);
+        done();
+      });
     }
   });
 
@@ -41,7 +64,7 @@ describe('Connection - functional', function () {
 
     test: function (done) {
       var configuration = this.configuration;
-      var client = configuration.newClient(
+      client = configuration.newClient(
         `mongodb://${encodeURIComponent('/tmp/mongodb-27017.sock')}?w=1`,
         { maxPoolSize: 1 }
       );
@@ -62,22 +85,37 @@ describe('Connection - functional', function () {
                 expect(err).to.not.exist;
                 test.equal(1, items.length);
 
-                client.close(done);
+                done();
               });
           }
         );
       });
     }
   });
 
+  it('should only pass one argument (topology and not error) for topology "open" events', function (done) {
+    const configuration = this.configuration;
+    client = configuration.newClient({ w: 1 }, { maxPoolSize: 1 });
+
+    client.on('topologyOpening', () => {
+      client.topology.on('open', (...args) => {
+        expect(args).to.have.lengthOf(1);
+        expect(args[0]).to.be.instanceOf(Topology);
+        done();
+      });
+    });
+
+    client.connect();
+  });
+
   it('should correctly connect to server using just events', function (done) {
     var configuration = this.configuration;
-    var client = configuration.newClient({ w: 1 }, { maxPoolSize: 1 });
+    client = configuration.newClient({ w: 1 }, { maxPoolSize: 1 });
 
     client.on('open', clientFromEvent => {
       expect(clientFromEvent).to.be.instanceOf(MongoClient);
       expect(clientFromEvent).to.equal(client);
-      clientFromEvent.close(done);
+      done();
     });
 
     client.connect();
@@ -90,9 +128,9 @@ describe('Connection - functional', function () {
 
     test: function (done) {
       var configuration = this.configuration;
-      var client = configuration.newClient({ w: 1 }, { maxPoolSize: 2000 });
+      client = configuration.newClient({ w: 1 }, { maxPoolSize: 2000 });
       client.on('open', function () {
-        client.close(done);
+        done();
       });
 
       client.connect();
@@ -130,11 +168,11 @@ describe('Connection - functional', function () {
 
     test: function (done) {
       const configuration = this.configuration;
-      const client = configuration.newClient();
+      client = configuration.newClient();
 
       client.connect(
-        connectionTester(configuration, 'testConnectNoOptions', function (client) {
-          client.close(done);
+        connectionTester(configuration, 'testConnectNoOptions', function () {
+          done();
         })
       );
     }
@@ -148,24 +186,24 @@ describe('Connection - functional', function () {
       const username = 'testConnectGoodAuth';
       const password = 'password';
 
-      const setupClient = configuration.newClient();
+      client = configuration.newClient();
 
       // First add a user.
-      setupClient.connect(function (err, client) {
+      client.connect(function (err, client) {
         expect(err).to.not.exist;
         var db = client.db(configuration.db);
 
         db.addUser(username, password, function (err) {
           expect(err).to.not.exist;
-          client.close(restOfTest);
+          restOfTest();
         });
       });
 
       function restOfTest() {
-        const testClient = configuration.newClient(configuration.url({ username, password }));
+        testClient = configuration.newClient(configuration.url({ username, password }));
         testClient.connect(
-          connectionTester(configuration, 'testConnectGoodAuth', function (client) {
-            client.close(done);
+          connectionTester(configuration, 'testConnectGoodAuth', function () {
+            done();
           })
         );
       }
@@ -181,25 +219,25 @@ describe('Connection - functional', function () {
       const password = 'password';
 
       // First add a user.
-      const setupClient = configuration.newClient();
-      setupClient.connect(function (err, client) {
+      client = configuration.newClient();
+      client.connect(function (err, client) {
         expect(err).to.not.exist;
         var db = client.db(configuration.db);
 
         db.addUser(username, password, { roles: ['read'] }, function (err) {
           expect(err).to.not.exist;
-          client.close(restOfTest);
+          restOfTest();
         });
       });
 
       function restOfTest() {
         var opts = { auth: { username, password }, authSource: configuration.db };
 
-        const testClient = configuration.newClient(opts);
+        testClient = configuration.newClient(opts);
 
         testClient.connect(
-          connectionTester(configuration, 'testConnectGoodAuthAsOption', function (client) {
-            client.close(done);
+          connectionTester(configuration, 'testConnectGoodAuthAsOption', function () {
+            done();
           })
         );
       }
@@ -211,7 +249,7 @@ describe('Connection - functional', function () {
 
     test: function (done) {
       var configuration = this.configuration;
-      const client = configuration.newClient(
+      client = configuration.newClient(
         configuration.url({ username: 'slithy', password: 'toves' })
       );
       client.connect(function (err, client) {