Help with node.js' handling of domain exceptions from clients created outside of their scopes. Check the comment for description

domains_and_clients.js

var domain = require('domain');
var redis = require('redis');
var eventEmitter = require('events').EventEmitter;

var tests = [];
var testCounter = 0;
var runTest = function(){
  if(tests.length > testCounter){
		tests[testCounter]();
	}else{
		console.log("all done!")
		process.exit();
	}
}

var myDomain = new domain.create();
myDomain.on('error', function(err){
	console.log('Yo, I just saved you from the error: ' + err);
	testCounter++;
	runTest();
});

// PASSING
tests[0] = function(){
	myDomain.run(function(){
		throw new Error('A simple error');
	});
};

// PASSING
tests[1] = function(){
	myDomain.run(function(){
		setTimeout(function(){
			process.nextTick(function(){
				var E = new eventEmitter;
				E.on('thing', function(){
					throw new Error('A deeply nested error');
				})
				setTimeout(function(){
					E.emit('thing');
				}, 100)
			});
		}, 100);
	});
}

// PASSING
var Emm = new eventEmitter;
Emm.on('thing', function(){
	throw new Error('Emmited Error defined outside of scope');
})
tests[2] = function(){
	myDomain.run(function(){
	setTimeout(function(){
			Emm.emit('thing');
		}, 100)
	})
};

// PASSING
tests[3] = function(){
	myDomain.run(function(){
		clientA = redis.createClient();
		clientA.hget('hash', 'key', function(err, data){
			throw new Error('An error after redis (A)');
		});
	});
}

// PASSING
tests[4] = function(){
	clientB = redis.createClient();
	myDomain.run(function(){
		clientB.hget('hash', 'key', function(err, data){
			throw new Error('An error after redis (B)');
		});
	});
};

// FAILING
clientC = redis.createClient();
tests[5] = function(){
	myDomain.run(function(){
		clientC.hget('hash', 'key', function(err, data){
			throw new Error('An error after redis (C)');
		});
	});
};

// start it up
runTest();

I need some help understanding how domains in node.js work :/
Here's a collection of tests to illustrate my confusion.

I set up the test to have a domain which I will run each test in, and I expect all of the tests to throw an error and to be caught by the domain's on('error') event. I chose to use a redis client here (because it's common), but I do not think that this is a problem with the awesome redis package, and I've observed similar behavior with the mysql / sequelize package(s) as well.

All of the tests work except #4, which throws an out-of-domain exception and causes the script to crash

• Test #0 is a simple throw in the scope of running domian. Everything works great.
• Test #1 is a very convoluted throw in the scope of the domain involving timeouts, process.nextTick, and event emitters (I was trying to break things). Node preforms like a boss, and the eventual throw is caught.
• Test #2 defines the emitter outside of the domain, but invokes the event from within it. Still the error is caught.
• Test #3 creates the redis client within the scope of the domain. The error is caught and all is well
• Test #4 creates the redis client OUTSIDE of the domain's scope, but within a containing function. This error is caught by the domain
• Test #5 fails, and the redis client here is created before the tests begin to run.

In all of these cases, I would have hoped for the domain to catch the exceptions. I also would have also expected tests #4 and #5 to behave the same way regardless of whether or not connections created outside of the scope of a domain can have their exceptions caught by it.

Can anyone help me to explain why test 5 fails?

---

I'm running node v0.8.14 on OSX Mountain Lion

---

For the lazy, here's my output:

> node test.js 
Yo, I just saved you from the error: Error: A simple error
Yo, I just saved you from the error: Error: A deeply nested error
Yo, I just saved you from the error: Error: Emmited Error defined outside of scope
Yo, I just saved you from the error: Error: An error after redis (A)
Yo, I just saved you from the error: Error: An error after redis (B)

domain.js:66
    throw er;
          ^
Error: An error after redis (C)
    at tests.(anonymous function) (/Users/evantahler/Desktop/test.js:85:10)
    at try_callback (/Users/evantahler/Desktop/node_modules/redis/index.js:520:9)
    at RedisClient.return_reply (/Users/evantahler/Desktop/node_modules/redis/index.js:590:13)
    at ReplyParser.RedisClient.init_parser (/Users/evantahler/Desktop/node_modules/redis/index.js:263:14)
    at ReplyParser.EventEmitter.emit (events.js:96:17)
    at ReplyParser.send_reply (/Users/evantahler/Desktop/node_modules/redis/lib/parser/javascript.js:297:10)
    at ReplyParser.execute (/Users/evantahler/Desktop/node_modules/redis/lib/parser/javascript.js:198:22)
    at RedisClient.on_data (/Users/evantahler/Desktop/node_modules/redis/index.js:476:27)
    at Socket.<anonymous> (/Users/evantahler/Desktop/node_modules/redis/index.js:79:14)
    at Socket.EventEmitter.emit (events.js:96:17)

It also seems that I can't select 'javascript' for the format of this gist for some reason. Sorry :/

Even though this is a weird hole in the design of domains right now, your tests are behaving exactly as they should. The Redis client creates a connection pool, so connections are being reused by multiple clients / async calls. We need some way of binding a specific connection to a specific call's domain.

There's a way to do this from your code -- simply call domain.bind() on the callback you pass to the HGET call and the callback will be bound to the domain. Part of the point of domains is that as an end user of the platform you shouldn't have to do anything special to make your stuff work, so this work falls on the shoulders of the maintainers of the relatively small set of modules that create and manage connection pools. I currently have a pull request open on node-redis with the requisite changes, but @mranney wanted to work through the queue of issues that came up as a result of the recent node-redis parser changes before getting to domains. You can grab my fork / look at the PR to get a better understanding of how it's supposed to work.

Also, name your main file main.js or something to get it to start using JS highlighting.

Thanks!

It makes perfect sense that the connections are shared between requests and exist outside of the domain. domain.bind works well, and it logically it makes senes to need to bind it to something shared like a pooled connection.

However, I'm still confused about why all instances of using the redis client (created outside of the domain) do not fail the test. I expect both #4 and #5 to fail right now?

And yay for syntax highlighting!

I'd have to spend some time in node-inspector to answer with certainty, but my guess is that node-redis creates the connections on need, so the first x calls to operations are going to originate connections that subsequently get released to the connection pool but semi-inadvertently get "correctly" bound to the domain as a side effect, but only for the first command executed on that connection.

Assuming that's correct: Having it break in this particular way is pretty bad because it is surprising and inconsistent, but it's also not easy to fix without code changes to affected modules. Fortunately, the number of modules with this problem is comparatively tiny.

I don't think that's the case, because I can add in n test #4's (I tried it with 10), and they will all pass.

My guess is that that extra function() changes a this down the line. Perhaps there is something special about not being at the 'top' level of the execution stack when a domain tries to catch an error.

You're right; I was looking at all of this on my iPhone's screen and didn't notice how tests #4 and #5 actually differed. Sorry! Pretty good theory I pulled straight out of my ass, though, no? ;)

The reason #5 is failing is pretty straightforward – you're creating the client in a completely different execution context than the one in which the call to Redis is being made. This is exactly the issue I was talking about in my first comment – by the time the domain comes into play, the EventEmitter underlying the connection is already in existence, and therefore doesn't get created / implicitly bound to the domain. Test #2 passes because the EventEmitter gets picked up because you're using it directly in domain.run(); in #5 the driver itself is an EE, but so is the underlying socket, so the domain is making it part of the way there but not all the way down through the layers of wrapping.

If you know that each connection is going to be bound to one domain for the lifetime of an app, you can use the the domain.add() call to bind the underlying EventEmitter to the domain. However, most of the time that isn't what you want, because different calls out to Redis are going to be attached to different domains. If you look at https://github.com/mranney/node_redis/pull/310/files you'll see how small the necessary change to a connection-pooling module is.

Gist needs a mobile view...
Thanks for all your help. It's all starting to make sense now :D

Thanks again @othiym23

ActionHero's cache commands now don't crash the server if something goes wrong :D

evantahler/actionhero@c5ebfc0