Node.js can just exit with 0 exit code when all timers mocked as per new default

[dominykas]

We understand you have a problem and are in a hurry, but please provide us with some info to make it much more likely for your issue to be understood, worked on and resolved quickly.

• FakeTimers version : v13.x
• Environment : Node.js
• Example URL : n/a
• Other libraries you are using: n/a

What did you expect to happen?

Node.js should hang as there are still tasks to be performed.

What actually happens

Node.js exits, as the queueMicrotask is now mocked out and so the pending callback is not visible to Node.js.

Moreover, Node.js exits with 0 as exit code - which, in the case of tests (where fake timers are most likely to be used) means that "test passed" (which is not the case here).

How to reproduce

require('@sinonjs/fake-timers').install();

new Promise((resolve, reject) => {
  setTimeout(() => {
    console.log('here');
    resolve();
  }, 1000);
});

This is a very simplified example, and one could tack on clock.tickAsync(1000) at the end to get the here to print, however I do have a real life use case which involves an http client/server inside a real test runner and Node.js just exits as it finds no more tasks to perform (I'll see if I can build up a more realistic example).

[fatso83]

Hi, @dominykas. Sorry for leaving you with nothing for so long, but I was hoping Simen might weigh in on these questions. I also assumed you understood why the mocking of queueMicrotask was happening, but I never actually verified if you did, and I now see that there is nothing that indicates that you did. Sorry for not seeing this before, as I somehow assumed this was implicitly a request to change the breaking behavior.

The reason you are seeing this is a breaking change in version 13, which is written in the changelog. Every major version implies a breaking change, so when bumping it's wise to check out the changelog for what changed. The docs on this sub-project of Sinon is a bit sparse, though, so the mother project has a more thorough migration guide which says a lot more about what to expect (although never enough, it seems 😄 ).

Anyway, the relevant change in 13.0.0 is that we fake all timers by default. Previously, nextTick and queueMicrotask was not faked. To restore the previous behavior, you just explicitly list the ones you want to fake in the config: toFake: ['Date', 'setTimeout'], so this is basically undoing #126 .

I am also pondering reverting this change, as this is obviously a nuisance. Feel free to join the discussion : sinonjs/sinon#2620 (comment)

[dominykas]

No worries, thanks for you response. I understand why this was introduced and the release notes also made that clear.

The reason I raised the issue is that the current behavior is surprising and unintuitive. Reverting is an option, but to address this particular issue, that is not necessary, for example we could also make sure that when a faked microtask is queued, a real microtask is also queued to prevent Node.js from exiting (I believe all it takes is to create a real promise, and resolve that promise when all microtasks are cleared or the context is reset? Just thinking out loud).

Another option would be to introduce a new way of selecting which things to mock - so that you could e.g. mock "everything except queueMicrotask)

[fatso83]

Yeah, I was wondering an exclude mechanism in the API, as well.

I am not totally sure what to do next, TBH. There are issues regardless.

[benjamingr]

Node has never prevented exiting because of pending microtasks or promises - this is somewhat surprising but was always the behavior.

I do see the merit of scheduling something "in the background" to keep Node alive but this isn't particularly new behavior (you're just seeing it with queueMicrotask).

If a (non mocked) queueMicrotask keeps Node alive that's probably a bug (since a promise queueing a microtask for example does not)

[dominykas]

I was specifically digging into the process exiting without waiting for https://github.com/hapijs/lab/blob/master/lib/runner.js#L112 to complete when queueMicrotask is faked. That was of course in a complex test which also had an HTTP server, etc inside.

Quite possibly my example here is incorrect - I'll see if I can revisit it with a more representative example.