Skip to content

Commit

Permalink
Revert "Client to emit errors now instead of throwing them asynchrono…
Browse files Browse the repository at this point in the history
…usly where they're uncatchable."

This reverts commit 6a3ccf6.
  • Loading branch information
brycebaril committed Jul 11, 2014
1 parent a8403a0 commit 08a8eed
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 74 deletions.
28 changes: 20 additions & 8 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ RedisClient.prototype.flush_and_error = function (message) {
try {
command_obj.callback(error);
} catch (callback_err) {
this.emit("error", callback_err);
process.nextTick(function () {
throw callback_err;
});
}
}
}
Expand All @@ -166,7 +168,9 @@ RedisClient.prototype.flush_and_error = function (message) {
try {
command_obj.callback(error);
} catch (callback_err) {
this.emit("error", callback_err);
process.nextTick(function () {
throw callback_err;
});
}
}
}
Expand Down Expand Up @@ -566,18 +570,24 @@ RedisClient.prototype.return_error = function (err) {
try {
command_obj.callback(err);
} catch (callback_err) {
this.emit("error", 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;
});
}
} else {
console.log("node_redis: no callback to send error: " + err.message);
this.emit("error", err);
// this will probably not make it anywhere useful, but we might as well throw
process.nextTick(function () {
throw 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(client, callback, reply) {
function try_callback(callback, reply) {
try {
callback(null, reply);
} catch (err) {
Expand All @@ -588,7 +598,9 @@ function try_callback(client, callback, reply) {
currDomain.exit();
}
} else {
client.emit("error", err);
process.nextTick(function () {
throw err;
});
}
}
}
Expand Down Expand Up @@ -670,7 +682,7 @@ RedisClient.prototype.return_reply = function (reply) {
reply = reply_to_object(reply);
}

try_callback(this, command_obj.callback, reply);
try_callback(command_obj.callback, reply);
} else if (exports.debug_mode) {
console.log("no callback for reply: " + (reply && reply.toString && reply.toString()));
}
Expand All @@ -696,7 +708,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(this, command_obj.callback, reply1String);
try_callback(command_obj.callback, reply1String);
}
this.emit(type, reply1String, reply[2]); // channel, count
} else {
Expand Down
66 changes: 0 additions & 66 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,72 +402,6 @@ 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";

Expand Down

0 comments on commit 08a8eed

Please sign in to comment.