Ava: Introduce t.throwsUncaughtException to assert the code is propagating the error globally

[zebateira]

So I have this use case of a nodeify function (similar to bluebird's nodeify) that helps users support both callback style and promises on APIs.
I need that setImmediate on line 16 to prevent the case where the callback throws so the error gets thrown globally.

The only way I can see to test this is to actually listen to the uncaughtException process event: https://github.com/IndigoUnited/js-promtie/blob/master/test/spec/nodeify.js#L47 . Although I also agree that this is not the best way to do it, so if you guys have any suggestion that would be awesome.

AVA adds its own uncaughtException listener so I can't do this that easy.
ATM I am removing that event listener and restoring it inside my event handler, which isn't that great.

I'm a beginner at AVA, so I don't quite see how this t.throwsUncaughtException could be implemented. Maybe doing something like this:

// ava/assert.js
x.throwsUncaughtException = function (handler) {
    process.on('uncaughtException', function tempHandler(error) {
        handler(error);
        process.removeListener('uncaughtException', tempHandler);
    });
};

(But this is doing almost exactly what I am doing and it feels just as hack-ish).

[vadimdemedes]

I don't think we'd want to have that method in AVA core, as it belongs to a very very specific use case. I'd suggest either using the helper you came up with or using child_process.exec and see if the process crashes with an error.

Thanks for taking the time to suggest a new idea, but I don't think it fits in AVA. I'll let other people speak up, but it's -1 from me.

[novemberborn]

Throwing uncaught exceptions can occasionally be useful when writing application code (_not_ small modules). I don't think we should have an assertion for this but we could conceivably make our uncaughtException handler extensible. Especially since it's too hard to mess with such handlers once they've been set up.

Something like this:

import test, { uncaught } from 'ava'

test.serial(async t => {
  doTheThing()

  const err = await uncaught.catchNext()
  t.true(err.message === 'The one I was waiting for')
})

The .serial modifier is needed so you can be sure to catch exceptions from this test. catchNext() would return a promise that is fulfilled with the first argument to the next uncaughtException event. AVA won't send this uncaught exception to the main process so the test keeps running. If no exception occurs the test will time out.

Using child processes is also possible but it'll be hard to serialize the exception and run assertions on it.

[jamestalmage]

:-1:

The Node.js documentation pretty clearly states that uncaughtExceptions are a bad idea, and "last resort".

If you follow the documentations advice, using uncaughtException handlers only to "perform synchronous cleanup of allocated resources (e.g. file descriptors, handles, etc) before shutting down the process", then you may want to test that. But it is probably better to build testable abstractions into your code then attempting to install that handler inside a test runner.

As for asserting some code will throw an uncaught exception, I would recommend figuring out a way to abstract your code so you call the throwing code synchronously from your tests. (Use of lolex or mocking event emitters cover most use cases).

[jamestalmage]

@carsy - I looked at your example. lolex should make it pretty easy to test nodeify.

test(t => {
  let clock;
  try {
    clock = lolex.install();  
    const wrapped = nodeify(function () {
      throw new Error('foo');
    });
    wrapped();
    t.throws(() => clock.tick(), 'foo');
  } finally {
    if (clock) {
      clock.uninstall();
    }
  }
});

[zebateira]

@jamestalmage oh awesome! thanks for the lolex tip.

I'm closing this now, unless @novemberborn still wants to progress his idea forward.
IMO @jamestalmage made good points that dismiss the need to add any kind of assertion that would allow to assert uncaughtException handler code.

[novemberborn]

Nah, no need to do complicated stuff while there are better approaches.