diff --git a/.npmignore b/.npmignore index 4df6949973..886d9eab91 100644 --- a/.npmignore +++ b/.npmignore @@ -6,3 +6,4 @@ generate_commands.js multi_bench.js test-unref.js changelog.md +*.log \ No newline at end of file diff --git a/index.js b/index.js index 010e4a8c59..6d21170ede 100644 --- a/index.js +++ b/index.js @@ -389,7 +389,6 @@ RedisClient.prototype.ready_check = function () { RedisClient.prototype.send_offline_queue = function () { var command_obj, buffered_writes = 0; - // TODO: Implement queue.pop() as it should be faster than shift and evaluate petka antonovs queue while (command_obj = this.offline_queue.shift()) { debug("Sending offline command: " + command_obj.command); buffered_writes += !this.send_command(command_obj.command, command_obj.args, command_obj.callback); @@ -995,15 +994,15 @@ Multi.prototype.hmset = Multi.prototype.HMSET = function (key, args, callback) { return this; }; -Multi.prototype.send_command = function (command, args, cb) { +Multi.prototype.send_command = function (command, args, index, cb) { var self = this; this._client.send_command(command, args, function (err, reply) { if (err) { if (cb) { cb(err); - } else { - self.errors.push(err); } + err.position = index - 1; + self.errors.push(err); } }); }; @@ -1029,7 +1028,7 @@ Multi.prototype.exec = Multi.prototype.EXEC = function (callback) { break; } } - this.send_command(command, args, cb); + this.send_command(command, args, index, cb); } this._client.send_command('exec', [], function(err, replies) { @@ -1042,9 +1041,10 @@ Multi.prototype.execute_callback = function (err, replies) { if (err) { if (err.code !== 'CONNECTION_BROKEN') { + err.code = 'EXECABORT'; + err.errors = this.errors; if (this.callback) { - this.errors.push(err); - this.callback(this.errors); + this.callback(err); } else { // Exclude CONNECTION_BROKEN so that error won't be emitted twice this._client.emit('error', err); @@ -1059,17 +1059,22 @@ Multi.prototype.execute_callback = function (err, replies) { args = this.queue[i]; // If we asked for strings, even in detect_buffers mode, then return strings: - if (this._client.options.detect_buffers && this.wants_buffers[i] === false) { - replies[i - 1] = reply = reply_to_strings(reply); - } - - // TODO - confusing and error-prone that hgetall is special cased in two places - if (reply && args[0] === "hgetall") { - replies[i - 1] = reply = reply_to_object(reply); + if (reply) { + if (this._client.options.detect_buffers && this.wants_buffers[i] === false) { + replies[i - 1] = reply = reply_to_strings(reply); + } + if (args[0] === "hgetall") { + // TODO - confusing and error-prone that hgetall is special cased in two places + replies[i - 1] = reply = reply_to_object(reply); + } } if (typeof args[args.length - 1] === "function") { - args[args.length - 1](null, reply); + if (reply instanceof Error) { + args[args.length - 1](reply); + } else { + args[args.length - 1](null, reply); + } } } } diff --git a/test/commands/multi.spec.js b/test/commands/multi.spec.js index be1ff04299..d2cfd097a3 100644 --- a/test/commands/multi.spec.js +++ b/test/commands/multi.spec.js @@ -249,15 +249,52 @@ describe("The 'multi' method", function () { }); }); - it('reports multiple exceptions when they occur', function (done) { + it('reports EXECABORT exceptions when they occur (while queueing)', function (done) { + client.multi().config("bar").set("foo").exec(function (err, reply) { + assert.equal(err.code, "EXECABORT"); + assert.equal(reply, undefined, "The reply should have been discarded"); + assert(err.message.match(/^EXECABORT/), "Error message should begin with EXECABORT"); + assert.equal(err.errors.length, 1, "err.errors should have 1 items"); + assert.strictEqual(err.errors[0].command_used, 'SET'); + assert.strictEqual(err.errors[0].position, 1); + assert(/^ERR/.test(err.errors[0].message), "Actuall error message should begin with ERR"); + return done(); + }); + }); + + it('reports multiple exceptions when they occur (while EXEC is running)', function (done) { + client.multi().config("bar").debug("foo").exec(function (err, reply) { + assert.strictEqual(reply.length, 2); + assert(/^ERR/.test(reply[0].message), "Error message should begin with ERR"); + assert(/^ERR/.test(reply[1].message), "Error message should begin with ERR"); + return done(); + }); + }); + + it('reports multiple exceptions when they occur (while EXEC is running) and calls cb', function (done) { + var multi = client.multi(); + multi.config("bar", helper.isError()); + multi.set('foo', 'bar', helper.isString('OK')); + multi.debug("foo").exec(function (err, reply) { + assert.strictEqual(reply.length, 3); + assert(/^ERR/.test(reply[0].message), "Error message should begin with ERR"); + assert(/^ERR/.test(reply[2].message), "Error message should begin with ERR"); + assert.strictEqual(reply[1], "OK"); + client.get('foo', helper.isString('bar', done)); + }); + }); + + it("emits an error if no callback has been provided and execabort error occured", function (done) { helper.serverVersionAtLeast.call(this, client, [2, 6, 5]); - client.multi().set("foo").exec(function (err, reply) { - assert(Array.isArray(err), "err should be an array"); - assert.equal(2, err.length, "err should have 2 items"); - assert(err[0].message.match(/^ERR/), "First error message should begin with ERR"); - assert(err[1].message.match(/^EXECABORT/), "First error message should begin with EXECABORT"); - return done(); + var multi = client.multi(); + multi.config("bar"); + multi.set("foo"); + multi.exec(); + + client.on('error', function(err) { + assert.equal(err.code, "EXECABORT"); + done(); }); }); diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index e650dbac8f..821dc170b6 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -77,6 +77,21 @@ describe("The node_redis client", function () { }); }); + it("connects correctly to localhost and no ready check", function (done) { + client = redis.createClient(undefined, undefined, { + no_ready_check: true + }); + client.on("error", done); + + client.once("ready", function () { + client.set('foo', 'bar'); + client.get('foo', function(err, res) { + assert.strictEqual(res, 'bar'); + done(err); + }); + }); + }); + it("throws on strange connection info", function () { try { redis.createClient(true);