Forget last router when rediscovery fails

Last router in the routing table was not forgotten correctly when none of the
routers respond. This commit makes code correctly foget it.

Author: lutovich

src/v1/internal/connection-providers.js

@@ -104,10 +104,8 @@ export class LoadBalancer extends ConnectionProvider {
         } else {
           // returned routing table was undefined, this means a connection error happened and we need to forget the
           // previous router and try the next one
-          const previousRouter = knownRouters[currentIndex - 1];
-          if (previousRouter) {
-            currentRoutingTable.forgetRouter(previousRouter);
-          }
+          const previousRouterIndex = currentIndex - 1;
+          this._forgetRouter(currentRoutingTable, knownRouters, previousRouterIndex);
         }
 
         // try next router
@@ -116,12 +114,17 @@ export class LoadBalancer extends ConnectionProvider {
       });
     }, Promise.resolve(null));
 
-    // todo: who removes the final router when all routers fail?
     return refreshedTablePromise.then(newRoutingTable => {
       if (newRoutingTable && !newRoutingTable.writers.isEmpty()) {
         this._updateRoutingTable(newRoutingTable);
         return newRoutingTable;
       }
+
+      // forget the last known router because it did not return a valid routing table
+      const lastRouterIndex = knownRouters.length - 1;
+      this._forgetRouter(currentRoutingTable, knownRouters, lastRouterIndex);
+
+      // none of the existing routers returned valid routing table, throw exception
       throw newError('Could not perform discovery. No routing servers available.', SERVICE_UNAVAILABLE);
     });
   }
@@ -142,4 +145,11 @@ export class LoadBalancer extends ConnectionProvider {
     // make this driver instance aware of the new table
     this._routingTable = newRoutingTable;
   }
+
+  _forgetRouter(routingTable, routersArray, routerIndex) {
+    const address = routersArray[routerIndex];
+    if (address) {
+      routingTable.forgetRouter(address);
+    }
+  }
 }

test/internal/connection-providers.test.js

@@ -539,6 +539,46 @@ describe('LoadBalancer', () => {
     });
   });
 
+  it('forgets all routers when they fail while acquiring read connection', done => {
+    const loadBalancer = newLoadBalancer(
+      ['server-1', 'server-2', 'server-3'],
+      ['server-4', 'server-5'],
+      ['server-6', 'server-7'],
+      newPool(),
+      int(0) // expired routing table
+    );
+
+    loadBalancer.acquireConnection(READ).catch(error => {
+      expect(error.code).toEqual(SERVICE_UNAVAILABLE);
+      expectRoutingTable(loadBalancer,
+        [],
+        ['server-4', 'server-5'],
+        ['server-6', 'server-7']
+      );
+      done();
+    });
+  });
+
+  it('forgets all routers when they fail while acquiring write connection', done => {
+    const loadBalancer = newLoadBalancer(
+      ['server-1', 'server-2', 'server-3'],
+      ['server-4', 'server-5'],
+      ['server-6', 'server-7'],
+      newPool(),
+      int(0) // expired routing table
+    );
+
+    loadBalancer.acquireConnection(WRITE).catch(error => {
+      expect(error.code).toEqual(SERVICE_UNAVAILABLE);
+      expectRoutingTable(loadBalancer,
+        [],
+        ['server-4', 'server-5'],
+        ['server-6', 'server-7']
+      );
+      done();
+    });
+  });
+
 });
 
 function newLoadBalancer(routers, readers, writers, pool = null, expirationTime = Integer.MAX_VALUE, routerToRoutingTable = {}) {