Connection timeout

Added ability to enforce max connection timeout. It is applied to new
connections and makes them terminate connection attempt after
configured number of milliseconds. Feature is needed because connect
part might take minutes in some environments.

NodeJS channel uses `net.Socket` when non-encrypted and it's subclass
`tls.TLSSocket` when encrypted. Connection timeout is implemented using
`socket.setTimeout()` function which triggers a callback after
configured millis of inactivity. We first initiate a connection attempt
and them set the timeout. If connection establishes before the timeout
then later is canceled. Otherwise `timeout` event is fired and socket
is destroyed.

WebSocket channel used in browser does not have built-in timeout
functionality. That is why we simply use `setTimeout()` that closes
WebSocket if it does not connect in time. Successful connection
cancels the timeout.

Feature is turned of by default and can be configured using
`connectionTimeout` property in the config. Example:

```
const driver = neo4j.driver(
                    'bolt://localhost',
                    neo4j.auth.basic('neo4j', 'pwd'),
                    {connectionTimeout: 5000 /* 5 seconds */ });
```

Author: lutovich

src/v1/index.js

@@ -116,7 +116,12 @@ const USER_AGENT = "neo4j-javascript/" + VERSION;
  *       // {@link Session#readTransaction()} and {@link Session#writeTransaction()} functions. These functions
  *       // will retry the given unit of work on `ServiceUnavailable`, `SessionExpired` and transient errors with
  *       // exponential backoff using initial delay of 1 second. Default value is 30000 which is 30 seconds.
- *       maxTransactionRetryTime: 30000,
+ *       maxTransactionRetryTime: 30000, // 30 seconds
+ *
+ *       // Specify socket connection timeout in milliseconds. Non-numeric, negative and zero values are treated as an
+ *       // infinite timeout. Connection will be then bound by the timeout configured on the operating system level.
+ *       // Timeout value should be numeric and greater or equal to zero.
+ *       connectionTimeout: 5000, // 5 seconds
  *     }
  *
  * @param {string} url The URL for the Neo4j database, for instance "bolt://localhost"

src/v1/internal/ch-config.js

@@ -20,38 +20,49 @@
 import hasFeature from './features';
 import {SERVICE_UNAVAILABLE} from '../error';
 
+const DEFAULT_CONNECTION_TIMEOUT_MILLIS = 0; // turned off by default
+
 export default class ChannelConfig {
 
   constructor(host, port, driverConfig, connectionErrorCode) {
     this.host = host;
     this.port = port;
-    this.encrypted = ChannelConfig._extractEncrypted(driverConfig);
-    this.trust = ChannelConfig._extractTrust(driverConfig);
-    this.trustedCertificates = ChannelConfig._extractTrustedCertificates(driverConfig);
-    this.knownHostsPath = ChannelConfig._extractKnownHostsPath(driverConfig);
+    this.encrypted = extractEncrypted(driverConfig);
+    this.trust = extractTrust(driverConfig);
+    this.trustedCertificates = extractTrustedCertificates(driverConfig);
+    this.knownHostsPath = extractKnownHostsPath(driverConfig);
     this.connectionErrorCode = connectionErrorCode || SERVICE_UNAVAILABLE;
+    this.connectionTimeout = extractConnectionTimeout(driverConfig);
   }
+}
 
-  static _extractEncrypted(driverConfig) {
-    // check if encryption was configured by the user, use explicit null check because we permit boolean value
-    const encryptionConfigured = driverConfig.encrypted == null;
-    // default to using encryption if trust-all-certificates is available
-    return encryptionConfigured ? hasFeature('trust_all_certificates') : driverConfig.encrypted;
-  }
+function extractEncrypted(driverConfig) {
+  // check if encryption was configured by the user, use explicit null check because we permit boolean value
+  const encryptionConfigured = driverConfig.encrypted == null;
+  // default to using encryption if trust-all-certificates is available
+  return encryptionConfigured ? hasFeature('trust_all_certificates') : driverConfig.encrypted;
+}
 
-  static _extractTrust(driverConfig) {
-    if (driverConfig.trust) {
-      return driverConfig.trust;
-    }
-    // default to using TRUST_ALL_CERTIFICATES if it is available
-    return hasFeature('trust_all_certificates') ? 'TRUST_ALL_CERTIFICATES' : 'TRUST_CUSTOM_CA_SIGNED_CERTIFICATES';
+function extractTrust(driverConfig) {
+  if (driverConfig.trust) {
+    return driverConfig.trust;
   }
+  // default to using TRUST_ALL_CERTIFICATES if it is available
+  return hasFeature('trust_all_certificates') ? 'TRUST_ALL_CERTIFICATES' : 'TRUST_CUSTOM_CA_SIGNED_CERTIFICATES';
+}
 
-  static _extractTrustedCertificates(driverConfig) {
-    return driverConfig.trustedCertificates || [];
-  }
+function extractTrustedCertificates(driverConfig) {
+  return driverConfig.trustedCertificates || [];
+}
+
+function extractKnownHostsPath(driverConfig) {
+  return driverConfig.knownHosts || null;
+}
 
-  static _extractKnownHostsPath(driverConfig) {
-    return driverConfig.knownHosts || null;
+function extractConnectionTimeout(driverConfig) {
+  const configuredTimeout = parseInt(driverConfig.connectionTimeout, 10);
+  if (!configuredTimeout || configuredTimeout < 0) {
+    return DEFAULT_CONNECTION_TIMEOUT_MILLIS;
   }
-};
+  return configuredTimeout;
+}

src/v1/internal/ch-node.js

@@ -320,6 +320,8 @@ class NodeChannel {
         self.write( pending[i] );
       }
     }, this._handleConnectionError);
+
+    this._setupConnectionTimeout(config, this._conn);
   }
 
   _handleConnectionError( err ) {
@@ -337,6 +339,30 @@ class NodeChannel {
       }
   }
 
+  /**
+   * Setup connection timeout on the socket, if configured.
+   * @param {ChannelConfig} config - configuration of this channel.
+   * @param {object} socket - `net.Socket` or `tls.TLSSocket` object.
+   * @private
+   */
+  _setupConnectionTimeout(config, socket) {
+    const timeout = config.connectionTimeout;
+    if (timeout) {
+      socket.on('connect', () => {
+        // connected - clear connection timeout
+        socket.setTimeout(0);
+      });
+
+      socket.on('timeout', () => {
+        // timeout fired - not connected within configured time. cancel timeout and destroy socket
+        socket.setTimeout(0);
+        socket.destroy(newError(`Failed to establish connection in ${timeout}ms`, config.connectionErrorCode));
+      });
+
+      socket.setTimeout(timeout);
+    }
+  }
+
   isEncrypted() {
     return this._encrypted;
   }

src/v1/internal/ch-websocket.js

@@ -37,9 +37,7 @@ class WebSocketChannel {
     this._pending = [];
     this._error = null;
     this._handleConnectionError = this._handleConnectionError.bind(this);
-    this._connectionErrorCode = config.connectionErrorCode;
-
-    this._encrypted = config.encrypted;
+    this._config = config;
 
     let scheme = "ws";
     //Allow boolean for backwards compatibility
@@ -67,6 +65,9 @@ class WebSocketChannel {
         }
     };
     this._ws.onopen = function() {
+      // Connected! Cancel connection timeout
+      clearTimeout(self._connectionTimeoutId);
+
       // Drain all pending messages
       let pending = self._pending;
       self._pending = null;
@@ -76,15 +77,28 @@ class WebSocketChannel {
     };
     this._ws.onmessage = (event) => {
       if( self.onmessage ) {
-        var b = new HeapBuffer( event.data );
+        const b = new HeapBuffer(event.data);
         self.onmessage( b );
       }
     };
 
     this._ws.onerror = this._handleConnectionError;
+
+    this._connectionTimeoutFired = false;
+    this._connectionTimeoutId = this._setupConnectionTimeout(config);
   }
 
   _handleConnectionError() {
+    if (this._connectionTimeoutFired) {
+      // timeout fired - not connected within configured time
+      this._error = newError(`Failed to establish connection in ${this._config.connectionTimeout}ms`, this._config.connectionErrorCode);
+
+      if (this.onerror) {
+        this.onerror(this._error);
+      }
+      return;
+    }
+
     // onerror triggers on websocket close as well.. don't get me started.
     if( this._open ) {
       // http://stackoverflow.com/questions/25779831/how-to-catch-websocket-connection-to-ws-xxxnn-failed-connection-closed-be
@@ -94,15 +108,15 @@ class WebSocketChannel {
         "the root cause of the failure. Common reasons include the database being " +
         "unavailable, using the wrong connection URL or temporary network problems. " +
         "If you have enabled encryption, ensure your browser is configured to trust the " +
-        "certificate Neo4j is configured to use. WebSocket `readyState` is: " + this._ws.readyState, this._connectionErrorCode );
+        'certificate Neo4j is configured to use. WebSocket `readyState` is: ' + this._ws.readyState, this._config.connectionErrorCode);
       if (this.onerror) {
         this.onerror(this._error);
       }
     }
   }
 
   isEncrypted() {
-    return this._encrypted;
+    return this._config.encrypted;
   }
 
   /**
@@ -130,6 +144,26 @@ class WebSocketChannel {
     this._ws.close();
     this._ws.onclose = cb;
   }
+
+  /**
+   * Set connection timeout on the given WebSocket, if configured.
+   * @return {number} the timeout id or null.
+   * @private
+   */
+  _setupConnectionTimeout() {
+    const timeout = this._config.connectionTimeout;
+    if (timeout) {
+      const webSocket = this._ws;
+
+      return setTimeout(() => {
+        if (webSocket.readyState !== WebSocket.OPEN) {
+          this._connectionTimeoutFired = true;
+          webSocket.close();
+        }
+      }, timeout);
+    }
+    return null;
+  }
 }
 
 let available = typeof WebSocket !== 'undefined';

test/internal/connector.test.js

@@ -209,6 +209,14 @@ describe('connector', () => {
     });
   });
 
+  it('should respect connection timeout', done => {
+    testConnectionTimeout(false, done);
+  });
+
+  it('should respect encrypted connection timeout', done => {
+    testConnectionTimeout(true, done);
+  });
+
   function packedHandshakeMessage() {
     const result = alloc(4);
     result.putInt32(0, 1);
@@ -244,4 +252,29 @@ describe('connector', () => {
     };
   }
 
+  function testConnectionTimeout(encrypted, done) {
+    const boltUri = 'bolt://10.0.0.0'; // use non-routable IP address which never responds
+    connection = connect(boltUri, {encrypted: encrypted, connectionTimeout: 1000}, 'TestErrorCode');
+
+    connection.initialize('mydriver/0.0.0', basicAuthToken(), {
+      onNext: record => {
+        done.fail('Should not receive records: ' + record);
+      },
+      onCompleted: () => {
+        done.fail('Should not be able to INIT');
+      },
+      onError: error => {
+        expect(error.code).toEqual('TestErrorCode');
+
+        // in some environments non-routable address results in immediate 'connection refused' error and connect
+        // timeout is not fired; skip message assertion for such cases, it is important for connect attempt to not hang
+        if (error.message.indexOf('Failed to establish connection') === 0) {
+          expect(error.message).toEqual('Failed to establish connection in 1000ms');
+        }
+
+        done();
+      }
+    });
+  }
+
 });

test/v1/session.test.js

@@ -965,6 +965,14 @@ describe('session', () => {
     });
   });
 
+  it('should respect connection timeout', done => {
+    testConnectionTimeout(false, done);
+  });
+
+  it('should respect encrypted connection timeout', done => {
+    testConnectionTimeout(true, done);
+  });
+
   function serverIs31OrLater(done) {
     // lazy way of checking the version number
     // if server has been set we know it is at least 3.1
@@ -1043,4 +1051,25 @@ describe('session', () => {
     });
   }
 
+  function testConnectionTimeout(encrypted, done) {
+    const boltUri = 'bolt://10.0.0.0'; // use non-routable IP address which never responds
+    const config = {encrypted: encrypted, connectionTimeout: 1000};
+
+    const localDriver = neo4j.driver(boltUri, sharedNeo4j.authToken, config);
+    const session = localDriver.session();
+    session.run('RETURN 1').then(() => {
+      done.fail('Query did not fail');
+    }).catch(error => {
+      expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
+
+      // in some environments non-routable address results in immediate 'connection refused' error and connect
+      // timeout is not fired; skip message assertion for such cases, it is important for connect attempt to not hang
+      if (error.message.indexOf('Failed to establish connection') === 0) {
+        expect(error.message).toEqual('Failed to establish connection in 1000ms');
+      }
+
+      done();
+    });
+  }
+
 });

test/v1/transaction.test.js

@@ -29,7 +29,7 @@ describe('transaction', () => {
   beforeEach(done => {
     // make jasmine timeout high enough to test unreachable bookmarks
     originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;
-    jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000;
+    jasmine.DEFAULT_TIMEOUT_INTERVAL = 40000;
 
     driver = neo4j.driver("bolt://localhost", sharedNeo4j.authToken);
     driver.onCompleted = meta => {
@@ -500,6 +500,14 @@ describe('transaction', () => {
     });
   });
 
+  it('should respect socket connection timeout', done => {
+    testConnectionTimeout(false, done);
+  });
+
+  it('should respect TLS socket connection timeout', done => {
+    testConnectionTimeout(true, done);
+  });
+
   function expectSyntaxError(error) {
     expect(error.code).toBe('Neo.ClientError.Statement.SyntaxError');
   }
@@ -519,4 +527,32 @@ describe('transaction', () => {
     }
     return false;
   }
+
+  function testConnectionTimeout(encrypted, done) {
+    const boltUri = 'bolt://10.0.0.0'; // use non-routable IP address which never responds
+    const config = {encrypted: encrypted, connectionTimeout: 1000};
+
+    const localDriver = neo4j.driver(boltUri, sharedNeo4j.authToken, config);
+    const session = localDriver.session();
+    const tx = session.beginTransaction();
+    tx.run('RETURN 1').then(() => {
+      tx.rollback();
+      session.close();
+      done.fail('Query did not fail');
+    }).catch(error => {
+      tx.rollback();
+      session.close();
+
+      expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
+
+      // in some environments non-routable address results in immediate 'connection refused' error and connect
+      // timeout is not fired; skip message assertion for such cases, it is important for connect attempt to not hang
+      if (error.message.indexOf('Failed to establish connection') === 0) {
+        expect(error.message).toEqual('Failed to establish connection in 1000ms');
+      }
+
+      done();
+    });
+  }
+
 });