await/awaitAll called before pending tasks are complete after a call to queue.abort()

[markrian]

This behaviour isn't explicitly documented, I don't think, but it's not what I'd expect.

So, for instance:

var queue = require("d3-queue").queue;

function longRunningTask(done) {
    setTimeout(function () {
        console.log('task finished')
        done(null);
    }, 1000);
}

var q = queue(1)
    .defer(longRunningTask)
    .defer(longRunningTask)
    .await(function (error) {
        console.log('all done');
    });

setTimeout(function () {
    q.abort();
});

I would expect this to eventually log these lines:

task finished
all done

but it actually logs:

all done
task finished

In my use case, I cannot define abort functions on my tasks, in case you suggest that I just provide those.

[markrian]

To clarify why I find this behaviour surprising, imagine that I'd want to put do some clean up in await/awaitAll that is only worth running when all async tasks are truly finished.

[mbostock]

This is the expected behavior.

You’ve defined a queue with a concurrency of 1. Thus, when you first call queue.defer, it starts your first longRunningTask. However, the second call to queue.defer does not start the second task because the first task is still running, and 2 > 1. Your tasks are not abortable, so once they start, there is no way to cancel them.

When you then abort the queue, the second task is still not started, but the await callback is immediately invoked with an error. (Per the queue.abort documentation: “invokes the queue.await or queue.awaitAll callback with an error indicating that the queue was aborted.”) You’re ignoring this error, but if you only want to do something if your tasks complete (successfully), then you need to check for the error instead, like this:

q.await(function(error) {
  if (error) throw error;
  console.log("all done");
});

Then you’d see this output:

Error: abort
    at Object.q.abort (…/node_modules/d3-queue/build/d3-queue.js:90:34)
    at null._onTimeout (repl:1:27)
    at Timer.listOnTimeout (timers.js:92:15)
task finished

Again, the only way to avoid the “task finished” message is to make your tasks abortable, and to abort the queue before they finish; otherwise there is no way of aborting them, short of terminating the entire process.

If you want to “abort” the queue in the sense of letting any running tasks finish without letting any further pending tasks start, well, that’s not what queue.abort does. But I suppose you could set a sentinel value that your longRunningTask could check on start, and they could immediately invoke the callback with a null result rather than doing any actual work.

[mbostock]

I’ve expanded the documentation for queue.abort.

I could maybe imagine a queue.cancel method which lets running tasks complete but doesn’t allow pending tasks to start. Something like this:

cancel: function() {
  if (error == null) waiting = NaN;
  return q;
}

But I haven’t tested it.

[markrian]

That cancel method sounds like what I'm after. Thanks for considering it!