4

When running tests that utilize domains for error handling, Mocha still appears to be throwing an error even if a domain handler inside a library should have caught the error. If I execute the code outside of Mocha, it functions correctly leading me to believe the problem is Mocha.

Example:

foo.js

module.exports = function(done) {
    var domain = require("domain");
    var d = domain.create();

    d.on("error", function() {
        done();
    });

    d.run(function() {
        throw new Error("foo");
    });
}

test.js - Error thrown inside foo.js is not being caught by the domain.

describe("test", function() {
    it("should succeed", function(done) {
        var foo = require("./foo.js");
        foo(function() {
            console.log("done");
            done();
        });
    });
});

result : error thrown

script.js - error is being properly caught by the domain and bubbled up.

var foo = require("./foo.js");
foo(function() {
    console.log("done");
});
result : done

As you can see above, if I node straight to script.js it functions as desired, the error is caught by the domain handler and the code continues. If I run the same block of code inside a Mocha test, the error halts the test and a failure is given. I believe this is because the error is being sent on an uncaughtException handler, or something like it. The other complication is that it works properly in Mocha, if I have a process.nextTick() around the function call, leading me to believe that Mocha is only unable to handle synchronous errors, but works just fine with async errors.

There is some talk of this problem here: https://groups.google.com/forum/#!msg/nodejs/n-W9BSfxCjI/SElI1DJ_6u0J and https://github.com/joyent/node/issues/4375 .

The confusion I have is that all of this discussion seems to state the problem has been resolved months ago. Anyone know of either a simple work-around for the issue, or a why I'm not seeing a bug fixed which other people seem to believe is fixed at this point in time.

I am running node v0.10.18 and Mocha 1.13.0 on CentOS 6.3 Vagrant VirtualBox on Windows 7.

Josh Lee
  • 171,072
  • 38
  • 269
  • 275
Owen Allen
  • 11,348
  • 9
  • 51
  • 63
  • try wrapping the call to `foo` function within and anonymous function and use the `should` functions `.should.not.throw()`,`.should.throw()` or call with error parameter `done(error)` – Gntem Oct 19 '13 at 08:57

2 Answers2

12

Found the problem. NodeJS domains catch synchronous errors but the event continues to bubble to a try/catch. If you wrap a domain.run() in a try/catch then the domain error handler AND the catch will be executed.

Thus, it seems the best practice is to use process.nextTick inside all domain.run(). This is shown in the docs example, but isn't expressed as explicitly as I would prefer.

Example:

d.run(function() {
    process.nextTick(function() {
        // do stuff
    });
});

In this case, the flaw is not in Mocha.

Proof of NodeJS domains not catching synchronous errors in try/catch: https://gist.github.com/owenallenaz/7141699

Owen Allen
  • 11,348
  • 9
  • 51
  • 63
  • I think it has to do with this line in the docs: *"Domains provide a way to handle multiple different **IO operations** as a single group"*. So domains shouldn't be used as an all-purpose exception handling mechanism, but specifically for handling I/O-related errors. Implying that domains will only work properly with async errors (they also fail with `require('fs').readFileSync('non-existent-file')`, for instance). – robertklep Oct 24 '13 at 19:51
  • My desire is to use domains like a try/catch, IE if anything inside throws an error, this will catch it. For people looking to do that, if you simply add a process.nextTick() then it doesn't matter whether the internal code throws because of async or sync, it will be caught and handled. This is especially important when communicating with libaries. I may not know that libraries internals to know whether it's going to throw sync or async errors, but what I want to do is `executFunction()` and catch if something goes and act accordingly. – Owen Allen Oct 24 '13 at 21:02
4

nodejs domains do definitely catch synchronous errors

See this simple test case

var domain = require("domain");
var d = domain.create();

d.on("error", function() {
    console.log("domain caught");
});


d.run(function() {
    throw new Error("foo");
});


// result: domain caught

EDIT: Since writing this answer I have written a blog post describing what is going on with domains and try catch and whether you can use domains as a wholesale replacement for try catch. It summarises most of what has been discussed here.

http://www.lighthouselogic.com/node-domains-as-a-replacement-for-try-catch/

ORIGINAL ANSWER:

Actually there is a problem with Mocha.

I wrote the following test function:

function error(callback){
     var d = domain.create().on('error', function(err){
        console.log("Caught Error in Domain Handler")
        return callback(err);
    });
    d.enter();
    throw new Error("TestError");
    d.exit();
}

Then I wrote a simple test without mocha:

error(function(err){
    if(err)
    {
        console.log("Error was returned");
    }else
    {
        console.log("Error was not returned")
    }
})

The output I received was:

Caught Error in Domain Handler
Error was returned

When I tested using Mocha:

describe('Domain Tests', function(){
    it('Should return an error when testing', function(done){
        error(function(err){
            if(err)
            {
                console.log("Error was returned");
            }else
            {
                console.log("Error was not returned")
            }
            return done();
        })
    });
});

I received the following output:

․

  0 passing (4ms)
  1 failing

  1) Domain Tests Should return an error when testing:
     Error: TestError
      at error (/Users/bensudbury/Documents/node_projects/testMochaDomains/test.js:9:11)
      at Context.<anonymous> (/Users/bensudbury/Documents/node_projects/testMochaDomains/testMocha.js:6:3)
      at Test.Runnable.run (/usr/local/share/npm/lib/node_modules/mocha/lib/runnable.js:194:15)
      at Runner.runTest (/usr/local/share/npm/lib/node_modules/mocha/lib/runner.js:358:10)
      at /usr/local/share/npm/lib/node_modules/mocha/lib/runner.js:404:12
      at next (/usr/local/share/npm/lib/node_modules/mocha/lib/runner.js:284:14)
      at /usr/local/share/npm/lib/node_modules/mocha/lib/runner.js:293:7
      at next (/usr/local/share/npm/lib/node_modules/mocha/lib/runner.js:237:23)
      at Object._onImmediate (/usr/local/share/npm/lib/node_modules/mocha/lib/runner.js:261:5)
      at processImmediate [as _immediateCallback] (timers.js:330:15)

As you can see: the domain error handler was short circuited.

This problem seems to be related to the following issues:

https://github.com/visionmedia/mocha/issues/513

While the Node issue has been closed, the issue in mocha is still open.

The workaround that was suggested at: https://gist.github.com/mcollina/4443963 didn't resolve the issue in this case.

I dug through the code of Mocha and found that the problem occurs because mocha wraps the tests in a try catch block. This means that the exception is caught and never sent to the uncaughtException or _fatalException handler depending upon the version of node that you are using.

Your workaround is good, but nodejs domains do definitely catch synchronous errors so I wouldn't change your code but instead change your test. Your new test should look like:

describe("test", function() {
    it("should succeed", function(done) {
        process.nextTick(function(){
            var foo = require("./foo.js");
            foo(function() {
                console.log("done");
                done();
            });
        })      
    });
});

I haven't tested this code, but similar code for my example works properly:

it('Should return an error when testing', function(done){   
    process.nextTick(function(){
        error(function(err){
            if(err)
            {
                console.log("Error was returned");
            }else
            {
                console.log("Error was not returned")
            }
            return done();
        });
    })
});

I have added a comment to the end of the issue in Mocha to see if it can be resolved:

https://github.com/visionmedia/mocha/issues/513

Sudsy
  • 931
  • 7
  • 16
  • Sudsy, I do not believe you are correct. https://gist.github.com/owenallenaz/7141699 here is my proof of concept. One shows a synchronous error which is NOT caught and bubbles up to the try/catch. The other shows an async error which IS caught and does not bubble to the try/catch. Do you get different results in your Node? Based on this proof, if Mocha wraps your test in a try/catch (which it does), then it errors will bubble to the try/catch causing Mocha to believe the test threw. Thus, the problem is not in Mocha. – Owen Allen Oct 24 '13 at 17:40
  • I get the same result as you for the gist that you have provided, but what you have provided is not the simplest test case. Please try the following gist https://gist.github.com/sudsy/7162271. This proves clearly that node.js domains can catch synchronous errors. – Sudsy Oct 25 '13 at 22:08
  • Your example does not actually show an example with synchronous errors because wrapping it in process.nextTick turns it into an asynchronous error. – Sudsy Oct 25 '13 at 22:18
  • Look at the gist I linked, there are two examples in it, one synchronous and the other asynchronous. – Owen Allen Oct 25 '13 at 22:27
  • But you didn't try the example with a synchronous error without a try catch block as shown in my gist. Just to make it clear what I am saying: 1. Node.js domains do catch synchronous errors and 2. try catch blocks can interfere with domain exception handling particularly for synchronous errors. – Sudsy Oct 25 '13 at 23:25
  • I am pleased to see that you edited your answer to indicate that node.js domains do in fact catch synchronous errors. You might throw me a bone and vote up my answer or comments though seeing as it was my input that led you to the answer. – Sudsy Oct 26 '13 at 02:40
  • 2
    If I may interject. Yes, domains do catch the error without the try...catch, but they also stop execution as if the error still happened. I've forked sudsy's gist to show the problem here (as it exists in Node 0.10.12): https://gist.github.com/user4815162342/7222689. So, removing the try...catch may not be a suitable answer in all situations. – user4815162342 Oct 29 '13 at 21:50
  • Very good point. This is a not so subtle difference between handling errors with domains and try catch that I hadn't considered before. I am due to write a blog article about the interplay between try catch and domains and will add this to it. I have been using domains for exception handling in many of my functions and normally I do want them to exit the function and return a callback if an exception occurs but sometimes you do want to continue and try catch is the only way to do that. – Sudsy Oct 29 '13 at 23:14
  • 1
    I have now written a blog article about using nodejs domains as a replacement for try catch blocks. http://www.lighthouselogic.com/node-domains-as-a-replacement-for-try-catch/ .@user4815162342 and @Nucleon will notice that it incorporates a number of the items we discussed here and does conclude that wrapping calls in process.nextTick is best practice after all. – Sudsy Oct 30 '13 at 04:32
  • Sudsy-- great blog post. Now I understand why it's working like this. That saved me a lot of research I had planned to be doing today. Thanks. – user4815162342 Oct 30 '13 at 17:40
  • Thanks @user4815162342 it was research I needed to do anyway and thought it worth sharing around. Thanks for your input about try catch handling. Any chance you might be able to give me some votes on this? My reputation is pretty low and could do with a boost. – Sudsy Oct 31 '13 at 19:38