Is Async somehow dependent on timers ?

[danielkcz]

I have run into rather weird issue. I am using SinonJS to write my unit tests. There is some neat utility to fake timers. Today I was totally stumbled when my tests became somewhat "stuck". I run them and nothing happened, process was just hanging there and doing nothing.

I started digging and find out that if I don't use fake timers, it works, but since I need to control time in my tests, I started digging more and found out that Async is probably forcing asynchronous behavior with real timers. Check out this JSFiddle.

It's rather innocent piece of code that is not even asynchronous and you can see resulting message in console. However once you remove comment on first line with fake timers, it doesn't finish.

For now I had to get rid of waterfall and it started working as expected. I suppose that nothing can be done about this :(

[aearly]

Yes, async uses timers in the browser (basically wherever process.nextTick and setImmediate aren't available). This is mostly setTimeout(iterate, 0) to prevent stack overflows in certain cases.

[danielkcz]

Well, I am running these tests in NodeJS. Are you sure about using process.nextTick? This one is not faked by Sinon and I am using it directly in my tests to fake asynchronous behavior. It doesn't get stuck there.

Edit: Yeah, just verified, waterfall especially is using async.setImmediate which is at the end faked by Sinon.

if (typeof setImmediate !== 'undefined') {
    async.setImmediate = function (fn) {
        // not a direct alias for IE10 compatibility
        setImmediate(fn);
   };
}

I don't fully understand why it is done this way, but it goes through this path during tests because setImmediateexists in NodeJS. Thus once I call sinon.useFakeTimers, it replaces the global function setImmediate function.If it would be referencing setImmediate directly, Sinon could not fake it for this.

[rlidwka]

Isn't it sinon issue?

async can't and shouldn't assume that environment could be changed.

[danielkcz]

How is that sinon issue ? It has access only to the global setImmediate. How it could do anything about it?

Also I would like to save some headache to other people who run into this. It's really hard to debug since the code just stops doing anything. Even if the Sinon would have included some sort of option to skip faking setImmediate, it would be really hard for other people to realize, that they need this without wondering why the code just stopped.

The solution above is rather weird and I cannot seem to understand what is different in IE10 that setImmediate cannot be aliased directly. Can you please elaborate on that ?

[rlidwka]

@FredyC , did you try to add clock.tick(10) at the end of that script like it's described in SinonJS docs?

[danielkcz]

@rlidwka Of course that works, but that's not the point. Read my previous post again, especially second paragraph please.

[rlidwka]

Let me rephrase... You're using a framework to delay timers, and complaining that timers are delayed. Correct?

[danielkcz]

It's not really about delay, but to control time during tests. Since my code is working with Date.now() I need predictable way how to write those tests. I cannot rely upon speed of clock during test. Unfortunately setImmediate got in the way because of its weird "workaround" which doesn't make any sense.

[danielkcz]

And to be more specific...it's not about me and my issues, not at all. I am just trying to save other people time if they run into same issue as me. Can you tell my what you don't like about my solution?

Actually my fix should even improve performance, because this way it looks for setImmediateRef only one closure up. While with the current solution it has to go through all the layers up to global object to find that function.

[rlidwka]

setImmediate is actually a recommended way of deferring callback in asynchronous api. "recommended" means a lot of libraries use it. Thus, "fixing" it here doesn't really solve anything.

If you think it's a bug, it's probably worth to bring it up on Sinon issue tracker. Maybe avoid replacing setImmediate or something like that.

[danielkcz]

Ok, I give up, you are just not listening and just avoiding any explanation to my questions. Thanks for your time.

[danielkcz]

Btw, what do you mean it doesn't solve anything? It fixes exactly that issue I have said, that code doesn't get stuck no matter what. No fix to Sinon can do that, this is just the issue of Async solution that calls current global setImmediate instead of calling native one.

[jadams74]

FreddyC's comment " I am just trying to save other people time if they run into same issue as me." was right on the money.

This issue just cost us a pile of time. Some of our Mocha tests written some time ago by devs on our team use sinon to manipulate the date much like FredyC describes. This caused our services that use Async.Auto() to fail on the final callbacks - but only while being run via Mocha. This took us quite a while to chase down.

This is the same issue here that was closed with no dialog: #106

This Sinon timer thing might be called out as a known issue in the main readme or some other documentation if no code fix is possible.

[danielkcz]

There is a fix and I have made pull request, but it was basically denied without explanation why it's bad solution... I hope this proves my point that it needs some kind of solution, not just ignorance.

[aearly]

Sounds like the fix is to do var tick = setImmediate in our init, as the sinon devs recommend.

[danielkcz]

Well it took quite a while to accept this solution. Thank you!