Skip to content

Commit 6a3ccf6

Browse files
committed
Client to emit errors now instead of throwing them asynchronously where they're uncatchable.
1 parent 34a2375 commit 6a3ccf6

File tree

2 files changed

+74
-20
lines changed

2 files changed

+74
-20
lines changed

index.js

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,7 @@ RedisClient.prototype.flush_and_error = function (message) {
147147
try {
148148
command_obj.callback(error);
149149
} catch (callback_err) {
150-
process.nextTick(function () {
151-
throw callback_err;
152-
});
150+
this.emit("error", callback_err);
153151
}
154152
}
155153
}
@@ -161,9 +159,7 @@ RedisClient.prototype.flush_and_error = function (message) {
161159
try {
162160
command_obj.callback(error);
163161
} catch (callback_err) {
164-
process.nextTick(function () {
165-
throw callback_err;
166-
});
162+
this.emit("error", callback_err);
167163
}
168164
}
169165
}
@@ -559,34 +555,26 @@ RedisClient.prototype.return_error = function (err) {
559555
try {
560556
command_obj.callback(err);
561557
} catch (callback_err) {
562-
// if a callback throws an exception, re-throw it on a new stack so the parser can keep going
563-
process.nextTick(function () {
564-
throw callback_err;
565-
});
558+
this.emit("error", callback_err);
566559
}
567560
} else {
568561
console.log("node_redis: no callback to send error: " + err.message);
569-
// this will probably not make it anywhere useful, but we might as well throw
570-
process.nextTick(function () {
571-
throw err;
572-
});
562+
this.emit("error", err);
573563
}
574564
};
575565

576566
// if a callback throws an exception, re-throw it on a new stack so the parser can keep going.
577567
// if a domain is active, emit the error on the domain, which will serve the same function.
578568
// put this try/catch in its own function because V8 doesn't optimize this well yet.
579-
function try_callback(callback, reply) {
569+
function try_callback(client, callback, reply) {
580570
try {
581571
callback(null, reply);
582572
} catch (err) {
583573
if (process.domain) {
584574
process.domain.emit('error', err);
585575
process.domain.exit();
586576
} else {
587-
process.nextTick(function () {
588-
throw err;
589-
});
577+
client.emit("error", err);
590578
}
591579
}
592580
}
@@ -668,7 +656,7 @@ RedisClient.prototype.return_reply = function (reply) {
668656
reply = reply_to_object(reply);
669657
}
670658

671-
try_callback(command_obj.callback, reply);
659+
try_callback(this, command_obj.callback, reply);
672660
} else if (exports.debug_mode) {
673661
console.log("no callback for reply: " + (reply && reply.toString && reply.toString()));
674662
}
@@ -694,7 +682,7 @@ RedisClient.prototype.return_reply = function (reply) {
694682
// reply[1] can be null
695683
var reply1String = (reply[1] === null) ? null : reply[1].toString();
696684
if (command_obj && typeof command_obj.callback === "function") {
697-
try_callback(command_obj.callback, reply1String);
685+
try_callback(this, command_obj.callback, reply1String);
698686
}
699687
this.emit(type, reply1String, reply[2]); // channel, count
700688
} else {

test.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,72 @@ tests.FWD_ERRORS_1 = function () {
402402
}, 150);
403403
};
404404

405+
tests.FWD_ERRORS_2 = function () {
406+
var name = "FWD_ERRORS_2";
407+
408+
var toThrow = new Error("Forced exception");
409+
var recordedError = null;
410+
411+
var originalHandler = client.listeners("error").pop();
412+
client.removeAllListeners("error");
413+
client.once("error", function (err) {
414+
recordedError = err;
415+
});
416+
417+
client.get("no_such_key", function (err, reply) {
418+
throw toThrow;
419+
});
420+
421+
setTimeout(function () {
422+
client.listeners("error").push(originalHandler);
423+
assert.equal(recordedError, toThrow, "Should have caught our forced exception");
424+
next(name);
425+
}, 150);
426+
};
427+
428+
tests.FWD_ERRORS_3 = function () {
429+
var name = "FWD_ERRORS_3";
430+
431+
var recordedError = null;
432+
433+
var originalHandler = client.listeners("error").pop();
434+
client.removeAllListeners("error");
435+
client.once("error", function (err) {
436+
recordedError = err;
437+
});
438+
439+
client.send_command("no_such_command", []);
440+
441+
setTimeout(function () {
442+
client.listeners("error").push(originalHandler);
443+
assert.ok(recordedError instanceof Error);
444+
next(name);
445+
}, 150);
446+
};
447+
448+
tests.FWD_ERRORS_4 = function () {
449+
var name = "FWD_ERRORS_4";
450+
451+
var toThrow = new Error("Forced exception");
452+
var recordedError = null;
453+
454+
var originalHandler = client.listeners("error").pop();
455+
client.removeAllListeners("error");
456+
client.once("error", function (err) {
457+
recordedError = err;
458+
});
459+
460+
client.send_command("no_such_command", [], function () {
461+
throw toThrow;
462+
});
463+
464+
setTimeout(function () {
465+
client.listeners("error").push(originalHandler);
466+
assert.equal(recordedError, toThrow, "Should have caught our forced exception");
467+
next(name);
468+
}, 150);
469+
};
470+
405471
tests.EVAL_1 = function () {
406472
var name = "EVAL_1";
407473

0 commit comments

Comments
 (0)