From 0b6870be5cab57315594fadd1fd205647b267932 Mon Sep 17 00:00:00 2001 From: Forrest L Norvell Date: Mon, 15 Oct 2012 00:21:55 -0700 Subject: [PATCH] Added direct support for domains. There are three pieces to this support, all of them small, and none of them with large overhead: 1. When sending a command, ensure that any callback is bound to the current domain, if one is present. 2. Also add the RedisClient to the current domain so that error events bubble properly. 3. In try_callback, when a domain is in play, instead of throwing on the next tick, emit the error on the domain. The parser can still finish processing the response and the error ends up in the correct place. --- index.js | 17 ++++++++++++++--- test.js | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index b83492f916..cfe321ce8a 100644 --- a/index.js +++ b/index.js @@ -573,14 +573,19 @@ RedisClient.prototype.return_error = function (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) { try { callback(null, reply); } catch (err) { - process.nextTick(function () { - throw err; - }); + if (process.domain) { + process.domain.emit('error', err); + } else { + process.nextTick(function () { + throw err; + }); + } } } @@ -750,6 +755,12 @@ RedisClient.prototype.send_command = function (command, args, callback) { throw new Error("send_command: second argument must be an array"); } + if (callback && process.domain) { + callback = process.domain.bind(callback); + // ensure that any unhandled error events emitted by the client are scoped to the correct domain + process.domain.add(this); + } + // if the last argument is an array and command is sadd or srem, expand it out: // client.sadd(arg1, [arg2, arg3, arg4], cb); // converts to: diff --git a/test.js b/test.js index e79832b474..ad2e79c800 100644 --- a/test.js +++ b/test.js @@ -1950,6 +1950,30 @@ tests.SLOWLOG = function () { }); } +tests.DOMAIN = function () { + var name = "DOMAIN"; + + var domain; + try { + domain = require('domain').create(); + } catch (err) { + console.log("Skipping " + name + " because this version of node doesn't have domains."); + next(name); + } + + if (domain) { + domain.run(function () { + client.set('domain', 'value', function (err, res) { + assert.ok(process.domain); + var notFound = res.not.existing.thing; // ohhh nooooo + }); + }); + + // this is the expected and desired behavior + domain.on('error', function (err) { next(name); }); + } +}; + // TODO - need a better way to test auth, maybe auto-config a local Redis server or something. // Yes, this is the real password. Please be nice, thanks. tests.auth = function () {