Add support for Node 0.8 domains

[chrisdickinson]

This patch adds support for the domains module added in node 0.8.x.

Specifically, this patch changes the following:

• All Database methods attach process.domain to their returned statements as statement.domain before returning them.
• Switched the TRY_CATCH_CALL macro to use MakeCallback (a standard way to call back into node while respecting domain objects).
• Call the builtin EventEmitter constructor function on Database objects upon instantiation since the constructor now has side effects based on the presence or absence of domains.
• Add tests for database methods.

Issues with Expresso

Expresso adds a process.on('uncaughtException') listener that assumes test failure without checking that the error has been handled by a domain. Currently, there's a hacky way to disable that feature, but in the future that should be addressed upstream. For the time being, in the interest of not reporting false negatives, I've included a function wrapper that prevents expresso from freaking out.

[kkaefer]

Cool, thanks for this pull request! I plan to move the tests to mocha anyway, so I should probably do that before merging the domains patch?

[chrisdickinson]

No problem!

Re: mocha, it looks like it may have the same issue as expresso -- again, it'll probably have to be fixed upstream. On the upside, at a glance it seems like the hack to make expresso work with domains will also work with mocha.

One other thing to note: MakeCallback (in C++-land) was introduced in 0.8 -- so it won't compile against <0.8 with this patch. I'm looking into wrapping the TRY_CATCH_CALL in a #if statement so we can use the old definition on pre-0.8.X code -- I just need to figure out what I should be comparing against (if node supplies a WITH_DOMAINS directive or if we should supply our own.) The JS code should be unaffected -- the statement.domain = process.domain; line should effectively be a no-op on pre-0.8.X code. I'll update the PR when I figure out a way forward.

[chrisdickinson]

Okay, I feel better about this now: it should work on node <0.8.X (falling back to the old TRY_CATCH_CALL), as well as supporting domains where available. If domains aren't available, the domain tests are skipped.

[ichernev]

Some questions about the patch:

• why do you set statement.domain to the current domain? I couldn't find a "best practice" or sth about this. If you save the current domain to .domain on Statementobjects, why not also set them on Database objects?
• it seems you set the current domain to each statement object after creation -- can't you put that logic in C++'s Statement construction code? If not -- why now wrap Statement construction in a js function, that would set .domain properly.

[chrisdickinson]

@ichernev .domain gets automatically set on descendants of EventEmitter -- as you can see, I'm calling EventEmitter against the Database object here.

To set domain properly through C++ would require looking at the global process object in C++ (which seemed hairy to me). If I follow your alternate suggestion, would it look something like:

function createStatement() {
  var stmt = new Statement
  stmt.domain = process.domain
  return stmt
}

? If so, I'm +1 on that change.

[ichernev]

@chrisdickinson thanks for the explanation! So the EventEmitter constructor does that assignment. If so why not use it, inside createStatement than an explicit stmt.domain = process.domain? It'll be clearer that way IMHO.

[aredridel]

That makes sense to me too

[cxreg]

FYI, node 0.9 has changed so that an uncaughtException is not emitted if there is an active domain

[springmeyer]

this pull is stale. If anyone is keen on updating this work to support both node v0.8.x and v0.10.x and the new mocha tests then please open a new pull request. closing now without merging.