diff --git a/README.md b/README.md index 597f817ebf..99e5ed63c5 100644 --- a/README.md +++ b/README.md @@ -200,9 +200,9 @@ once the connection has been established. Setting `enable_offline_queue` to with an error, or an error will be emitted if no callback is specified. * `retry_max_delay`: *null*; By default every time the client tries to connect and fails the reconnection delay almost doubles. This delay normally grows infinitely, but setting `retry_max_delay` limits it to the maximum value, provided in milliseconds. -* `connect_timeout`: *86400000*; Setting `connect_timeout` limits total time for client to reconnect. -The value is provided in milliseconds and is counted once the disconnect occurred. The last retry is going to happen exactly at the timeout time. -That way the default is to try reconnecting until 24h passed. +* `connect_timeout`: *3600000*; Setting `connect_timeout` limits total time for client to connect and reconnect. +The value is provided in milliseconds and is counted from the moment on a new client is created / a connection is lost. The last retry is going to happen exactly at the timeout time. +Default is to try connecting until the default system socket timeout has been exceeded and to try reconnecting until 1h passed. * `max_attempts`: *0*; By default client will try reconnecting until connected. Setting `max_attempts` limits total amount of connection tries. Setting this to 1 will prevent any reconnect tries. * `auth_pass`: *null*; If set, client will run redis auth command on connect. diff --git a/changelog.md b/changelog.md index 533088d624..8fdd6c800a 100644 --- a/changelog.md +++ b/changelog.md @@ -13,12 +13,14 @@ Features - Refactored manual backpressure control ([@BridgeAR](https://github.com/BridgeAR)) - Removed the high water mark and low water mark. Such a mechanism should be implemented by a user instead - The `drain` event is from now on only emitted if the stream really had to buffer +- Reduced the default connect_timeout to be one hour instead of 24h ([@BridgeAR](https://github.com/BridgeAR)) Bugfixes - Fixed a js parser error that could result in a timeout ([@BridgeAR](https://github.com/BridgeAR)) - Fixed .multi / .batch used with Node.js 0.10.x not working properly after a reconnect ([@BridgeAR](https://github.com/BridgeAR)) - Fixed fired but not yet returned commands not being rejected after a connection loss ([@BridgeAR](https://github.com/BridgeAR)) +- Fixed connect_timeout not respected if no connection has ever been established ([@gagle](https://github.com/gagle) & [@benjie](https://github.com/benjie)) ## v.2.2.5 - 18 Oct, 2015 diff --git a/index.js b/index.js index d7bd5245f5..e13d93a02f 100644 --- a/index.js +++ b/index.js @@ -85,7 +85,7 @@ function RedisClient(stream, options) { this.max_attempts = options.max_attempts | 0; this.command_queue = new Queue(); // Holds sent commands to de-pipeline them this.offline_queue = new Queue(); // Holds commands issued but not able to be sent - this.connect_timeout = +options.connect_timeout || 86400000; // 24 * 60 * 60 * 1000 ms + this.connect_timeout = +options.connect_timeout || 3600000; // 60 * 60 * 1000 ms this.enable_offline_queue = options.enable_offline_queue === false ? false : true; this.retry_max_delay = +options.retry_max_delay || null; this.initialize_retry_vars(); @@ -108,6 +108,13 @@ util.inherits(RedisClient, events.EventEmitter); RedisClient.prototype.install_stream_listeners = function() { var self = this; + if (this.options.connect_timeout) { + this.stream.setTimeout(this.connect_timeout, function () { + self.retry_totaltime = self.connect_timeout; + self.connection_gone('timeout'); + }); + } + this.stream.on('connect', function () { self.on_connect(); }); diff --git a/test/connection.spec.js b/test/connection.spec.js index 62251fdef4..8a04366ba1 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -125,6 +125,48 @@ describe("connection tests", function () { describe("when not connected", function () { + it("emit an error after the socket timeout exceeded the connect_timeout time", function (done) { + var connect_timeout = 1000; // in ms + var client = redis.createClient({ + parser: parser, + host: '1.2.3.4', + connect_timeout: connect_timeout + }); + assert(client.stream._events.timeout); + assert.strictEqual(client.address, '1.2.3.4:6379'); + var time = Date.now(); + + client.on("reconnecting", function (params) { + throw new Error('No reconnect, since no connection was ever established'); + }); + + client.on('error', function(err) { + assert(/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)); + assert(Date.now() - time < connect_timeout + 50); + done(); + }); + }); + + it("use the system socket timeout if the connect_timeout has not been provided", function () { + var client = redis.createClient({ + parser: parser, + host: '1.2.3.4' + }); + assert(client.stream._events.timeout === undefined); + }); + + it("clears the socket timeout after a connection has been established", function (done) { + var client = redis.createClient({ + parser: parser, + connect_timeout: 1000 + }); + assert.strictEqual(client.stream._idleTimeout, 1000); + client.on('connect', function () { + assert.strictEqual(client.stream._idleTimeout, -1); + done(); + }); + }); + it("connect with host and port provided in the options object", function (done) { client = redis.createClient({ host: 'localhost',