From 6a3ccf64f4b49804e0f3514ce3aab4baafd67b00 Mon Sep 17 00:00:00 2001 From: Bryce Baril Date: Sat, 21 Dec 2013 11:08:00 -0800 Subject: [PATCH] Client to emit errors now instead of throwing them asynchronously where they're uncatchable. --- index.js | 28 +++++++----------------- test.js | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/index.js b/index.js index 0ea2312377..7f0fd588b9 100644 --- a/index.js +++ b/index.js @@ -147,9 +147,7 @@ RedisClient.prototype.flush_and_error = function (message) { try { command_obj.callback(error); } catch (callback_err) { - process.nextTick(function () { - throw callback_err; - }); + this.emit("error", callback_err); } } } @@ -161,9 +159,7 @@ RedisClient.prototype.flush_and_error = function (message) { try { command_obj.callback(error); } catch (callback_err) { - process.nextTick(function () { - throw callback_err; - }); + this.emit("error", callback_err); } } } @@ -559,24 +555,18 @@ RedisClient.prototype.return_error = function (err) { try { command_obj.callback(err); } catch (callback_err) { - // if a callback throws an exception, re-throw it on a new stack so the parser can keep going - process.nextTick(function () { - throw callback_err; - }); + this.emit("error", callback_err); } } else { console.log("node_redis: no callback to send error: " + err.message); - // this will probably not make it anywhere useful, but we might as well throw - process.nextTick(function () { - throw err; - }); + this.emit("error", err); } }; // if a callback throws an exception, re-throw it on a new stack so the parser can keep going. // if a domain is active, emit the error on the domain, which will serve the same function. // put this try/catch in its own function because V8 doesn't optimize this well yet. -function try_callback(callback, reply) { +function try_callback(client, callback, reply) { try { callback(null, reply); } catch (err) { @@ -584,9 +574,7 @@ function try_callback(callback, reply) { process.domain.emit('error', err); process.domain.exit(); } else { - process.nextTick(function () { - throw err; - }); + client.emit("error", err); } } } @@ -668,7 +656,7 @@ RedisClient.prototype.return_reply = function (reply) { reply = reply_to_object(reply); } - try_callback(command_obj.callback, reply); + try_callback(this, command_obj.callback, reply); } else if (exports.debug_mode) { console.log("no callback for reply: " + (reply && reply.toString && reply.toString())); } @@ -694,7 +682,7 @@ RedisClient.prototype.return_reply = function (reply) { // reply[1] can be null var reply1String = (reply[1] === null) ? null : reply[1].toString(); if (command_obj && typeof command_obj.callback === "function") { - try_callback(command_obj.callback, reply1String); + try_callback(this, command_obj.callback, reply1String); } this.emit(type, reply1String, reply[2]); // channel, count } else { diff --git a/test.js b/test.js index 1147dc8901..b6850440cb 100644 --- a/test.js +++ b/test.js @@ -402,6 +402,72 @@ tests.FWD_ERRORS_1 = function () { }, 150); }; +tests.FWD_ERRORS_2 = function () { + var name = "FWD_ERRORS_2"; + + var toThrow = new Error("Forced exception"); + var recordedError = null; + + var originalHandler = client.listeners("error").pop(); + client.removeAllListeners("error"); + client.once("error", function (err) { + recordedError = err; + }); + + client.get("no_such_key", function (err, reply) { + throw toThrow; + }); + + setTimeout(function () { + client.listeners("error").push(originalHandler); + assert.equal(recordedError, toThrow, "Should have caught our forced exception"); + next(name); + }, 150); +}; + +tests.FWD_ERRORS_3 = function () { + var name = "FWD_ERRORS_3"; + + var recordedError = null; + + var originalHandler = client.listeners("error").pop(); + client.removeAllListeners("error"); + client.once("error", function (err) { + recordedError = err; + }); + + client.send_command("no_such_command", []); + + setTimeout(function () { + client.listeners("error").push(originalHandler); + assert.ok(recordedError instanceof Error); + next(name); + }, 150); +}; + +tests.FWD_ERRORS_4 = function () { + var name = "FWD_ERRORS_4"; + + var toThrow = new Error("Forced exception"); + var recordedError = null; + + var originalHandler = client.listeners("error").pop(); + client.removeAllListeners("error"); + client.once("error", function (err) { + recordedError = err; + }); + + client.send_command("no_such_command", [], function () { + throw toThrow; + }); + + setTimeout(function () { + client.listeners("error").push(originalHandler); + assert.equal(recordedError, toThrow, "Should have caught our forced exception"); + next(name); + }, 150); +}; + tests.EVAL_1 = function () { var name = "EVAL_1";