unhandled promise rejection in async tests

[tswaters]

I can't seem to get this to work. If a routine under test uses promises at some point and catches any errors returning the result to the callback function, it's not possible to throw assertion exceptions in the test code without resulting in an unhandled promise rejection and the test timing out

Consider the following:

const assert = require('assert');

function test (cb) {
    new Promise((resolve, reject) => reject(new Error('aw snap!')))
        .then(result => cb(null, result))
        .catch(err => cb(err));
}

describe('test', () => {
    it('should function properly', done => {
        test(err => {
            assert.equal(err.message, 'oops!');
            done();
        });
    });
});

Results in:

$ mocha 


  test
(node:15999) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): AssertionError: 'aw snap!' == 'oops!'
^CTerminated

I don't understand why the assert.equal throw isn't handled - the only promise in play here is in the test function and it is handled and returns the error to the first parameter of cb properly. I would call catch on the promise in the test code, but as far as I can tell there is no promise to attach a catch to!

To get this to work, I need to wrap assertion code in a try/catch and call done with the error if one is encountered.... which is not ideal.

    it('should function properly', done => {
        test(err => {
            try {
                assert.equal(err.message, 'oops!');
            } catch (e) {
              return done(e);
            }
            done();
        });
    });

Or, attach a event handler for unhandledRejection on process which, again, is not ideal

process.on('unhandledRejection', e => { throw e; });

Is there a reason mocha doesn't do this itself, i.e.:

diff --git a/lib/runner.js b/lib/runner.js
index b024e0d..21c56f9 100644
--- a/lib/runner.js
+++ b/lib/runner.js
@@ -827,6 +827,7 @@ Runner.prototype.run = function (fn) {
 
   // uncaught exception
   process.on('uncaughtException', uncaught);
+  process.on('unhandledRejection', uncaught);
 
   if (this._delay) {
     // for reporters, I guess.

With that modified directly in the node_modules directory everything works as expected.

[ScottFreeCode]

We have #2640 for the unhandled rejection detection, for what it's worth. (Closing this as a duplicate of that as far as resolution on Mocha's end, but feel free to discuss your particular case here further as needed.)

However, the caveat I mentioned there applies here: the function named test in your example will swallow any errors thrown in the error-handling path by the callback passed to handle errors from the function itself, which is potentially problematic in real uses and not just in tests. Possible fixes include not using promises internally in such functions, or (if you can change their API) making them return the promise, or (if you are ok with swallowing errors in the callback as long as you know not to let it happen) documenting that the callback's error-handling path MUST NOT throw and adding try-catch to the test as shown here. (There may be other solutions, but I'm not sure if there are any other good ones.)

---

To address this specific case in more detail:

I don't understand why the assert.equal throw isn't handled - the only promise in play here is in the test function and it is handled and returns the error to the first parameter of cb properly.

The reject(...) error is being handled by being passed to the cb. The cb is the little function passed to test in your test (it), which is what's throwing the assertion. So you're throwing out of the catch handler and not handling that. It's equivalent to if you inlined the code thus:

const assert = require('assert');

describe('test', () => {
    it('should function properly', done => {
        new Promise((resolve, reject) => reject(new Error('aw snap!')))
            //.then will never be called from the rejected promise
            .catch(err => { // this handles the `new Error` rejection above
                assert.equal(err.message, 'oops!'); // this may throw
                done();
            }); // this promise may now be rejected with an assertion error thrown out of the catch block
        // because promise is neither returned nor used further, if it ended up rejected then that's unhandled
    });
});

I would call catch on the promise in the test code, but as far as I can tell there is no promise to attach a catch to!

Well, there is, it's just never leaving the function in which it's created... which at least partly defeats the purpose of using promises instead of callbacks: they're intended to give us back the ability to handle errors in the caller's scope (among other design patterns from synchronous programming).

---

A couple unrelated promise programming tips:

• new Promise((resolve, reject) => reject(<error>)) can be simplified to Promise.reject(<error>) and new Promise((resolve, reject) => resolve(<value>)) can be simplified to Promise.resolve(<value>)
• .catch(error => <function>(error)) can be simplified to .catch(<function>)

[tswaters]

Thank you for the very detailed explanation! It completely makes sense that calling cb in catch block is still technically in the promise and any exceptions thrown there go unhandled.

This came up refactoring an existing callback-based service to integrate with some new promise-based code. I think for now I'll keep the unhandledRejection handler there in the test code and make absolutely sure the code path after cb can't throw.

I suppose it would make sense to rework the entire code path to use promises... no half measures, Walt

[martin-ayvazyan13]

Hello,

Could you please let me know how can I implement cases when is expected that promise should be rejected and contain 'my_message' message?

[martin-ayvazyan13]

function(args).catch((err) => {
expect(err.message).to.equal('expected');
})
it's not what I want, as I also need to require that the promise should be rejected.

[plroebuck]

@martin-ayvazyan13, please open a new issue...