Unify Connection Acquisition Timeout with other Drivers (#1292)

we’ve decided upon unifying the drivers to consider the timeout to count towards everything needed for acquiring a connection. This can include multiple connection acquisitions, ROUTE request, custom resolver/provider calls, etc; anything that’s needed to get a connection for the session.

Author: MaxAake

packages/bolt-connection/src/connection-provider/connection-provider-routing.js

@@ -89,6 +89,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
     this._loadBalancingStrategy = new LeastConnectedLoadBalancingStrategy(
       this._connectionPool
     )
+    this._startTime = 0
     this._hostNameResolver = hostNameResolver
     this._dnsResolver = new HostNameResolver()
     this._log = log
@@ -152,6 +153,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
       (error, address, conn) => this._handleSecurityError(error, address, conn, context.database)
     )
 
+    this._startTime = new Date().getTime()
     let conn
     if (this.SSREnabled() && homeDb !== undefined && database === '') {
       const currentRoutingTable = this._routingTableRegistry.get(
@@ -205,7 +207,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
     }
 
     try {
-      const connection = await this._connectionPool.acquire({ auth }, address)
+      const connection = await this._connectionPool.acquire({ auth, startTime: this._startTime }, address)
 
       if (auth) {
         await this._verifyStickyConnection({
@@ -585,7 +587,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
 
   async _createSessionForRediscovery (routerAddress, bookmarks, impersonatedUser, auth) {
     try {
-      const connection = await this._connectionPool.acquire({ auth }, routerAddress)
+      const connection = await this._connectionPool.acquire({ auth, startTime: this._startTime }, routerAddress)
 
       if (auth) {
         await this._verifyStickyConnection({

packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js

@@ -3494,7 +3494,7 @@ describe.each([
             .catch(functional.identity)
 
           expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
-          expect(poolAcquire).toHaveBeenCalledWith({ auth }, server3)
+          expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server3)
           expect(connection.release).toHaveBeenCalled()
           expect(connection._sticky).toEqual(isStickyConn)
         })
@@ -3536,7 +3536,7 @@ describe.each([
             .catch(functional.identity)
 
           expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
-          expect(poolAcquire).toHaveBeenCalledWith({ auth }, server1)
+          expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server1)
           expect(connection.release).toHaveBeenCalled()
           expect(connection._sticky).toEqual(isStickyConn)
         })
@@ -3580,7 +3580,7 @@ describe.each([
             .catch(functional.identity)
 
           expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
-          expect(poolAcquire).toHaveBeenCalledWith({ auth }, server0)
+          expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server0)
           expect(connection.release).toHaveBeenCalled()
           expect(connection._sticky).toEqual(isStickyConn)
         })
@@ -3609,7 +3609,7 @@ describe.each([
             .catch(functional.identity)
 
           expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
-          expect(poolAcquire).toHaveBeenCalledWith({ auth }, server0)
+          expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server0)
           expect(connection.release).toHaveBeenCalled()
           expect(connection._sticky).toEqual(isStickyConn)
         })

packages/core/src/internal/pool/pool.ts

@@ -28,6 +28,7 @@ type ValidateOnRelease<R extends unknown = unknown> = (resource: R) => (Promise<
 type InstallObserver<R extends unknown = unknown> = (resource: R, observer: unknown) => void
 type RemoveObserver<R extends unknown = unknown> = (resource: R) => void
 interface AcquisitionConfig { requireNew?: boolean }
+interface AcquisitionContext { startTime?: number, forceReAuth?: boolean, skipReAuth?: boolean, auth?: object, triggerValidation?: boolean }
 
 interface ConstructorParam<R extends unknown = unknown> {
   create?: Create<R>
@@ -111,12 +112,13 @@ class Pool<R extends unknown = unknown> {
    * @param {boolean} config.requireNew Indicate it requires a new resource
    * @return {Promise<Object>} resource that is ready to use.
    */
-  async acquire (acquisitionContext: unknown, address: ServerAddress, config?: AcquisitionConfig): Promise<R> {
+  async acquire (acquisitionContext: AcquisitionContext, address: ServerAddress, config?: AcquisitionConfig): Promise<R> {
     const key = address.asKey()
 
     // We're out of resources and will try to acquire later on when an existing resource is released.
     const allRequests = this._acquireRequests
     const requests = allRequests[key]
+    const elapsedTime = (acquisitionContext.startTime != null && acquisitionContext.startTime !== 0) ? new Date().getTime() - acquisitionContext.startTime : 0
     if (requests == null) {
       allRequests[key] = []
     }
@@ -143,7 +145,7 @@ class Pool<R extends unknown = unknown> {
             )
           )
         }
-      }, this._acquisitionTimeout)
+      }, this._acquisitionTimeout - elapsedTime)
 
       if (typeof timeoutId === 'object') {
         // eslint-disable-next-line

packages/neo4j-driver-deno/lib/bolt-connection/connection-provider/connection-provider-routing.js

@@ -198,10 +198,6 @@ const skippedTests = [
   skip(
     'Needs trying all DNS resolved addresses for hosts in the routing table',
     ifEndsWith('.test_ipv6_read').and(startsWith('stub.routing.test_routing_'))
-  ),
-  skip(
-    'Driver has separate timeouts for every connection it attempts to open. This will change in 6.0',
-    ifEquals('stub.homedb.test_homedb.TestHomeDbMixedCluster.test_connection_acquisition_timeout_during_fallback')
   )
 ]