diff --git a/README.md b/README.md index 3a4abf9eb9..afe1c3ac15 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ are passed an object containing `delay` (in ms) and `attempt` (the attempt #) at ### "error" `client` will emit `error` when encountering an error connecting to the Redis server or when any other in node_redis occurs. +If you use a command without callback and encounter a ReplyError it is going to be emitted to the error listener. So please attach the error listener to node_redis. @@ -293,6 +294,51 @@ client.get("foo_rand000000000000", function (err, reply) { `client.end()` without the flush parameter set to true should NOT be used in production! +## Error handling (>= v.2.6) + +All redis errors are returned as `ReplyError`. +All unresolved commands that get rejected due to what ever reason return a `AbortError`. +As subclass of the `AbortError` a `AggregateError` exists. This is emitted in case multiple unresolved commands without callback got rejected in debug_mode. +They are all aggregated and a single error is emitted in that case. + +Example: +```js +var redis = require('./'); +var assert = require('assert'); +var client = redis.createClient(); + +client.on('error', function (err) { + assert(err instanceof Error); + assert(err instanceof redis.AbortError); + assert(err instanceof redis.AggregateError); + assert.strictEqual(err.errors.length, 2); // The set and get got aggregated in here + assert.strictEqual(err.code, 'NR_CLOSED'); +}); +client.set('foo', 123, 'bar', function (err, res) { // To many arguments + assert(err instanceof redis.ReplyError); // => true + assert.strictEqual(err.command, 'SET'); + assert.deepStrictEqual(err.args, ['foo', 123, 'bar']); + + redis.debug_mode = true; + client.set('foo', 'bar'); + client.get('foo'); + process.nextTick(function () { + client.end(true); // Force closing the connection while the command did not yet return + redis.debug_mode = false; + }); +}); + +``` + +Every `ReplyError` contains the `command` name in all-caps and the arguments (`args`). + +If node_redis emits a library error because of another error, the triggering error is added to the returned error as `origin` attribute. + +___Error codes___ + +node_redis returns a `NR_CLOSED` error code if the clients connection dropped. If a command unresolved command got rejected a `UNERCTAIN_STATE` code is returned. +A `CONNECTION_BROKEN` error code is used in case node_redis gives up to reconnect. + ## client.unref() Call `unref()` on the underlying socket connection to the Redis server, allowing the program to exit once no more commands are pending. @@ -537,7 +583,7 @@ Redis. The interface in `node_redis` is to return an individual `Batch` object b The only difference between .batch and .multi is that no transaction is going to be used. Be aware that the errors are - just like in multi statements - in the result. Otherwise both, errors and results could be returned at the same time. -If you fire many commands at once this is going to **boost the execution speed by up to 400%** [sic!] compared to fireing the same commands in a loop without waiting for the result! See the benchmarks for further comparison. Please remember that all commands are kept in memory until they are fired. +If you fire many commands at once this is going to boost the execution speed significantly compared to fireing the same commands in a loop without waiting for the result! See the benchmarks for further comparison. Please remember that all commands are kept in memory until they are fired. ## Monitor mode diff --git a/index.js b/index.js index a94710a67a..a4957bd219 100644 --- a/index.js +++ b/index.js @@ -5,7 +5,7 @@ var tls = require('tls'); var util = require('util'); var utils = require('./lib/utils'); var Queue = require('double-ended-queue'); -var CommandError = require('./lib/customError'); +var errorClasses = require('./lib/customErrors'); var Command = require('./lib/command').Command; var OfflineCommand = require('./lib/command').OfflineCommand; var EventEmitter = require('events'); @@ -189,13 +189,24 @@ function create_parser (self) { self.return_reply(data); }, returnError: function (err) { - self.return_error(err); + // Return a ReplyError to indicate Redis returned an error + self.return_error(new errorClasses.ReplyError(err)); }, returnFatalError: function (err) { // Error out all fired commands. Otherwise they might rely on faulty data. We have to reconnect to get in a working state again - self.flush_and_error(err, ['command_queue']); - self.stream.destroy(); - self.return_error(err); + // Note: the execution order is important. First flush and emit, then create the stream + err = new errorClasses.ReplyError(err); + err.message += '. Please report this.'; + self.ready = false; + self.flush_and_error({ + message: 'Fatal error encountert. Command aborted.', + code: 'NR_FATAL' + }, { + error: err, + queues: ['command_queue'] + }); + self.emit('error', err); + self.create_stream(); }, returnBuffers: self.buffers || self.message_buffers, name: self.options.parser, @@ -240,7 +251,7 @@ RedisClient.prototype.create_stream = function () { this.stream.setTimeout(this.connect_timeout, function () { // Note: This is only tested if a internet connection is established self.retry_totaltime = self.connect_timeout; - self.connection_gone('timeout', new Error('Redis connection gone from timeout event')); + self.connection_gone('timeout'); }); } @@ -270,11 +281,11 @@ RedisClient.prototype.create_stream = function () { }); this.stream.once('close', function (hadError) { - self.connection_gone('close', hadError ? new Error('Stream connection closed with a transmission error') : null); + self.connection_gone('close'); }); this.stream.once('end', function () { - self.connection_gone('end', null); + self.connection_gone('end'); }); this.stream.on('drain', function () { @@ -325,30 +336,46 @@ RedisClient.prototype.warn = function (msg) { }; // Flush provided queues, erroring any items with a callback first -RedisClient.prototype.flush_and_error = function (error, queue_names) { - var callbacks_not_called = []; - queue_names = queue_names || ['offline_queue', 'command_queue']; +RedisClient.prototype.flush_and_error = function (error_attributes, options) { + options = options || {}; + var aggregated_errors = []; + var queue_names = options.queues || ['command_queue', 'offline_queue']; // Flush the command_queue first to keep the order intakt for (var i = 0; i < queue_names.length; i++) { + // If the command was fired it might have been processed so far + if (queue_names[i] === 'command_queue') { + error_attributes.message += ' It might have been processed.'; + } else { // As the command_queue is flushed first, remove this for the offline queue + error_attributes.message = error_attributes.message.replace(' It might have been processed.', ''); + } + // Don't flush everything from the queue for (var command_obj = this[queue_names[i]].shift(); command_obj; command_obj = this[queue_names[i]].shift()) { - var err = new CommandError(error); + var err = new errorClasses.AbortError(error_attributes); err.command = command_obj.command.toUpperCase(); - if (command_obj.args.length) { + if (command_obj.args && command_obj.args.length) { err.args = command_obj.args; } + if (options.error) { + err.origin = options.error; + } if (typeof command_obj.callback === 'function') { command_obj.callback(err); } else { - callbacks_not_called.push(err); + aggregated_errors.push(err); } } - this[queue_names[i]] = new Queue(); } - // Mutate the original error that will be emitted - // This is fine, as we don't manipulate any user errors - if (callbacks_not_called.length !== 0) { - error.errors = callbacks_not_called; + // Currently this would be a breaking change, therefore it's only emitted in debug_mode + if (exports.debug_mode && aggregated_errors.length) { + var error; + if (aggregated_errors.length === 1) { + error = aggregated_errors[0]; + } else { + error_attributes.message = error_attributes.message.replace('It', 'They').replace(/command/i, '$&s'); + error = new errorClasses.AggregateError(error_attributes); + error.errors = aggregated_errors; + } + this.emit('error', error); } - return callbacks_not_called.length === 0; }; RedisClient.prototype.on_error = function (err) { @@ -538,6 +565,7 @@ RedisClient.prototype.connection_gone = function (why, error) { if (this.retry_timer) { return; } + error = error || null; debug('Redis connection is gone from ' + why + ' event.'); this.connected = false; @@ -564,9 +592,12 @@ RedisClient.prototype.connection_gone = function (why, error) { // If this is a requested shutdown, then don't retry if (this.closing) { debug('Connection ended by quit / end command, not retrying.'); - error = new Error('Stream connection ended and running command aborted. It might have been processed.'); - error.code = 'NR_OFFLINE'; - this.flush_and_error(error); + this.flush_and_error({ + message: 'Stream connection ended and command aborted.', + code: 'NR_CLOSED' + }, { + error: error + }); return; } @@ -586,31 +617,39 @@ RedisClient.prototype.connection_gone = function (why, error) { if (typeof this.retry_delay !== 'number') { // Pass individual error through if (this.retry_delay instanceof Error) { - error = new CommandError(this.retry_delay); - } - // Attention: there might be the case where there's no error! - if (!error) { - error = new Error('Stream connection ended and running command aborted. It might have been processed.'); - error.code = 'NR_OFFLINE'; - } - // Only emit an error in case that a running command had no callback - if (!this.flush_and_error(error)) { - error.message = 'Stream connection ended and all running commands aborted. They might have been processed.'; - this.emit('error', error); + error = this.retry_delay; } + this.flush_and_error({ + message: 'Stream connection ended and command aborted.', + code: 'NR_CLOSED' + }, { + error: error + }); this.end(false); return; } } if (this.max_attempts !== 0 && this.attempts >= this.max_attempts || this.retry_totaltime >= this.connect_timeout) { - var message = this.retry_totaltime >= this.connect_timeout ? - 'connection timeout exceeded.' : - 'maximum connection attempts exceeded.'; - error = new Error('Redis connection in broken state: ' + message); - error.code = 'CONNECTION_BROKEN'; - this.flush_and_error(error); - this.emit('error', error); + 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; } @@ -620,13 +659,13 @@ RedisClient.prototype.connection_gone = function (why, error) { this.offline_queue.unshift.apply(this.offline_queue, this.command_queue.toArray()); this.command_queue.clear(); } else if (this.command_queue.length !== 0) { - error = new Error('Redis connection lost and command aborted in uncertain state. It might have been processed.'); - error.code = 'UNCERTAIN_STATE'; - if (!this.flush_and_error(error, ['command_queue'])) { - // Only emit if not all commands had a callback that already handled the error - error.message = 'Redis connection lost and commands aborted in uncertain state. They might have been processed.'; - this.emit('error', error); - } + this.flush_and_error({ + message: 'Redis connection lost and command aborted.', + code: 'UNCERTAIN_STATE' + }, { + error: error, + queues: ['command_queue'] + }); } if (this.retry_max_delay !== null && this.retry_delay > this.retry_max_delay) { @@ -643,11 +682,9 @@ RedisClient.prototype.connection_gone = function (why, error) { RedisClient.prototype.return_error = function (err) { var command_obj = this.command_queue.shift(); - if (command_obj && command_obj.command && command_obj.command.toUpperCase) { - err.command = command_obj.command.toUpperCase(); - if (command_obj.args.length) { - err.args = command_obj.args; - } + err.command = command_obj.command.toUpperCase(); + if (command_obj.args && command_obj.args.length) { + err.args = command_obj.args; } // Count down pub sub mode if in entering modus @@ -661,7 +698,7 @@ RedisClient.prototype.return_error = function (err) { err.code = match[1]; } - utils.callback_or_emit(this, command_obj && command_obj.callback, err); + utils.callback_or_emit(this, command_obj.callback, err); }; RedisClient.prototype.drain = function () { @@ -809,14 +846,16 @@ function handle_offline_command (self, command_obj) { msg = 'Stream not writeable.'; } } else { - msg = 'The connection has already been closed.'; + msg = 'The connection is already closed.'; } - err = new Error(command + " can't be processed. " + msg); - err.command = command; - if (command_obj.args.length) { + err = new errorClasses.AbortError({ + message: command + " can't be processed. " + msg, + code: 'NR_CLOSED', + command: command + }); + if (command_obj.args && command_obj.args.length) { err.args = command_obj.args; } - err.code = 'NR_OFFLINE'; utils.reply_in_order(self, callback, err); } else { debug('Queueing ' + command + ' for next server connection.'); @@ -889,8 +928,8 @@ RedisClient.prototype.internal_send_command = function (command, args, callback, args_copy[i] = '' + args[i]; } } - args = null; - command_obj = new Command(command, args_copy, buffer_args, callback); + // Pass the original args to make sure in error cases the original arguments are returned + command_obj = new Command(command, args, buffer_args, callback); if (this.options.prefix) { prefix_keys = commands.getKeyIndexes(command, args_copy); @@ -1053,6 +1092,9 @@ exports.createClient = function () { exports.RedisClient = RedisClient; exports.print = utils.print; exports.Multi = require('./lib/multi'); +exports.AbortError = errorClasses.AbortError; +exports.ReplyError = errorClasses.ReplyError; +exports.AggregateError = errorClasses.AggregateError; // Add all redis commands / node_redis api to the client require('./lib/individualCommands'); diff --git a/lib/customError.js b/lib/customError.js deleted file mode 100644 index 07101b5806..0000000000 --- a/lib/customError.js +++ /dev/null @@ -1,16 +0,0 @@ -'use strict'; - -var util = require('util'); - -function CommandError (error) { - Error.captureStackTrace(this, this.constructor); - this.name = this.constructor.name; - this.message = error.message; - for (var keys = Object.keys(error), key = keys.pop(); key; key = keys.pop()) { - this[key] = error[key]; - } -} - -util.inherits(CommandError, Error); - -module.exports = CommandError; diff --git a/lib/customErrors.js b/lib/customErrors.js new file mode 100644 index 0000000000..7c6e457036 --- /dev/null +++ b/lib/customErrors.js @@ -0,0 +1,78 @@ +'use strict'; + +var util = require('util'); + +function AbortError (obj) { + Error.captureStackTrace(this, this.constructor); + var message; + Object.defineProperty(this, 'name', { + get: function () { + return this.constructor.name; + } + }); + Object.defineProperty(this, 'message', { + get: function () { + return message; + }, + set: function (msg) { + message = msg; + } + }); + for (var keys = Object.keys(obj), key = keys.pop(); key; key = keys.pop()) { + this[key] = obj[key]; + } + // Explicitly add the message + // If the obj is a error itself, the message is not enumerable + this.message = obj.message; +} + +function ReplyError (obj) { + Error.captureStackTrace(this, this.constructor); + var tmp; + Object.defineProperty(this, 'name', { + get: function () { + return this.constructor.name; + } + }); + Object.defineProperty(this, 'message', { + get: function () { + return tmp; + }, + set: function (msg) { + tmp = msg; + } + }); + this.message = obj.message; +} + +function AggregateError (obj) { + Error.captureStackTrace(this, this.constructor); + var tmp; + Object.defineProperty(this, 'name', { + get: function () { + return this.constructor.name; + } + }); + Object.defineProperty(this, 'message', { + get: function () { + return tmp; + }, + set: function (msg) { + tmp = msg; + } + }); + for (var keys = Object.keys(obj), key = keys.pop(); key; key = keys.pop()) { + this[key] = obj[key]; + } + this.message = obj.message; +} + +util.inherits(ReplyError, Error); +util.inherits(AbortError, Error); +util.inherits(AggregateError, AbortError); + +module.exports = { + ReplyError: ReplyError, + AbortError: AbortError, + AggregateError: AggregateError +}; diff --git a/lib/extendedApi.js b/lib/extendedApi.js index ef6a2faa6e..ece77d22fe 100644 --- a/lib/extendedApi.js +++ b/lib/extendedApi.js @@ -47,10 +47,10 @@ RedisClient.prototype.send_command = RedisClient.prototype.sendCommand = functio RedisClient.prototype.end = function (flush) { // Flush queue if wanted if (flush) { - var err = new Error("The command can't be processed. The connection has already been closed."); - err.code = 'NR_OFFLINE'; - this.flush_and_error(err); - // TODO: Emit an error in case a command did not have a callback + this.flush_and_error({ + message: 'Connection forcefully ended and command aborted.', + code: 'NR_CLOSED' + }); } else if (arguments.length === 0) { this.warn( 'Using .end() without the flush parameter is deprecated and throws from v.3.0.0 on.\n' + diff --git a/lib/individualCommands.js b/lib/individualCommands.js index c9cef49b1b..808ad99a0b 100644 --- a/lib/individualCommands.js +++ b/lib/individualCommands.js @@ -79,7 +79,7 @@ Multi.prototype.monitor = Multi.prototype.MONITOR = function monitor (callback) function quit_callback (self, callback) { return function (err, res) { - if (err && err.code === 'NR_OFFLINE') { + if (err && err.code === 'NR_CLOSED') { // Pretent the quit command worked properly in this case. // Either the quit landed in the offline queue and was flushed at the reconnect // or the offline queue is deactivated and the command was rejected right away @@ -90,7 +90,7 @@ function quit_callback (self, callback) { } utils.callback_or_emit(self, callback, err, res); if (self.stream.writable) { - // If the socket is still alive, kill it. This could happen if quit got a NR_OFFLINE error code + // If the socket is still alive, kill it. This could happen if quit got a NR_CLOSED error code self.stream.destroy(); } }; diff --git a/test/batch.spec.js b/test/batch.spec.js index 7b382eee42..762b2f7049 100644 --- a/test/batch.spec.js +++ b/test/batch.spec.js @@ -36,7 +36,7 @@ describe("The 'batch' method", function () { batch.set('foo', 'bar'); batch.exec(function (err, res) { assert.strictEqual(err, null); - assert.strictEqual(res[0].code, 'NR_OFFLINE'); + assert.strictEqual(res[0].code, 'NR_CLOSED'); done(); }); }); diff --git a/test/commands/dbsize.spec.js b/test/commands/dbsize.spec.js index b1a3d3c32e..4fdf80010d 100644 --- a/test/commands/dbsize.spec.js +++ b/test/commands/dbsize.spec.js @@ -31,7 +31,7 @@ describe("The 'dbsize' method", function () { it('reports an error', function (done) { client.dbsize([], function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); }); diff --git a/test/commands/flushdb.spec.js b/test/commands/flushdb.spec.js index 9081105349..61535e2e71 100644 --- a/test/commands/flushdb.spec.js +++ b/test/commands/flushdb.spec.js @@ -31,7 +31,7 @@ describe("The 'flushdb' method", function () { it('reports an error', function (done) { client.flushdb(function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); }); diff --git a/test/commands/get.spec.js b/test/commands/get.spec.js index 83a330ad12..e2b9a7db07 100644 --- a/test/commands/get.spec.js +++ b/test/commands/get.spec.js @@ -31,14 +31,14 @@ describe("The 'get' method", function () { it('reports an error', function (done) { client.get(key, function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); }); it('reports an error promisified', function () { return client.getAsync(key).then(assert, function (err) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); }); }); }); diff --git a/test/commands/getset.spec.js b/test/commands/getset.spec.js index ea6b5d5ef7..e5da857311 100644 --- a/test/commands/getset.spec.js +++ b/test/commands/getset.spec.js @@ -32,7 +32,7 @@ describe("The 'getset' method", function () { it('reports an error', function (done) { client.get(key, function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); }); diff --git a/test/commands/hgetall.spec.js b/test/commands/hgetall.spec.js index 8e8b88a309..ba44326510 100644 --- a/test/commands/hgetall.spec.js +++ b/test/commands/hgetall.spec.js @@ -24,8 +24,10 @@ describe("The 'hgetall' method", function () { it('handles simple keys and values', function (done) { client.hmset(['hosts', '__proto__', '1', 'another', '23', 'home', '1234'], helper.isString('OK')); client.HGETALL(['hosts'], function (err, obj) { - assert.strictEqual(3, Object.keys(obj).length); - assert.strictEqual('1', obj.__proto__.toString()); // eslint-disable-line no-proto + if (!/^v0\.10/.test(process.version)) { + assert.strictEqual(3, Object.keys(obj).length); + assert.strictEqual('1', obj.__proto__.toString()); // eslint-disable-line no-proto + } assert.strictEqual('23', obj.another.toString()); assert.strictEqual('1234', obj.home.toString()); done(err); diff --git a/test/commands/mset.spec.js b/test/commands/mset.spec.js index c6963641a2..9fac90728c 100644 --- a/test/commands/mset.spec.js +++ b/test/commands/mset.spec.js @@ -33,7 +33,7 @@ describe("The 'mset' method", function () { it('reports an error', function (done) { client.mset(key, value, key2, value2, function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); }); @@ -96,7 +96,8 @@ describe("The 'mset' method", function () { // this behavior is different from the 'set' behavior. it('emits an error', function (done) { client.on('error', function (err) { - assert.equal(err.message, "ERR wrong number of arguments for 'mset' command"); + assert.strictEqual(err.message, "ERR wrong number of arguments for 'mset' command"); + assert.strictEqual(err.name, 'ReplyError'); done(); }); diff --git a/test/commands/select.spec.js b/test/commands/select.spec.js index d8878fe48c..4297dca7f3 100644 --- a/test/commands/select.spec.js +++ b/test/commands/select.spec.js @@ -23,7 +23,7 @@ describe("The 'select' method", function () { it('returns an error if redis is not connected', function (done) { var buffering = client.select(1, function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); assert(typeof buffering === 'boolean'); diff --git a/test/commands/set.spec.js b/test/commands/set.spec.js index 7cb4b8314a..01b6244381 100644 --- a/test/commands/set.spec.js +++ b/test/commands/set.spec.js @@ -31,7 +31,7 @@ describe("The 'set' method", function () { it('reports an error', function (done) { client.set(key, value, function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); }); diff --git a/test/connection.spec.js b/test/connection.spec.js index eb807295ed..9661f42d3b 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -53,7 +53,7 @@ describe('connection tests', function () { } }); client.set('foo', 'bar', function (err, res) { - assert.strictEqual(err.message, 'Stream connection ended and running command aborted. It might have been processed.'); + assert.strictEqual(err.message, 'Stream connection ended and command aborted.'); called = -1; }); }); @@ -62,7 +62,7 @@ describe('connection tests', function () { var called = false; client = redis.createClient(9999); client.set('foo', 'bar', function (err, res) { - assert.strictEqual(err.message, 'Stream connection ended and running command aborted. It might have been processed.'); + assert.strictEqual(err.message, 'Stream connection ended and command aborted.'); called = true; }); var bool = client.quit(function (err, res) { @@ -259,13 +259,12 @@ describe('connection tests', function () { it('emits error once if reconnecting after command has been executed but not yet returned without callback', function (done) { client = redis.createClient.apply(null, args); - client.on('error', function (err) { - assert.strictEqual(err.code, 'UNCERTAIN_STATE'); - done(); - }); client.on('ready', function () { - client.set('foo', 'bar'); + client.set('foo', 'bar', function (err) { + assert.strictEqual(err.code, 'UNCERTAIN_STATE'); + done(); + }); // Abort connection before the value returned client.stream.destroy(); }); @@ -281,7 +280,8 @@ describe('connection tests', function () { retryStrategy: function (options) { if (options.totalRetryTime > 150) { client.set('foo', 'bar', function (err, res) { - assert.strictEqual(err.message, 'Connection timeout'); + assert.strictEqual(err.message, 'Stream connection ended and command aborted.'); + assert.strictEqual(err.origin.message, 'Connection timeout'); done(); }); // Pass a individual error message to the error handler @@ -308,7 +308,9 @@ describe('connection tests', function () { retry_strategy: function (options) { if (options.total_retry_time > 150) { client.set('foo', 'bar', function (err, res) { - assert.strictEqual(err.code, 'ECONNREFUSED'); + assert.strictEqual(err.message, 'Stream connection ended and command aborted.'); + assert.strictEqual(err.code, 'NR_CLOSED'); + assert.strictEqual(err.origin.code, 'ECONNREFUSED'); done(); }); return false; @@ -319,10 +321,15 @@ describe('connection tests', function () { }); }); - it('retry_strategy used to reconnect with defaults', function (done) { + it('retryStrategy used to reconnect with defaults', function (done) { + var unhookIntercept = intercept(function () { + return ''; + }); + redis.debugMode = true; client = redis.createClient({ - retry_strategy: function (options) { + retryStrategy: function (options) { client.set('foo', 'bar'); + assert(redis.debugMode); return null; } }); @@ -330,9 +337,10 @@ describe('connection tests', function () { client.stream.destroy(); }, 50); client.on('error', function (err) { - assert.strictEqual(err.code, 'NR_OFFLINE'); - assert.strictEqual(err.errors.length, 1); - assert.notStrictEqual(err.message, err.errors[0].message); + assert.strictEqual(err.code, 'NR_CLOSED'); + assert.strictEqual(err.message, 'Stream connection ended and command aborted.'); + unhookIntercept(); + redis.debugMode = false; done(); }); }); diff --git a/test/multi.spec.js b/test/multi.spec.js index 8f500958fa..8deae7f920 100644 --- a/test/multi.spec.js +++ b/test/multi.spec.js @@ -111,7 +111,7 @@ describe("The 'multi' method", function () { it('reports an error', function (done) { var multi = client.multi(); var notBuffering = multi.exec(function (err, res) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); done(); }); assert.strictEqual(notBuffering, false); @@ -119,7 +119,7 @@ describe("The 'multi' method", function () { it('reports an error if promisified', function () { return client.multi().execAsync().catch(function (err) { - assert(err.message.match(/The connection has already been closed/)); + assert(err.message.match(/The connection is already closed/)); }); }); }); diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index c87968dea8..8ba91fdfe4 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -3,6 +3,7 @@ var assert = require('assert'); var fs = require('fs'); var path = require('path'); +var intercept = require('intercept-stdout'); var config = require('./lib/config'); var helper = require('./helper'); var utils = require('../lib/utils'); @@ -58,6 +59,12 @@ describe('The node_redis client', function () { assert.strictEqual(client2.options[elem], client.options[elem]); } } + client2.on('error', function (err) { + assert.strictEqual(err.message, 'Connection forcefully ended and command aborted. It might have been processed.'); + assert.strictEqual(err.command, 'SELECT'); + assert(err instanceof Error); + assert.strictEqual(err.name, 'AbortError'); + }); client2.on('ready', function () { client2.end(true); done(); @@ -193,10 +200,10 @@ describe('The node_redis client', function () { }); }); - it('using multi with send_command should work as individual command instead of using the internal multi', function (done) { + it('using multi with sendCommand should work as individual command instead of using the internal multi', function (done) { // This is necessary to keep backwards compatibility and it is the only way to handle multis as you want in node_redis - client.send_command('multi'); - client.send_command('set', ['foo', 'bar'], helper.isString('QUEUED')); + client.sendCommand('multi'); + client.sendCommand('set', ['foo', 'bar'], helper.isString('QUEUED')); client.get('foo'); client.exec(function (err, res) { // exec is not manipulated if not fired by the individual multi command // As the multi command is handled individually by the user he also has to handle the return value @@ -340,7 +347,8 @@ describe('The node_redis client', function () { } }, 20); var cb = function (err, res) { - assert(/The connection has already been closed/.test(err.message)); + assert(/Connection forcefully ended|The connection is already closed./.test(err.message)); + assert.strictEqual(err.code, 'NR_CLOSED'); end(); }; for (var i = 0; i < 20; i++) { @@ -361,7 +369,7 @@ describe('The node_redis client', function () { done(); }, 20); var cb = function (err, res) { - assert(/The connection has already been closed./.test(err.message)); + assert(/Connection forcefully ended|The connection is already closed./.test(err.message)); end(); }; for (var i = 0; i < 20; i++) { @@ -373,6 +381,59 @@ describe('The node_redis client', function () { } }); + it('emits an aggregate error if no callback was present for multiple commands in debug_mode', function (done) { + redis.debug_mode = true; + var unhookIntercept = intercept(function (data) { + return ''; // Don't print the debug messages + }); + client.set('foo', 'bar'); + client.set('baz', 'hello world'); + client.on('error', function (err) { + assert(err instanceof Error); + assert(err instanceof redis.AbortError); + assert(err instanceof redis.AggregateError); + assert.strictEqual(err.name, 'AggregateError'); + assert.strictEqual(err.errors.length, 2); + assert.strictEqual(err.message, 'Connection forcefully ended and commands aborted.'); + assert.strictEqual(err.code, 'NR_CLOSED'); + assert.strictEqual(err.errors[0].message, 'Connection forcefully ended and command aborted. It might have been processed.'); + assert.strictEqual(err.errors[0].command, 'SET'); + assert.strictEqual(err.errors[0].code, 'NR_CLOSED'); + assert.deepEqual(err.errors[0].args, ['foo', 'bar']); + done(); + }); + client.end(true); + unhookIntercept(); + redis.debug_mode = false; + }); + + it('emits an abort error if no callback was present for a single commands', function (done) { + redis.debug_mode = true; + var unhookIntercept = intercept(function (data) { + return ''; // Don't print the debug messages + }); + client.set('foo', 'bar'); + client.on('error', function (err) { + assert(err instanceof Error); + assert(err instanceof redis.AbortError); + assert(!(err instanceof redis.AggregateError)); + assert.strictEqual(err.message, 'Connection forcefully ended and command aborted. It might have been processed.'); + assert.strictEqual(err.command, 'SET'); + assert.strictEqual(err.code, 'NR_CLOSED'); + assert.deepEqual(err.args, ['foo', 'bar']); + done(); + }); + client.end(true); + unhookIntercept(); + redis.debug_mode = false; + }); + + it('does not emit abort errors if no callback was present while not being in debug_mode ', function (done) { + client.set('foo', 'bar'); + client.end(true); + setTimeout(done, 100); + }); + }); describe('commands after using .quit should fail', function () { @@ -385,7 +446,7 @@ describe('The node_redis client', function () { client = redis.createClient(); client.quit(function () { client.get('foo', function (err, res) { - assert.strictEqual(err.message, 'Stream connection ended and running command aborted. It might have been processed.'); + assert.strictEqual(err.message, 'Stream connection ended and command aborted. It might have been processed.'); assert.strictEqual(client.offline_queue.length, 0); done(); }); @@ -398,7 +459,7 @@ describe('The node_redis client', function () { client.quit(); setTimeout(function () { client.get('foo', function (err, res) { - assert.strictEqual(err.message, 'GET can\'t be processed. The connection has already been closed.'); + assert.strictEqual(err.message, 'GET can\'t be processed. The connection is already closed.'); assert.strictEqual(err.command, 'GET'); assert.strictEqual(client.offline_queue.length, 0); done(); @@ -410,7 +471,7 @@ describe('The node_redis client', function () { if (helper.redisProcess().spawnFailed()) this.skip(); client.quit(); client.on('error', function (err) { - assert.strictEqual(err.message, 'SET can\'t be processed. The connection has already been closed.'); + assert.strictEqual(err.message, 'SET can\'t be processed. The connection is already closed.'); assert.strictEqual(err.command, 'SET'); assert.strictEqual(client.offline_queue_length, 0); done(); @@ -542,7 +603,7 @@ describe('The node_redis client', function () { }); domain.on('error', function (err) { - assert.strictEqual(err.message, 'SET can\'t be processed. The connection has already been closed.'); + assert.strictEqual(err.message, 'SET can\'t be processed. The connection is already closed.'); domain.exit(); done(); }); @@ -919,8 +980,11 @@ describe('The node_redis client', function () { it('should gracefully recover and only fail on the already send commands', function (done) { client = redis.createClient.apply(null, args); + var error; client.on('error', function (err) { - assert.strictEqual(err.message, 'Protocol error, got "a" as reply type byte'); + assert.strictEqual(err.message, 'Protocol error, got "a" as reply type byte. Please report this.'); + assert.strictEqual(err, error); + assert(err instanceof redis.ReplyError); // After the hard failure work properly again. The set should have been processed properly too client.get('foo', function (err, res) { assert.strictEqual(res, 'bar'); @@ -929,7 +993,10 @@ describe('The node_redis client', function () { }); client.once('ready', function () { client.set('foo', 'bar', function (err, res) { - assert.strictEqual(err.message, 'Protocol error, got "a" as reply type byte'); + assert.strictEqual(err.message, 'Fatal error encountert. Command aborted. It might have been processed.'); + assert.strictEqual(err.code, 'NR_FATAL'); + assert(err instanceof redis.AbortError); + error = err.origin; }); // Fail the set answer. Has no corresponding command obj and will therefore land in the error handler and set client.reply_parser.execute(new Buffer('a*1\r*1\r$1`zasd\r\na')); @@ -974,7 +1041,7 @@ describe('The node_redis client', function () { setTimeout(function () { client.set('foo', 'bar', function (err, result) { if (!finished) done(err); - assert.strictEqual(err.message, "The command can't be processed. The connection has already been closed."); + assert.strictEqual(err.message, 'Connection forcefully ended and command aborted.'); }); setTimeout(function () { @@ -993,10 +1060,15 @@ describe('The node_redis client', function () { var i = 0; client.on('error', function (err) { - if (err.message === 'Redis connection in broken state: maximum connection attempts exceeded.') { + if (err.code === 'CONNECTION_BROKEN') { assert(i, 3); assert.strictEqual(client.offline_queue.length, 0); - done(); + assert.strictEqual(err.origin.code, 'ECONNREFUSED'); + if (!(err instanceof redis.AbortError)) { + done(); + } else { + assert.strictEqual(err.command, 'SET'); + } } else { assert.equal(err.code, 'ECONNREFUSED'); assert.equal(err.errno, 'ECONNREFUSED'); @@ -1111,10 +1183,13 @@ describe('The node_redis client', function () { it('flushes the command queue if connection is lost', function (done) { client = redis.createClient({ parser: parser, - max_attempts: 2, enable_offline_queue: false }); + redis.debug_mode = true; + var unhookIntercept = intercept(function () { + return ''; + }); client.once('ready', function () { var multi = client.multi(); multi.config('bar'); @@ -1133,18 +1208,20 @@ describe('The node_redis client', function () { var end = helper.callFuncAfter(done, 3); client.on('error', function (err) { + assert.equal(client.command_queue.length, 0); if (err.command === 'EXEC') { - assert.equal(client.command_queue.length, 0); assert.equal(err.errors.length, 9); end(); } else if (err.code === 'UNCERTAIN_STATE') { - assert.equal(client.command_queue.length, 0); assert.equal(err.errors.length, 4); end(); } else { assert.equal(err.code, 'ECONNREFUSED'); assert.equal(err.errno, 'ECONNREFUSED'); assert.equal(err.syscall, 'connect'); + redis.debug_mode = false; + client.end(true); + unhookIntercept(); end(); } });