Hi Brad, I’m working in Node.js and am writing automated tests using Mocha. Most of our code uses Promises and we tend to terminate our promise chains with the .done() API. Should I be doing that in our test code too? Should I use Bluebird.done() with Mocha?

I ran into some code in a project I work on that does exactly that. In our test code we call our function which we were testing and it returns a promise. We added a .then() block to assert what the promise was returning then we call the .done() API to terminate the chain since its test code and nothing will be added to the chain. I noticed when doing some refactoring which caused one of the tests to fail that my test execution crashed. That is mocha didn’t finish and print out a summary of the failed test, no it instead just crashed.

Turns out the crash was caused by an unhandled exception. The exception was occurring because we were calling the .done() API in the test code.

it('should pass', function () {
  return myPromise.doStuff()
    .then(() => {...})
    .done();
});

What’s the Problem?

The issue is how the .done() API works. In both Q and Bluebird if the promise rejects and the rejection is not handled before hitting the .done() API an exception is thrown.

The idea behind the .done() API is that its used to terminate a promise chain. You use the .done() API to indicate that no further processing will be handled after this point. Therefore if you’ve not caught and handled any errors by this point your going to get an unhandled rejection. Both libraries take the stance that unhandled rejections should not be ignored and thus will throw exceptions which will crash the node process.

Q

promise.done(onFulfilled, onRejected, onProgress)

Much like then, but with different behavior around unhandled rejection. If there is an unhandled rejection, either because promise is rejected and no onRejected callback was provided, or because onFulfilled or onRejected threw an error or returned a rejected promise, the resulting rejection reason is thrown as an exception in a future turn of the event loop.

This method should be used to terminate chains of promises that will not be passed elsewhere. Since exceptions thrown in then callbacks are consumed and transformed into rejections, exceptions at the end of the chain are easy to accidentally, silently ignore. By arranging for the exception to be thrown in a future turn of the event loop, so that it won’t be caught, it causes an onerror event on the browser window, or an uncaughtException event on Node.js’s process object.

Bluebird

Like .then, but any unhandled rejection that ends up here will crash the process (in node) or be thrown as an error (in browsers). The use of this method is heavily discouraged and it only exists for historical reasons.

There is nothing wrong with using .done() at the end of your promise chains if you are really terminating the chain. I’m not a big fan of it personally because it feels kind of like the sealed keyword in C# which indicates that a class can no longer be extended; feels like it goes against the OCP. By using .done() your hindering your ability to simply add to the chain without going in and modifying the existing chain (you’ll most likely need to remove the .done()). However I can’t argue with the sentiment of crashing quickly is better then dealing with gremlins if you have unhandled rejections. For your tests, however, its not a good idea to terminate your promise chains in this way because Mocha will attach a .catch() block that will fail the test if execution fails into it. The promises returned in your test code will never be unhandled.

it('should pass', function () {
  return myPromise.doStuff()
    .then(() => {...});
});

So there you have it should you use Bluebird’s .done() API with Mocha?
I’d suggested letting Mocha handle the rejections vs. having .done() crash the process. At least that way you can complete your test run and view all errors instead of hunting down a process crash.

Until next time think imaginatively and design creatively