Passing data into node.js callbacks - the asynchronous issue

Question

Take the code below, in which an object data is initialized with some values that are then processed by some time-intensive function, like database access. If the function succeeds, the name of the successful data item is printed to the console. Otherwise, a failure notice is printed:

data = {first:  'someinfo',   second:  'somemoreinfo',   third:  'evenmoreinfo'};

for (var item in data) {
    timeIntensiveFunction(item, data[item], function(err) {
        if (!err) {
            console.log(item + ' processed successfully');
        } else {
            console.log(item + ' failed');
        }
    });
}

You would expect the console to show this, assuming the function succeeds for all three data items:

first processed successfully
second processed successfully
third processed successfully

Instead it will show this, assuming the first database access takes longer than the for loop:

third processed successfully
third processed successfully
third processed successfully

This is because the console logging is done in a callback, which would reasonably only be called after the for loop is done, because timeIntensiveFunction() takes so long. By the time the first callback is called, item already has its last value, third.

How do you pass the 'current' value of item into the callback?

Object keys in JavaScript are not ordered. Consider a simple ordered array like `["first", "second", "third"]` or one with objects `[{id: "first"}, {id: "second"}, {id: "third"}]`. — maček, Jul 23 '14 at 02:51
@maček Not what I was getting at. The problem is that regardless of the ordering, node will process the three `data` items in **some** order. Assuming `timeIntensiveFunction()` takes longer than the `for` loop to complete, all three console lines will show the last value of `item`, instead of the value evaluated by `timeIntensiveFunction()` — TheEnvironmentalist, Jul 23 '14 at 02:55
@go-oleg: Actually I think it is a valid duplicate. For-in vs for-loop doesn't make a difference, and with the OP already recognising that the callbacks are asynchronously invoked after the loop ended every necessary bit of information is there. — Bergi, Jul 23 '14 at 02:58

score 0 · Answer 1 · answered Jul 23 '14 at 02:52

0

Problem is because it's calling the callback with the last item only.

You can bind each of your item with a function like bellow.

var printStatus = function(item){
    return function(err) {
        if (!err) {
            console.log(item + ' processed successfully');
        } else {
            console.log(item + ' failed');
        }
    }
}

for (var item in data) {
    timeIntensiveFunction(item, data[item], printStatus(item));
}

answered Jul 23 '14 at 02:52

Mritunjay

25,338
7
55
68

This doesn't guarantee the items are processed sequentially – maček Jul 23 '14 at 02:53
@maček I think it gives, because it will call the function `printStatus` with respective `item` only. – Mritunjay Jul 23 '14 at 02:55
It guarantees that timeIntensiveFunction is called on the items sequentially, but makes no guarantees about the timing of the callbacks. But that's sort of the whole point of asynchronicity and callback functions in the first place. Also, this is essentially the same answer as mine, in a different style. – Mike Bell Jul 23 '14 at 02:58
If you wrote `timeIntensiveFunction` yourself, this helps, if you got it from a module, you're not going to want to wrap it in your own function – TheEnvironmentalist Jul 23 '14 at 02:58
@TheEnvironmentalist I think I didn't get your point, I am not wrapping `timeIntensiveFunction`. I am just wrapping my callback function which I'll define anyway, and the function `timeIntensiveFunction` needs a callback and it's getting that, I don't think it matters it's from a module or my own. – Mritunjay Jul 23 '14 at 03:03

score 0 · Answer 2 · answered Jul 23 '14 at 02:53

0

This is a common "gotcha" with closures in javascript. One way around it is to wrap your function call in an anonymous function and rescope item. Like so:

for (var item in data) {
    (function(item){
        timeIntensiveFunction(item, data[item], function(err) {
           if (!err) {
               console.log(item + ' processed successfully');
           } else {
               console.log(item + ' failed');
           }
        });
   })(item);
}

answered Jul 23 '14 at 02:53

Mike Bell

1,356
11
9

I thought about this, but it's messy, and I'd have wrappers for some twenty different functions – TheEnvironmentalist Jul 23 '14 at 03:03
You could always use a library like underscore (http://underscorejs.org/) or the .forEach syntax (if your javascript implementation supports it) to do the iteration and the callback function in one line instead of two. /shrug. That's probably about as good as you're gonna get. – Mike Bell Jul 23 '14 at 03:12

maček · Answer 3 · 2014-07-23T03:19:05.023

0

If you're looking for a library to make working with async tasks more easy, check out caolan/async.

var async = require("async");

var data = [{id: "first"}, {id: "second"}, {id: "third"}];

function timeIntensiveFunction(item, done) {
  // do something
  console.log("time intensive task started:", item.id);

  // err?
  // if (err) return done(err);

  done();
}

function processItem(item, done) {
  timeIntensiveFunction(item, function(err) {
    if (err) return done(err);
    console.log("task complete:", item.id);
    done();
  });
}

async.map(data, processItem);

Output

time intensive task started: first
task complete: first
time intensive task started: second
task complete: second
time intensive task started: third
task complete: third

For users looking to learn how to do this without a library, you can see the revision history of this answer.

edited Jul 23 '14 at 03:19

answered Jul 23 '14 at 02:58

maček

76,434
37
167
198

This defeats the purpose of the `for` loop – TheEnvironmentalist Jul 23 '14 at 03:01
Well that's kind of the point; a `for` loop cannot (easily) be used for asynchronous serial processing. – maček Jul 23 '14 at 03:03
If `async.series` doesn't suit you, perhaps you want to look into a Promises api and chain `.then` calls on your promises object. – maček Jul 23 '14 at 03:04
But this adds so much weight to an otherwise relatively simple process. There's a reason for the node.js `for` loop. The `data` object is coming from a database. I don't know what data `timeIntensiveFunction()` will operate on ahead of time, or even how many times it will run. – TheEnvironmentalist Jul 23 '14 at 03:06
@TheEnvironmentalist, added weight or not, you're failing to realize the complexity of your problem. The `for` loop does not exist to solve all problems. – maček Jul 23 '14 at 03:11
I realize the problem, I just think there's a better solution, even if I don't know it yet – TheEnvironmentalist Jul 23 '14 at 03:11
1

I mentioned two libraries that make it easier to deal with these things, and they'll clean up the syntax a lot. Look into `async` and `Promises`. I'm only going to provide a native JS solution. You can abstract it further and make it prettier from there. – maček Jul 23 '14 at 03:12
@TheEnvironmentalist, it seems like you don't want to learn how to do it or understand how it works, so I've modified my answer to just use the `async` library. – maček Jul 23 '14 at 03:20
I'm perfectly happy going along with whatever method is best, but I tend to prefer cleaner solutions, that's all. I'm not confused, I'm just a stickler for clean code :-) – TheEnvironmentalist Jul 23 '14 at 03:23
I'm not very familiar with `async`. In the console example you give, the tasks are completed in order. Is this enforced by `async` or could the next task have started before the last one completed? – TheEnvironmentalist Jul 23 '14 at 03:29
@TheEnvironmentalist, the original code I gave you in my example could've been abstracted to be as clean as you wanted. Anyway, the `async` lib lets you decide how you want your items processed. `async.parallel` will starts all tasks simultaneously. `async.series` will wait for 1 to be complete before 2 begins. The lib has over 9,000 followers and you can bet even more people are using it; it's considered the go-to for this kind of task in nodejs. Read through the documentation to answer more of your questions. – maček Jul 23 '14 at 03:35

Passing data into node.js callbacks - the asynchronous issue

3 Answers3