-
-
Save evantahler/4274698 to your computer and use it in GitHub Desktop.
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 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
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.