setTimeout considered harmful

Date: August 4, 2015

setTimeout - Steve's Tweet

Recently I learned the hard way about setTimeouts side effects.

101 setTimeout issue

Let's say we have a following piece of code:

var someVar = 0;

var changeVar = function () {
    someVar = 10;
};

And unit test:

QUnit.asyncTest('should change variable to 10', function () {
    // Arrange
    someVar = 0;

    // Act
    changeVar();

    // Assert
    equals(someVar, 10);
});

Of course test passes, and we are happy.

Then, after some time, because of a reason (e.g., we want to fix some bug), we are adding setTimeout to changeVar function:

var changeVar = function () {
    setTimeout(function () {
        someVar = 10;
    }, 10);
};

Unit test does not pass anymore. So we are adding setTimeout to our unit test as well (ideally: appropriately longer than one in changeVar function to avoid confusion!):

QUnit.asyncTest('should change variable to 10', function () {
    // Arrange
    someVar = 0;

    // Act
    changeVar();

    // Assert
    setTimeout(function () {
        equals(someVar, 10);
    }, 20);
});

Then, we are introducing change to our code. Update only when someVar is not zero. We update function, and test accordingly:

var changeVar = function () {
    setTimeout(function () {
        if (someVar !== 0) {
            someVar = 10;
        }
    }, 10);
};

QUnit.asyncTest('should change variable to 10 if someVar != 0', function () {
    // Arrange
    someVar = 0;

    // Act
    changeVar();

    // Assert
    setTimeout(function () {
        equal(someVar, 0);
    }, 20);
});

Everything works. Great! Then, somebody else is fixing another bug - of course by increasing timeout:

var changeVar = function () {
    setTimeout(function () {
        if (someVar !== 0) {
            someVar = 10;
        }
    }, 50);
};

Still works, but when after some time we decide to change our logic:

var changeVar = function () {
    setTimeout(function () {
        if (someVar === 0) {
            someVar = 10;
        }
    }, 50);
};

Our test is still passing...when it shouldn't!

Of course all of this is happening in large codebase with more complex logic.

But this is not that bad. Let's take a look at more interesting scenario.

More complex case

We are in worst situation when we have waterfall of setTimeouts.

var fun1 = function () {
    setTimeout(function () {
        someVar++;
    }, 10);
};

var fun2 = function () {
    setTimeout(function () {
        fun1();
        someVar = 10;
    }, 10);
};

Guess if this unit test will pass:

QUnit.asyncTest('should change variable to 11', function() {
    // Arrange
    someVar = 0;

    // Act
    fun2();

    // Assert
    setTimeout(function () {
    	equal(someVar, 11);
    	start();
    }, 20);
});

You think it should? You are wrong!

But this test will pass:

QUnit.asyncTest('should change variable to 11', function() {
    // Arrange
    someVar = 0;

    // Act
    fun2();

    // Assert
    setTimeout(function () {
    	equal(someVar, 11);
    	start();
    }, 21);
});

Do you see the difference? Yes, timeout is 21 instead of 20. And of course everything will fall apart if somebody increase timeout in fun1 or fun2.

Solution

The best solution is not to use setTimeout at all. Unfortunately we need async operations sometimes. In this case - you should use promises if possible. Unfortunately World is not perfect, especially Web Development World, and sometimes you have to use setTimeout (e.g., for UI effects etc.). You may think that if you set long enough timeout in your unit tests, everything should be good, right? Well...remember that it will make your unit tests slower. Instead - you should use polling approach.

To apply it to the last example - using "Without Deferreds" approach - copy poll function to your codebase:

function poll(fn, callback, errback, timeout, interval) {
    var endTime = Number(new Date()) + (timeout || 2000);
    interval = interval || 100;

    (function p() {
            // If the condition is met, we're done! 
            if(fn()) {
                callback();
            }
            // If the condition isn't met but the timeout hasn't elapsed, go again
            else if (Number(new Date()) < endTime) {
                setTimeout(p, interval);
            }
            // Didn't match and too much time, reject!
            else {
                errback(new Error('timed out for ' + fn + ': ' + arguments));
            }
    })();
}

And call it in your unit test:

QUnit.asyncTest('should change variable to 11', function() {
    // Arrange
    someVar = 0;

    // Act
    fun2();

    // Assert
    poll(
	    function() {
	        return someVar === 11;
	    },
	    function() {
	        ok(true);
	        start();
	    },
	    function() {
	        ok(false);
	        start();
	    }
	);
});

Now, you do not have to guess what value will be appropriate for setTimeout delay, and you will speed up your tests as well.

When I see some strange behavior in code, the first thing I am looking at are setTimeout calls.

Tags: programming