Skip to content

Commit 548cbec

Browse files
authored
Merge pull request #286 from lutovich/1.4-connection-timeout
Connection timeout
2 parents 82af0d8 + 6c1f5d1 commit 548cbec

File tree

8 files changed

+205
-31
lines changed

8 files changed

+205
-31
lines changed

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "neo4j-driver",
3-
"version": "1.2.0-dev",
3+
"version": "1.4.1-dev",
44
"description": "Connect to Neo4j 3.1.0 and up from JavaScript",
55
"author": "Neo Technology Inc.",
66
"license": "Apache-2.0",

src/v1/index.js

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

src/v1/internal/ch-config.js

+33-22
Original file line numberDiff line numberDiff line change
@@ -20,38 +20,49 @@
2020
import hasFeature from './features';
2121
import {SERVICE_UNAVAILABLE} from '../error';
2222

23+
const DEFAULT_CONNECTION_TIMEOUT_MILLIS = 0; // turned off by default
24+
2325
export default class ChannelConfig {
2426

2527
constructor(host, port, driverConfig, connectionErrorCode) {
2628
this.host = host;
2729
this.port = port;
28-
this.encrypted = ChannelConfig._extractEncrypted(driverConfig);
29-
this.trust = ChannelConfig._extractTrust(driverConfig);
30-
this.trustedCertificates = ChannelConfig._extractTrustedCertificates(driverConfig);
31-
this.knownHostsPath = ChannelConfig._extractKnownHostsPath(driverConfig);
30+
this.encrypted = extractEncrypted(driverConfig);
31+
this.trust = extractTrust(driverConfig);
32+
this.trustedCertificates = extractTrustedCertificates(driverConfig);
33+
this.knownHostsPath = extractKnownHostsPath(driverConfig);
3234
this.connectionErrorCode = connectionErrorCode || SERVICE_UNAVAILABLE;
35+
this.connectionTimeout = extractConnectionTimeout(driverConfig);
3336
}
37+
}
3438

35-
static _extractEncrypted(driverConfig) {
36-
// check if encryption was configured by the user, use explicit null check because we permit boolean value
37-
const encryptionConfigured = driverConfig.encrypted == null;
38-
// default to using encryption if trust-all-certificates is available
39-
return encryptionConfigured ? hasFeature('trust_all_certificates') : driverConfig.encrypted;
40-
}
39+
function extractEncrypted(driverConfig) {
40+
// check if encryption was configured by the user, use explicit null check because we permit boolean value
41+
const encryptionConfigured = driverConfig.encrypted == null;
42+
// default to using encryption if trust-all-certificates is available
43+
return encryptionConfigured ? hasFeature('trust_all_certificates') : driverConfig.encrypted;
44+
}
4145

42-
static _extractTrust(driverConfig) {
43-
if (driverConfig.trust) {
44-
return driverConfig.trust;
45-
}
46-
// default to using TRUST_ALL_CERTIFICATES if it is available
47-
return hasFeature('trust_all_certificates') ? 'TRUST_ALL_CERTIFICATES' : 'TRUST_CUSTOM_CA_SIGNED_CERTIFICATES';
46+
function extractTrust(driverConfig) {
47+
if (driverConfig.trust) {
48+
return driverConfig.trust;
4849
}
50+
// default to using TRUST_ALL_CERTIFICATES if it is available
51+
return hasFeature('trust_all_certificates') ? 'TRUST_ALL_CERTIFICATES' : 'TRUST_CUSTOM_CA_SIGNED_CERTIFICATES';
52+
}
4953

50-
static _extractTrustedCertificates(driverConfig) {
51-
return driverConfig.trustedCertificates || [];
52-
}
54+
function extractTrustedCertificates(driverConfig) {
55+
return driverConfig.trustedCertificates || [];
56+
}
57+
58+
function extractKnownHostsPath(driverConfig) {
59+
return driverConfig.knownHosts || null;
60+
}
5361

54-
static _extractKnownHostsPath(driverConfig) {
55-
return driverConfig.knownHosts || null;
62+
function extractConnectionTimeout(driverConfig) {
63+
const configuredTimeout = parseInt(driverConfig.connectionTimeout, 10);
64+
if (!configuredTimeout || configuredTimeout < 0) {
65+
return DEFAULT_CONNECTION_TIMEOUT_MILLIS;
5666
}
57-
};
67+
return configuredTimeout;
68+
}

src/v1/internal/ch-node.js

+26
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,8 @@ class NodeChannel {
320320
self.write( pending[i] );
321321
}
322322
}, this._handleConnectionError);
323+
324+
this._setupConnectionTimeout(config, this._conn);
323325
}
324326

325327
_handleConnectionError( err ) {
@@ -337,6 +339,30 @@ class NodeChannel {
337339
}
338340
}
339341

342+
/**
343+
* Setup connection timeout on the socket, if configured.
344+
* @param {ChannelConfig} config - configuration of this channel.
345+
* @param {object} socket - `net.Socket` or `tls.TLSSocket` object.
346+
* @private
347+
*/
348+
_setupConnectionTimeout(config, socket) {
349+
const timeout = config.connectionTimeout;
350+
if (timeout) {
351+
socket.on('connect', () => {
352+
// connected - clear connection timeout
353+
socket.setTimeout(0);
354+
});
355+
356+
socket.on('timeout', () => {
357+
// timeout fired - not connected within configured time. cancel timeout and destroy socket
358+
socket.setTimeout(0);
359+
socket.destroy(newError(`Failed to establish connection in ${timeout}ms`, config.connectionErrorCode));
360+
});
361+
362+
socket.setTimeout(timeout);
363+
}
364+
}
365+
340366
isEncrypted() {
341367
return this._encrypted;
342368
}

src/v1/internal/ch-websocket.js

+40-6
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@ class WebSocketChannel {
3737
this._pending = [];
3838
this._error = null;
3939
this._handleConnectionError = this._handleConnectionError.bind(this);
40-
this._connectionErrorCode = config.connectionErrorCode;
41-
42-
this._encrypted = config.encrypted;
40+
this._config = config;
4341

4442
let scheme = "ws";
4543
//Allow boolean for backwards compatibility
@@ -67,6 +65,9 @@ class WebSocketChannel {
6765
}
6866
};
6967
this._ws.onopen = function() {
68+
// Connected! Cancel connection timeout
69+
clearTimeout(self._connectionTimeoutId);
70+
7071
// Drain all pending messages
7172
let pending = self._pending;
7273
self._pending = null;
@@ -76,15 +77,28 @@ class WebSocketChannel {
7677
};
7778
this._ws.onmessage = (event) => {
7879
if( self.onmessage ) {
79-
var b = new HeapBuffer( event.data );
80+
const b = new HeapBuffer(event.data);
8081
self.onmessage( b );
8182
}
8283
};
8384

8485
this._ws.onerror = this._handleConnectionError;
86+
87+
this._connectionTimeoutFired = false;
88+
this._connectionTimeoutId = this._setupConnectionTimeout(config);
8589
}
8690

8791
_handleConnectionError() {
92+
if (this._connectionTimeoutFired) {
93+
// timeout fired - not connected within configured time
94+
this._error = newError(`Failed to establish connection in ${this._config.connectionTimeout}ms`, this._config.connectionErrorCode);
95+
96+
if (this.onerror) {
97+
this.onerror(this._error);
98+
}
99+
return;
100+
}
101+
88102
// onerror triggers on websocket close as well.. don't get me started.
89103
if( this._open ) {
90104
// http://stackoverflow.com/questions/25779831/how-to-catch-websocket-connection-to-ws-xxxnn-failed-connection-closed-be
@@ -94,15 +108,15 @@ class WebSocketChannel {
94108
"the root cause of the failure. Common reasons include the database being " +
95109
"unavailable, using the wrong connection URL or temporary network problems. " +
96110
"If you have enabled encryption, ensure your browser is configured to trust the " +
97-
"certificate Neo4j is configured to use. WebSocket `readyState` is: " + this._ws.readyState, this._connectionErrorCode );
111+
'certificate Neo4j is configured to use. WebSocket `readyState` is: ' + this._ws.readyState, this._config.connectionErrorCode);
98112
if (this.onerror) {
99113
this.onerror(this._error);
100114
}
101115
}
102116
}
103117

104118
isEncrypted() {
105-
return this._encrypted;
119+
return this._config.encrypted;
106120
}
107121

108122
/**
@@ -130,6 +144,26 @@ class WebSocketChannel {
130144
this._ws.close();
131145
this._ws.onclose = cb;
132146
}
147+
148+
/**
149+
* Set connection timeout on the given WebSocket, if configured.
150+
* @return {number} the timeout id or null.
151+
* @private
152+
*/
153+
_setupConnectionTimeout() {
154+
const timeout = this._config.connectionTimeout;
155+
if (timeout) {
156+
const webSocket = this._ws;
157+
158+
return setTimeout(() => {
159+
if (webSocket.readyState !== WebSocket.OPEN) {
160+
this._connectionTimeoutFired = true;
161+
webSocket.close();
162+
}
163+
}, timeout);
164+
}
165+
return null;
166+
}
133167
}
134168

135169
let available = typeof WebSocket !== 'undefined';

test/internal/connector.test.js

+33
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,14 @@ describe('connector', () => {
209209
});
210210
});
211211

212+
it('should respect connection timeout', done => {
213+
testConnectionTimeout(false, done);
214+
});
215+
216+
it('should respect encrypted connection timeout', done => {
217+
testConnectionTimeout(true, done);
218+
});
219+
212220
function packedHandshakeMessage() {
213221
const result = alloc(4);
214222
result.putInt32(0, 1);
@@ -244,4 +252,29 @@ describe('connector', () => {
244252
};
245253
}
246254

255+
function testConnectionTimeout(encrypted, done) {
256+
const boltUri = 'bolt://10.0.0.0'; // use non-routable IP address which never responds
257+
connection = connect(boltUri, {encrypted: encrypted, connectionTimeout: 1000}, 'TestErrorCode');
258+
259+
connection.initialize('mydriver/0.0.0', basicAuthToken(), {
260+
onNext: record => {
261+
done.fail('Should not receive records: ' + record);
262+
},
263+
onCompleted: () => {
264+
done.fail('Should not be able to INIT');
265+
},
266+
onError: error => {
267+
expect(error.code).toEqual('TestErrorCode');
268+
269+
// in some environments non-routable address results in immediate 'connection refused' error and connect
270+
// timeout is not fired; skip message assertion for such cases, it is important for connect attempt to not hang
271+
if (error.message.indexOf('Failed to establish connection') === 0) {
272+
expect(error.message).toEqual('Failed to establish connection in 1000ms');
273+
}
274+
275+
done();
276+
}
277+
});
278+
}
279+
247280
});

test/v1/session.test.js

+29
Original file line numberDiff line numberDiff line change
@@ -965,6 +965,14 @@ describe('session', () => {
965965
});
966966
});
967967

968+
it('should respect connection timeout', done => {
969+
testConnectionTimeout(false, done);
970+
});
971+
972+
it('should respect encrypted connection timeout', done => {
973+
testConnectionTimeout(true, done);
974+
});
975+
968976
function serverIs31OrLater(done) {
969977
// lazy way of checking the version number
970978
// if server has been set we know it is at least 3.1
@@ -1043,4 +1051,25 @@ describe('session', () => {
10431051
});
10441052
}
10451053

1054+
function testConnectionTimeout(encrypted, done) {
1055+
const boltUri = 'bolt://10.0.0.0'; // use non-routable IP address which never responds
1056+
const config = {encrypted: encrypted, connectionTimeout: 1000};
1057+
1058+
const localDriver = neo4j.driver(boltUri, sharedNeo4j.authToken, config);
1059+
const session = localDriver.session();
1060+
session.run('RETURN 1').then(() => {
1061+
done.fail('Query did not fail');
1062+
}).catch(error => {
1063+
expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
1064+
1065+
// in some environments non-routable address results in immediate 'connection refused' error and connect
1066+
// timeout is not fired; skip message assertion for such cases, it is important for connect attempt to not hang
1067+
if (error.message.indexOf('Failed to establish connection') === 0) {
1068+
expect(error.message).toEqual('Failed to establish connection in 1000ms');
1069+
}
1070+
1071+
done();
1072+
});
1073+
}
1074+
10461075
});

test/v1/transaction.test.js

+37-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('transaction', () => {
2929
beforeEach(done => {
3030
// make jasmine timeout high enough to test unreachable bookmarks
3131
originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;
32-
jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000;
32+
jasmine.DEFAULT_TIMEOUT_INTERVAL = 40000;
3333

3434
driver = neo4j.driver("bolt://localhost", sharedNeo4j.authToken);
3535
driver.onCompleted = meta => {
@@ -500,6 +500,14 @@ describe('transaction', () => {
500500
});
501501
});
502502

503+
it('should respect socket connection timeout', done => {
504+
testConnectionTimeout(false, done);
505+
});
506+
507+
it('should respect TLS socket connection timeout', done => {
508+
testConnectionTimeout(true, done);
509+
});
510+
503511
function expectSyntaxError(error) {
504512
expect(error.code).toBe('Neo.ClientError.Statement.SyntaxError');
505513
}
@@ -519,4 +527,32 @@ describe('transaction', () => {
519527
}
520528
return false;
521529
}
530+
531+
function testConnectionTimeout(encrypted, done) {
532+
const boltUri = 'bolt://10.0.0.0'; // use non-routable IP address which never responds
533+
const config = {encrypted: encrypted, connectionTimeout: 1000};
534+
535+
const localDriver = neo4j.driver(boltUri, sharedNeo4j.authToken, config);
536+
const session = localDriver.session();
537+
const tx = session.beginTransaction();
538+
tx.run('RETURN 1').then(() => {
539+
tx.rollback();
540+
session.close();
541+
done.fail('Query did not fail');
542+
}).catch(error => {
543+
tx.rollback();
544+
session.close();
545+
546+
expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
547+
548+
// in some environments non-routable address results in immediate 'connection refused' error and connect
549+
// timeout is not fired; skip message assertion for such cases, it is important for connect attempt to not hang
550+
if (error.message.indexOf('Failed to establish connection') === 0) {
551+
expect(error.message).toEqual('Failed to establish connection in 1000ms');
552+
}
553+
554+
done();
555+
});
556+
}
557+
522558
});

0 commit comments

Comments
 (0)