Skip to content

Connection timeout #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
7 changes: 6 additions & 1 deletion src/v1/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
55 changes: 33 additions & 22 deletions src/v1/internal/ch-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
26 changes: 26 additions & 0 deletions src/v1/internal/ch-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ class NodeChannel {
self.write( pending[i] );
}
}, this._handleConnectionError);

this._setupConnectionTimeout(config, this._conn);
}

_handleConnectionError( err ) {
Expand All @@ -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;
}
Expand Down
46 changes: 40 additions & 6 deletions src/v1/internal/ch-websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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';
Expand Down
33 changes: 33 additions & 0 deletions test/internal/connector.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
});
}

});
29 changes: 29 additions & 0 deletions test/v1/session.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
});
}

});
38 changes: 37 additions & 1 deletion test/v1/transaction.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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');
}
Expand All @@ -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();
});
}

});