node.js domain module async error handling

example.coffee

(req, res, next) ->
  # req and res are also created in the scope of serverDomain
  # however, we'd prefer to have a separate domain for each request.
  # create it first thing, and add req and res to it.
  reqd = domain.create()
  reqd.add(req)
  reqd.add(res)
  reqd.on "error", (er) ->
    console.error "Error", er, req.url
    try
      res.writeHead 500
      res.end "Error occurred, sorry."
      res.on "close", ->
        # forcibly shut down any other things added to this domain
        reqd.dispose()

    catch er
      console.error "Error sending 500", er, req.url
      
      # tried our best.  clean up anything remaining.
      reqd.dispose()
      
  next()

copy/pasted from email

Here are some usefule links:

• http://nodejs.org/docs/v0.8.0/api/domain.html
• http://geoff.greer.fm/2012/06/10/nodejs-dealing-with-errors/
• http://benno.id.au/blog/2011/08/08/nodejs-exceptions

I got excited about that "api/domain" link because it's new and seems to solve the problem. But after digging into it (nodejs/node-v0.x-archive#3908) I'm still at the same place as where I started...

The issue is you essentially have to pass around the "context" in every method in your app, which means rewriting every method in tower.

Here is why (briefly):

When an HTTP request comes in, you can create a "domain" for it and attach it to the request. Perfect. This is the "context". So if an error is thrown, set error.context = request.context and you can respond with an error message in the request for that user.

The issue though is the way these "per-request domains" are stored. They're stored globally in a stack. If you think about it, that's the only way (unless you pass the context to every method in all your code).

So say a request comes in and runs an async callback that takes 5 seconds. Then another request comes in that has an async callback that takes 10 seconds. If you push and pop from the stack then when the first request finishes it's going to pop off the second request, which is wrong. This would mean the second user would get the error response from the first user's request! You definitely do NOT want to go down this road.

So, node's "domain" code doesn't allow for async callbacks. What they do is this:

var domain = require('domain');

var globalDomain = domain.create();

App.Post.all(globalDomain.bind(function() {
  throw new Error('an error');
}));

That function passed to bind is actually async, but the way bind is implemented is, it calls the passed function in a "synchronous" way:

domain.enter();
var ret = cb.apply(this, arguments);
domain.exit();

When domain.enter is called, it pushes to the global stack, when domain.exit is called, it pops. That wont cause the problems described before (first request is 5 seconds, second request is 10 seconds) because that cb.apply is "supposed to be" a synchronous function (it's the function with throw new Error('an error') above, which is synchronous).

So it seems to work there... But if you have a nested async function, it fails:

App.Post.all(globalDomain.bind(function() {
  App.Post.all(function() {
    throw new Error('I will crash the server!');
  }); 
}));

This is because that domain.enter(); cb(); domain.exit() code which calls the function: it's synchronous, so the active domain exits before the nested callback is called. That is the main issue.

You can get around that by wrapping every function with globalDomain.bind, but then we're back to the initial problem: now I'd have to rewrite every function in tower to use this new convention. That's what this guys plugin did: https://github.com/ypocat/laeh2#3-laeh2-an-elegant-solution

So, before I consider rewriting all of the code to use something like that plugin, we must see if there is an alternative. This will take time, maybe a long time.