New feature: setting time limits

[vote539]

In my application, I needed an API that let me set a "time limit" on an asynchronous function. If the asynchronous function does not call its callback within the time limit, then the callback should be called with some default arguments.

A real-life use case would be, for example, loading content from a URL. Maybe you want to wait only 30 seconds for the content to return, and return some error if the content hasn't loaded in time.

Here's what I came up with:

async.timeLimit = function(milliseconds, defaults, callback) {
    var calledNext = false;
    var timer = null;

    var normalCallback = function() {
        if (!calledNext) callback.apply(null, arguments);
        clearTimeout(timer);
    };
    var timeoutCallback = function() {
        callback.apply(null, defaults);
        calledNext = true;
    };

    timer = setTimeout(timeoutCallback, milliseconds);
    return normalCallback;
};

Here's how you use it:

function callback(err) {
    if (err) console.log(err);
    else console.log("Hello World");
}

// No time limit:
setTimeout(
    callback,
    3000);

// Normal callback wins:
setTimeout(
    async.timeLimit(5000, [new Error("Out of time")], callback),
    3000);

// Time limit wins:
setTimeout(
    async.timeLimit(1000, [new Error("Out of time")], callback),
    3000);

So in general, anywhere you would normally pass a callback as an argument, you can wrap that callback in async.timeLimit.

One possible issue with the above implementation is that the garbage collector will not be able to remove the reference to callback until the normal callback has been called, since the function normalCallback retains a reference to callback, and normalCallback is retained by the asynchronous function that hasn't finished yet. I came up with an alternate implementation that might be friendlier on the GC: when the timeout callback is resolved, the normal callback is replaced by a noop. I haven't run any profiles to test whether or not this is an improvement.

async.timeLimit = function(milliseconds, defaults, callback) {
    var timer, normalCallbackRef;

    var normalCallback = function() {
        callback.apply(null, arguments);
        clearTimeout(timer);
    };
    var timeoutCallback = function() {
        callback.apply(null, defaults);
        normalCallbackRef = function(){}; // noop
    };

    timer = setTimeout(timeoutCallback, milliseconds);
    normalCallbackRef = normalCallback;

    return function() {
        normalCallbackRef.apply(null, arguments);
    };
};

Any thoughts on async.timeLimit? I can submit a PR if you want.

[aearly]

This has been discussed before a bit. (#427 #587)

The ability to time-out an async function would be useful -- it has been requested a few times before. Rather than giving the callback itself a timeout, I'd prefer to see a higher-order function that takes in the whole async function and returns a version that will timeout. This way, the timeout can start when the function is invoked, rather than be started when you define the callback. If you don't have access to the individual callback (e.g. when using an async library method) it would make it much less elegant to use.

Example interface:

var timeoutAsyncFn = async.timeout(60000,  function asyncFn(args, callback) {
  //...
});

//example
async.each(items, timeoutAsyncFn, done);

[vote539]

I see the utility in both approaches. For async.each, wrapping the async function produces a good API. If I correctly understand async.timeout, this is how it would work with async.waterfall:

// with async.timeout (aearly)
// (added default arguments array as second parameter for comparison)
async.waterfall([
    (next) => {
        async.timeout(1000, [new Error("time's up")], fs.readFile)("foo.txt", next);
    },
    (data, next) => {
        // ...
    }
], (err) => { /* ... */ });

// with async.timeLimit (OP)
async.waterfall([
    (next) => {
        fs.readFile("foo.txt", async.timeLimit(1000, [new Error("time's up")], next));
    },
    (data, next) => {
        // ...
    }
], (err) => { /* ... */ });

There's also the case when the callback function is not well-defined. For example, consider a function that starts by looking something like this:

async.waterfall([
    (next) => {
        var id = uuid.v4();
        socket.emit("ping", id);
        socket.on("pong", function pongCb(_id) {
            if (_id === id) {
                next();
                socket.removeListener("pong", pongCb);
            }
        });
    },
    // ...
], (err) => { /* ... */ });

The wrapped-callback API in the OP would be easy to add; it would look like this:

// with async.timeLimit (OP)
async.waterfall([
    (next) => {
        var _next = async.timeLimit(5000, [new Error("pong timeout")], next);
        var id = uuid.v4();
        socket.emit("ping", id);
        socket.on("pong", function pongCb(_id) {
            if (_id === id) {
                _next();
                socket.removeListener("pong", pongCb);
            }
        });
    },
    // ...
], (err) => { /* ... */ });

With async.timeout, you'd need to make a wrapper function, like this:

// with async.timeout (aearly)
async.waterfall([
    (next) => {
        async.timeout(5000, [new Error("pong timeout")], (_next) => {
            var id = uuid.v4();
            socket.emit("ping", id);
            socket.on("pong", function pongCb(_id) {
                if (_id === id) {
                    _next();
                    socket.removeListener("pong", pongCb);
                }
            });
        })(next);
    },
    // ...
], (err) => { /* ... */ });

It works either way. For the ping-pong example, I like how async.timeLimit results in less nesting, but I like async.timeout for the fs.readFile example. As you said, async.timeout has the rather important advantage that the timeout doesn't start until the function is invoked.

[aearly]

Closed via #1027 !