diff --git a/package.json b/package.json index e4e289d0e..9fb7dea1a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "neo4j-driver", - "version": "1.2.0-dev", + "version": "1.4.1-dev", "description": "Connect to Neo4j 3.1.0 and up from JavaScript", "author": "Neo Technology Inc.", "license": "Apache-2.0", diff --git a/src/v1/index.js b/src/v1/index.js index 948259443..4fd9a6977 100644 --- a/src/v1/index.js +++ b/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" diff --git a/src/v1/internal/ch-config.js b/src/v1/internal/ch-config.js index faf6ba10a..80e07b6e8 100644 --- a/src/v1/internal/ch-config.js +++ b/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; +} diff --git a/src/v1/internal/ch-node.js b/src/v1/internal/ch-node.js index 4bfa06122..7d4fb3458 100644 --- a/src/v1/internal/ch-node.js +++ b/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; } diff --git a/src/v1/internal/ch-websocket.js b/src/v1/internal/ch-websocket.js index c937fded7..b8437f68e 100644 --- a/src/v1/internal/ch-websocket.js +++ b/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,7 +108,7 @@ 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); } @@ -102,7 +116,7 @@ class WebSocketChannel { } 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'; diff --git a/test/internal/connector.test.js b/test/internal/connector.test.js index ba4889a9f..10e384053 100644 --- a/test/internal/connector.test.js +++ b/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(); + } + }); + } + }); diff --git a/test/v1/session.test.js b/test/v1/session.test.js index daf6a0dc2..b6dc104ad 100644 --- a/test/v1/session.test.js +++ b/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(); + }); + } + }); diff --git a/test/v1/transaction.test.js b/test/v1/transaction.test.js index baa5309b2..8dfe78a39 100644 --- a/test/v1/transaction.test.js +++ b/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(); + }); + } + });