Skip to content

Commit 4f0443c

Browse files
author
Ruben Bridgewater
committed
Emit errors instead of throwing them
Thrown errors might kill the users app. By emitting the errors the user is able to catch all errors in one place without the app going down
1 parent c6ae783 commit 4f0443c

File tree

6 files changed

+43
-45
lines changed

6 files changed

+43
-45
lines changed

‎index.js

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,8 @@ RedisClient.prototype.init_parser = function () {
280280
return true;
281281
}
282282
})) {
283+
// Do not emit this error
284+
// This should take down the app if anyone made such a huge mistake or should somehow be handled in user code
283285
throw new Error("Couldn't find named parser " + self.options.parser + " on this system");
284286
}
285287
} else {
@@ -629,10 +631,12 @@ RedisClient.prototype.return_reply = function (reply) {
629631
}
630632
this.emit(type, reply1String, reply[2]); // channel, count
631633
} else {
632-
throw new Error("subscriptions are active but got unknown reply type " + type);
634+
this.emit("error", new Error("subscriptions are active but got unknown reply type " + type));
635+
return;
633636
}
634-
} else if (! this.closing) {
635-
throw new Error("subscriptions are active but got an invalid reply: " + reply);
637+
} else if (!this.closing) {
638+
this.emit("error", new Error("subscriptions are active but got an invalid reply: " + reply));
639+
return;
636640
}
637641
} else if (this.monitoring) {
638642
len = reply.indexOf(" ");
@@ -643,7 +647,7 @@ RedisClient.prototype.return_reply = function (reply) {
643647
});
644648
this.emit("monitor", timestamp, args);
645649
} else {
646-
throw new Error("node_redis command queue state error. If you can reproduce this, please report it.");
650+
this.emit("error", new Error("node_redis command queue state error. If you can reproduce this, please report it."));
647651
}
648652
};
649653

@@ -702,9 +706,16 @@ RedisClient.prototype.send_command = function (command, args, callback) {
702706

703707
// if the value is undefined or null and command is set or setx, need not to send message to redis
704708
if (command === 'set' || command === 'setex') {
705-
if(args[args.length - 1] === undefined || args[args.length - 1] === null) {
709+
if (args.length === 0) {
710+
return;
711+
}
712+
if (args[args.length - 1] === undefined || args[args.length - 1] === null) {
706713
var err = new Error('send_command: ' + command + ' value must not be undefined or null');
707-
return callback && callback(err);
714+
if (callback) {
715+
return callback && callback(err);
716+
}
717+
this.emit("error", err);
718+
return;
708719
}
709720
}
710721

@@ -731,7 +742,8 @@ RedisClient.prototype.send_command = function (command, args, callback) {
731742
if (command_obj.callback) {
732743
command_obj.callback(not_writeable_error);
733744
} else {
734-
throw not_writeable_error;
745+
this.emit("error", not_writeable_error);
746+
return;
735747
}
736748
}
737749

@@ -745,7 +757,8 @@ RedisClient.prototype.send_command = function (command, args, callback) {
745757
} else if (command === "quit") {
746758
this.closing = true;
747759
} else if (this.pub_sub_mode === true) {
748-
throw new Error("Connection in subscriber mode, only subscriber commands may be used");
760+
this.emit("error", new Error("Connection in subscriber mode, only subscriber commands may be used"));
761+
return;
749762
}
750763
this.command_queue.push(command_obj);
751764
this.commands_sent += 1;
@@ -1032,7 +1045,7 @@ Multi.prototype.exec = function (callback) {
10321045
callback(errors);
10331046
return;
10341047
} else {
1035-
throw new Error(err);
1048+
self._client.emit('error', err);
10361049
}
10371050
}
10381051

‎test/commands/eval.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ describe("The 'eval' method", function () {
114114
client.evalsha(sha, 0, helper.isString('eval get sha test', done));
115115
});
116116

117-
it('throws an error if SHA does not exist', function (done) {
117+
it('returns an error if SHA does not exist', function (done) {
118118
helper.serverVersionAtLeast.call(this, client, [2, 5, 0]);
119119
client.evalsha('ffffffffffffffffffffffffffffffffffffffff', 0, helper.isError(done));
120120
});

‎test/commands/mset.spec.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,10 @@ describe("The 'mset' method", function () {
9292

9393
describe("with undefined 'key' and missing 'value' parameter", function () {
9494
// this behavior is different from the 'set' behavior.
95-
it("throws an error", function (done) {
96-
var mochaListener = helper.removeMochaListener();
97-
98-
process.once('uncaughtException', function (err) {
99-
process.on('uncaughtException', mochaListener);
100-
helper.isError()(err, null);
101-
return done();
95+
it("emits an error", function (done) {
96+
client.on('error', function (err) {
97+
assert.equal(err.message, "ERR wrong number of arguments for 'mset' command");
98+
done();
10299
});
103100

104101
client.mset();

‎test/commands/select.spec.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ describe("The 'select' method", function () {
2424
});
2525
});
2626

27-
it("throws an error if redis is not connected", function (done) {
27+
it("returns an error if redis is not connected", function (done) {
2828
client.select(1, function (err, res) {
2929
assert(err.message.match(/Redis connection gone/));
3030
done();
@@ -67,7 +67,7 @@ describe("The 'select' method", function () {
6767
});
6868

6969
describe("with an invalid db index", function () {
70-
it("emits an error", function (done) {
70+
it("returns an error", function (done) {
7171
assert.strictEqual(client.selected_db, null, "default db should be null");
7272
client.select(9999, function (err) {
7373
assert.equal(err.message, 'ERR invalid DB index');
@@ -90,14 +90,12 @@ describe("The 'select' method", function () {
9090
});
9191

9292
describe("with an invalid db index", function () {
93-
it("throws an error when callback not provided", function (done) {
94-
var mochaListener = helper.removeMochaListener();
93+
it("emits an error when callback not provided", function (done) {
9594
assert.strictEqual(client.selected_db, null, "default db should be null");
9695

97-
process.once('uncaughtException', function (err) {
98-
process.on('uncaughtException', mochaListener);
96+
client.on('error', function (err) {
9997
assert.equal(err.message, 'ERR invalid DB index');
100-
return done();
98+
done();
10199
});
102100

103101
client.select(9999);

‎test/commands/set.spec.js

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ describe("The 'set' method", function () {
9696
this.timeout(200);
9797

9898
client.once("error", function (err) {
99-
helper.isError()(err, null);
100-
return done(err);
99+
done(err);
101100
});
102101

103102
client.set();
@@ -107,20 +106,13 @@ describe("The 'set' method", function () {
107106
}, 100);
108107
});
109108

110-
it("does not throw an error", function (done) {
111-
this.timeout(200);
112-
var mochaListener = helper.removeMochaListener();
113-
114-
process.once('uncaughtException', function (err) {
115-
process.on('uncaughtException', mochaListener);
116-
return done(err);
109+
it("does emit an error", function (done) {
110+
client.on('error', function (err) {
111+
assert.equal(err.message, "ERR wrong number of arguments for 'set' command");
112+
done();
117113
});
118114

119-
client.set();
120-
121-
setTimeout(function () {
122-
done();
123-
}, 100);
115+
client.set('foo');
124116
});
125117
});
126118
});

‎test/node_redis.spec.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ describe("The node_redis client", function () {
647647

648648
describe('enable_offline_queue', function () {
649649
describe('true', function () {
650-
it("does not throw an error and enqueues operation", function (done) {
650+
it("does not return an error and enqueues operation", function (done) {
651651
var client = redis.createClient(9999, null, {
652652
max_attempts: 1,
653653
parser: parser
@@ -674,20 +674,18 @@ describe("The node_redis client", function () {
674674
});
675675

676676
describe('false', function () {
677-
it("does not throw an error and enqueues operation", function (done) {
677+
it("does not emit an error and enqueues operation", function (done) {
678678
var client = redis.createClient(9999, null, {
679679
parser: parser,
680680
max_attempts: 1,
681681
enable_offline_queue: false
682682
});
683683

684-
client.on('error', function() {
685-
// ignore, b/c expecting a "can't connect" error
684+
client.on('error', function(err) {
685+
assert(/send_command: stream not writeable|ECONNREFUSED/.test(err.message));
686686
});
687687

688-
assert.throws(function () {
689-
client.set('foo', 'bar');
690-
});
688+
client.set('foo', 'bar');
691689

692690
assert.doesNotThrow(function () {
693691
client.set('foo', 'bar', function (err) {

0 commit comments

Comments
 (0)