From b3106a45c4cc1f68d85c4d0049db140d43796db4 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 17 Dec 2016 17:38:27 +0100 Subject: [PATCH] Remove deprecated max_attempts --- README.md | 1 - index.js | 37 --------------------- test/connection.spec.js | 71 +---------------------------------------- test/multi.spec.js | 4 +-- test/node_redis.spec.js | 8 +++-- 5 files changed, 8 insertions(+), 113 deletions(-) diff --git a/README.md b/README.md index 71cf7e15ae..07b0727e26 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,6 @@ __Tip:__ If the Redis server runs on the same machine as the client consider usi | enable_offline_queue | true | By default, if there is no active connection to the Redis server, commands are added to a queue and are executed once the connection has been established. Setting `enable_offline_queue` to `false` will disable this feature and the callback will be executed immediately with an error, or an error will be emitted if no callback is specified. | | retry_max_delay | null | __Deprecated__ _Please use `retry_strategy` instead._ 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 | 3600000 | __Deprecated__ _Please use `retry_strategy` instead._ Setting `connect_timeout` limits the total time for the client to connect and reconnect. The value is provided in milliseconds and is counted from the moment a new client is created or from the time the 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 has elapsed. | -| max_attempts | 0 | __Deprecated__ _Please use `retry_strategy` instead._ By default, a client will try reconnecting until connected. Setting `max_attempts` limits total amount of connection attempts. Setting this to 1 will prevent any reconnect attempt. | | retry_unfulfilled_commands | false | If set to `true`, all commands that were unfulfilled while the connection is lost will be retried after the connection has been reestablished. Use this with caution if you use state altering commands (e.g. `incr`). This is especially useful if you use blocking commands. | | password | null | If set, client will run Redis auth command on connect. Alias `auth_pass` __Note__ `node_redis` < 2.5 must use `auth_pass` | | db | null | If set, client will run Redis `select` command on connect. | diff --git a/index.js b/index.js index 79b53cbae9..176c5c7f72 100644 --- a/index.js +++ b/index.js @@ -68,11 +68,6 @@ function RedisClient (options, stream) { } // Warn on misusing deprecated functions if (typeof options.retry_strategy === 'function') { - if ('max_attempts' in options) { - self.warn('WARNING: You activated the retry_strategy and max_attempts at the same time. This is not possible and max_attempts will be ignored.'); - // Do not print deprecation warnings twice - delete options.max_attempts; - } if ('retry_max_delay' in options) { self.warn('WARNING: You activated the retry_strategy and retry_max_delay at the same time. This is not possible and retry_max_delay will be ignored.'); // Do not print deprecation warnings twice @@ -102,14 +97,6 @@ function RedisClient (options, stream) { this.handle_reply = handle_detect_buffers_reply; } this.should_buffer = false; - this.max_attempts = options.max_attempts | 0; - if ('max_attempts' in options) { - self.warn( - 'max_attempts is deprecated and will be removed in v.3.0.0.\n' + - 'To reduce the number of options and to improve the reconnection handling please use the new `retry_strategy` option instead.\n' + - 'This replaces the max_attempts and retry_max_delay option.' - ); - } 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.pipeline_queue = new Queue(); // Holds all pipelined commands @@ -605,30 +592,6 @@ RedisClient.prototype.connection_gone = function (why, error) { } } - if (this.max_attempts !== 0 && this.attempts >= this.max_attempts || this.retry_totaltime >= this.connect_timeout) { - var message = 'Redis connection in broken state: '; - if (this.retry_totaltime >= this.connect_timeout) { - message += 'connection timeout exceeded.'; - } else { - message += 'maximum connection attempts exceeded.'; - } - - this.flush_and_error({ - message: message, - code: 'CONNECTION_BROKEN', - }, { - error: error - }); - var err = new Error(message); - err.code = 'CONNECTION_BROKEN'; - if (error) { - err.origin = error; - } - this.emit('error', err); - this.end(false); - return; - } - // Retry commands after a reconnect instead of throwing an error. Use this with caution if (this.options.retry_unfulfilled_commands) { this.offline_queue.unshift.apply(this.offline_queue, this.command_queue.toArray()); diff --git a/test/connection.spec.js b/test/connection.spec.js index f92dca1949..1659eb9073 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -152,73 +152,6 @@ describe('connection tests', function () { describe('using ' + parser + ' and ' + ip, function () { describe('on lost connection', function () { - it('emit an error after max retry attempts and do not try to reconnect afterwards', function (done) { - var maxAttempts = 3; - var options = { - parser: parser, - maxAttempts: maxAttempts - }; - client = redis.createClient(options); - assert.strictEqual(client.retryBackoff, 1.7); - assert.strictEqual(client.retryDelay, 200); - assert.strictEqual(Object.keys(options).length, 2); - var calls = 0; - - client.once('ready', function () { - helper.killConnection(client); - }); - - client.on('reconnecting', function (params) { - calls++; - }); - - client.on('error', function (err) { - if (/Redis connection in broken state: maximum connection attempts.*?exceeded./.test(err.message)) { - process.nextTick(function () { // End is called after the error got emitted - assert.strictEqual(calls, maxAttempts - 1); - assert.strictEqual(client.emitted_end, true); - assert.strictEqual(client.connected, false); - assert.strictEqual(client.ready, false); - assert.strictEqual(client.closing, true); - done(); - }); - } - }); - }); - - it('emit an error after max retry timeout and do not try to reconnect afterwards', function (done) { - // TODO: Investigate why this test fails with windows. Reconnect is only triggered once - if (process.platform === 'win32') this.skip(); - - var connect_timeout = 600; // in ms - client = redis.createClient({ - parser: parser, - connect_timeout: connect_timeout - }); - var time = 0; - - client.once('ready', function () { - helper.killConnection(client); - }); - - client.on('reconnecting', function (params) { - time += params.delay; - }); - - client.on('error', function (err) { - if (/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)) { - process.nextTick(function () { // End is called after the error got emitted - assert.strictEqual(client.emitted_end, true); - assert.strictEqual(client.connected, false); - assert.strictEqual(client.ready, false); - assert.strictEqual(client.closing, true); - assert.strictEqual(client.retry_totaltime, connect_timeout); - assert.strictEqual(time, connect_timeout); - done(); - }); - } - }); - }); it('end connection while retry is still ongoing', function (done) { var connect_timeout = 1000; // in ms @@ -243,7 +176,7 @@ describe('connection tests', function () { host: 'somewhere', port: 6379, family: ip, - max_attempts: 1 + retryStrategy: function () {} }; client = redis.createClient(options); assert.strictEqual(client.connection_options.family, ip === 'IPv6' ? 6 : 4); @@ -289,14 +222,12 @@ describe('connection tests', function () { } return Math.min(options.attempt * 25, 200); }, - maxAttempts: 5, retryMaxDelay: 123, port: 9999 }); process.nextTick(function () { assert.strictEqual( text, - 'node_redis: WARNING: You activated the retry_strategy and max_attempts at the same time. This is not possible and max_attempts will be ignored.\n' + 'node_redis: WARNING: You activated the retry_strategy and retry_max_delay at the same time. This is not possible and retry_max_delay will be ignored.\n' ); unhookIntercept(); diff --git a/test/multi.spec.js b/test/multi.spec.js index e654375dc8..6e3f52c8f0 100644 --- a/test/multi.spec.js +++ b/test/multi.spec.js @@ -219,7 +219,7 @@ describe("The 'multi' method", function () { client = redis.createClient({ host: 'somewhere', port: 6379, - max_attempts: 1 + retryStrategy: function () {} }); client.on('error', function (err) { @@ -239,7 +239,7 @@ describe("The 'multi' method", function () { client = redis.createClient({ host: 'somewhere', port: 6379, - max_attempts: 1 + retryStrategy: function () {} }); client.on('error', function (err) { diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index 1ba3dd946f..2ffcd6a740 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -843,7 +843,6 @@ describe('The node_redis client', function () { it('does not return an error and enqueues operation', function (done) { client = redis.createClient(9999, null, { - max_attempts: 0, parser: parser }); var finished = false; @@ -867,7 +866,11 @@ describe('The node_redis client', function () { it('enqueues operation and keep the queue while trying to reconnect', function (done) { client = redis.createClient(9999, null, { - max_attempts: 4, + retryStrategy: function (options) { + if (options.attempt < 4) { + return 200; + } + }, parser: parser }); var i = 0; @@ -971,7 +974,6 @@ describe('The node_redis client', function () { it('emit an error and does not enqueues operation', function (done) { client = redis.createClient(9999, null, { parser: parser, - max_attempts: 0, enable_offline_queue: false }); var end = helper.callFuncAfter(done, 3);