Merge pull request #858 from fintura/codes

Add redis error codes to the errors returned by the parser. Fixes #538

Author: BridgeAR

index.js

@@ -500,12 +500,19 @@ RedisClient.prototype.connection_gone = function (why) {
     this.retry_timer = setTimeout(retry_connection, this.retry_delay, this);
 };
 
+var err_code = /^([A-Z]+)\s+(.+)$/;
 RedisClient.prototype.return_error = function (err) {
     var command_obj = this.command_queue.shift(), queue_len = this.command_queue.length;
     if (command_obj.command && command_obj.command.toUpperCase) {
         err.command_used = command_obj.command.toUpperCase();
     }
 
+    var match = err.message.match(err_code);
+    // LUA script could return user errors that don't behave like all other errors!
+    if (match) {
+        err.code = match[1];
+    }
+
     if (this.pub_sub_mode === false && queue_len === 0) {
         this.command_queue = new Queue();
         this.emit("idle");
@@ -1053,7 +1060,6 @@ 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.callback(err);
@@ -1071,7 +1077,13 @@ 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 (reply) {
+            if (reply instanceof Error) {
+                var match = reply.message.match(err_code);
+                // LUA script could return user errors that don't behave like all other errors!
+                if (match) {
+                    reply.code = match[1];
+                }
+            } else if (reply) {
                 if (this._client.options.detect_buffers && this.wants_buffers[i] === false) {
                     replies[i - 1] = reply = reply_to_strings(reply);
                 }

test/commands/eval.spec.js

@@ -52,7 +52,11 @@ describe("The 'eval' method", function () {
 
             it('converts lua error to an error response', function (done) {
                 helper.serverVersionAtLeast.call(this, client, [2, 5, 0]);
-                client.eval("return {err='this is an error'}", 0, helper.isError(done));
+                client.eval("return {err='this is an error'}", 0, function(err) {
+                    assert(err.code === undefined);
+                    helper.isError()(err);
+                    done();
+                });
             });
 
             it('represents a lua table appropritely', function (done) {
@@ -124,6 +128,7 @@ describe("The 'eval' method", function () {
                     helper.serverVersionAtLeast.call(this, client, [2, 5, 0]);
                     client.evalsha('ffffffffffffffffffffffffffffffffffffffff', 0);
                     client.on('error', function(err) {
+                        assert.equal(err.code, 'NOSCRIPT');
                         assert(/NOSCRIPT No matching script. Please use EVAL./.test(err.message));
                         done();
                     });

test/commands/keys.spec.js

@@ -14,6 +14,9 @@ describe("The 'keys' method", function () {
             var client;
 
             beforeEach(function (done) {
+                args = args || {};
+                // This is going to test if the high water is also respected
+                args.command_queue_high_water = 100;
                 client = redis.createClient.apply(redis.createClient, args);
                 client.once("connect", function () {
                     client.flushdb(done);

test/commands/multi.spec.js

@@ -254,15 +254,19 @@ describe("The 'multi' method", function () {
                         assert(err.message.match(/^EXECABORT/), "Error message should begin with EXECABORT");
                         assert.equal(err.errors.length, 2, "err.errors should have 2 items");
                         assert.strictEqual(err.errors[0].command_used, 'SET');
+                        assert.strictEqual(err.errors[0].code, 'ERR');
                         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);
+                    client.multi().config("bar").debug("foo").eval("return {err='this is an error'}", 0).exec(function (err, reply) {
+                        assert.strictEqual(reply.length, 3);
+                        assert.equal(reply[0].code, 'ERR');
+                        assert.equal(reply[2].code, undefined);
+                        assert(/^this is an error/.test(reply[2].message));
                         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();
@@ -275,6 +279,7 @@ describe("The 'multi' method", function () {
                     multi.set('foo', 'bar', helper.isString('OK'));
                     multi.debug("foo").exec(function (err, reply) {
                         assert.strictEqual(reply.length, 3);
+                        assert.strictEqual(reply[0].code, 'ERR');
                         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");

test/commands/select.spec.js

@@ -57,7 +57,8 @@ describe("The 'select' method", function () {
                     describe("with a valid db index", function () {
                         it("selects the appropriate database", function (done) {
                             assert.strictEqual(client.selected_db, null, "default db should be null");
-                            client.select(1, function () {
+                            client.select(1, function (err) {
+                                assert.equal(err, null);
                                 assert.equal(client.selected_db, 1, "we should have selected the new valid DB");
                                 return done();
                             });
@@ -68,6 +69,7 @@ describe("The 'select' method", function () {
                         it("returns an error", function (done) {
                             assert.strictEqual(client.selected_db, null, "default db should be null");
                             client.select(9999, function (err) {
+                                assert.equal(err.code, 'ERR');
                                 assert.equal(err.message, 'ERR invalid DB index');
                                 return done();
                             });