Why am I not able to access the result of multiple networking calls?

Question

In my Node app I am trying to get shipment data for packages. I need a way to get the json data and append it to an object or something so that I can pass it to my render page (using pug).

This is my code:

    var test;

    for(var i = 0; i < result.length; i++) {
        var currentNumber = result[i].trackingNumber;
        ups.track(currentNumber, function(err, tracking) {
            test += tracking
        });
    }

    res.send(result)

It seems that anything I do inside of the ups.track has no scope into the var test, and I can't think of a better way to do this. Any help would be appreciated.

Answer 1

First off, ups.track() is an asynchronous function. That means that it will call its callback some indeterminate time in the future. So, your for loop runs to its completion, launching result.length calls to ups.track() and then you do res.send(result) before any of them have completed, before any of them have called their callback.

Secondly, you aren't initializing your test variable so test += ... won't produce a good result.

So, you can't know the results until all of them have completed. There are various ways to do that.

Low-tech Counter

The "low-tech" way to do that is to keep a counter:

var test = "";

var cntr = 0;
for(var i = 0; i < result.length; i++) {
    var currentNumber = result[i].trackingNumber;
    ups.track(currentNumber, function(err, tracking) {
        test += tracking;
        // see if we are done with all the ups.track() calls yet
        if (++cntr === result.length) {
            res.send(test);
        }
    });
}

Using Promises

The higher-tech way is to use promises (this has larger benefits to learn for lots of other reasons). While promises show only modest benefits here, they provide huge benefits as soon as you have sequences of asynchronous operations you need to coordinate and they also make error handling (something you are ignoring in your code example) much simpler too.

First, you make a "promisified" version of ups.track() which is a function that returns a promise that resolves or rejects when ups.track() finishes.

function upsTrack(num) {
    return new Promise(function(resolve, reject) {
        ups.track(num, function(err, tracking) {
           err ? reject(err) : resolve(tracking);
        });
    });
}

Now, we'll use a combination of .map() to create an array or promises from all your calls to upsTrack() and Promise.all() to tell us when all those promises have completed.

Promise.all(result.map(function(item) {
    return upsTrack(item.trackingNumber);
})).then(function(allResults) {
    // allResults is an array of tracking results
    // process that array into your final result and send it
    res.send(allResults.join(","));
}).catch(function(err) {
    // handle error here
    res.status(500).end();
});

Note that this structure also gets rid of multiple variables defined in the previous low-tech way of doing this as there is no need for test, cntr or i any more. Looping structures and functions that process arrays or handle arrays of results take care of all that for us.

Using the Bluebird Promise library

And, you can take even a few more shortcuts by using a smarter promise library like Bluebird:

const Promise = require('bluebird');
ups = Promise.promisifyAll(ups);

Promise.map(result, function(item) {
    return ups.trackAsync(item.trackingNumber);
}).then(function(allResults) {
    // allResults is an array of tracking results
    // process that array into your final result and send it
    res.send(allResults.join(","));
}).catch(function(err) {
    // handle error here
    res.status(500).end();
});

The Bluebird promise library has two useful things here. It has a promisifyAll() method which will make promisified methods of all the ups object methods for you automatically and it also has Promise.map() which just combines array.map() and Promise.all() in one step (since it's a commonly used structure).

If you're interested in Promises and Promise libraries, you may find this useful:

Are there still reasons to use promise libraries like Q or BlueBird now that we have ES6 promises?

Answer 2

The first problem is that test is undefined at the start. That's bad. You want a default value of the same type you expect it to be var test = "";

The big problem, though, is that you are sending before any of those ups callbacks ever return.

You've fired off 18 (or whatever) requests to UPS, but you've returned result before the UPS server even got the first request, let alone completed them all.

Rather than show you the really hairy code that might be written to make things work in your loops, I'd recommend trying something like this:

const track = trackingNumber =>
  new Promise((resolve, reject) =>
    ups.track(trackingNumber, (err, data) =>
      err ? reject(err) : resolve(data)));

Promise.all(result.map(info => track(info.trackingNumber)))
  .then(trackingNumbers => trackingNumbers.join(''))
  .then(bigStringOfNumbers => res.send(bigStringOfNumbers));

Learning about Promises, Promise.all(), and [].map() is going to be a much more helpful path than trying to build out the complex callback version of this solution.