7

I've got an api call that sometimes returns paged responses. I'd like to automatically add these to my promises so I get the callback once all the data has arrived.

This is my attempt. I'd expect the new promise to be added and Promise.all to resolve once that is done.

What actually happens is that Promise.all doesn't wait for the second request. My guess is that Promise.all attaches "listeners" when it's called.

Is there a way to "reintialize" Promise.all()?

function testCase (urls, callback) {
    var promises = [];
    $.each(urls, function (k, v) {
        promises.push(new Promise(function(resolve, reject) {
            $.get(v, function(response) {
                if (response.meta && response.meta.next) {
                    promises.push(new Promise(function (resolve, reject) {
                        $.get(v + '&offset=' + response.meta.next, function (response) {
                            resolve(response);
                        });
                    }));
                }
                resolve(response);
            }).fail(function(e) {reject(e)});
        }));
    });

    Promise.all(promises).then(function (data) {
        var response = {resource: []};
        $.each(data, function (i, v) {
            response.resource = response.resource.concat(v.resource);
        });
        callback(response);
    }).catch(function (e) {
        console.log(e);
    });
}   

Desired flow is something like:

  1. Create a set of promises.
  2. Some of the promises spawn more promises.
  3. Once all the initial promises and spawned promises resolve, call the callback.
Vikas Sardana
  • 1,593
  • 2
  • 18
  • 37
Josiah
  • 3,008
  • 1
  • 34
  • 45
  • Where are you initializing `promises`? I see you pushing to it, but I don't see you creating it. – T.J. Crowder Feb 13 '17 at 17:46
  • What's `url`? (If it's an array, normally that would be a plural, e.g., `urls`.) – T.J. Crowder Feb 13 '17 at 17:47
  • Why `response.resource = response.resource.concat(v.resource);`? That creates a whole new array each time...? – T.J. Crowder Feb 13 '17 at 17:48
  • @T.J.Crowder - thanks for the catches. I've cleaned up the test case a bit. This isn't my production code, just demonstrates the problem. – Josiah Feb 13 '17 at 17:49
  • If a response has `response.meta.next`, do you want *both* that original response *and* the "next" response in the result? – T.J. Crowder Feb 13 '17 at 17:52
  • Yes, I want both responses as one large response. That's why I'm concatenating the two (or more) arrays. – Josiah Feb 13 '17 at 17:53
  • But that's later, and on the responses to `urls`, not the `response.meta.next` result. – T.J. Crowder Feb 13 '17 at 17:54
  • @T.J.Crowder My objective is to handle the promises for response.meta.next and urls in the same Promise resolver. That's why I'm trying to add a promise to `Promises.all`. – Josiah Feb 13 '17 at 18:01
  • This seems to be a duplicate of this question: [How to know when all Promises are Resolved in a dynamic “iterable” parameter?](http://stackoverflow.com/q/37801654/1426891). Marking as a dupe; I put one potential solution in [my answer here](http://stackoverflow.com/a/37819138/1426891). – Jeff Bowman Feb 13 '17 at 20:08
  • @JeffBowman - that seems to be asking the same thing. I like my title better though. – Josiah Feb 13 '17 at 20:14
  • I like yours better too; the goal is simply that both questions point to a canonical series of answers useful for both. Marking as a "dupe" still lets your question be upvoted, too. – Jeff Bowman Feb 13 '17 at 20:16

2 Answers2

5

It looks like the overall goal is:

  1. For each entry in urls, call $.get and wait for it to complete.
    • If it returns just a response without "next", keep that one response
    • If it returns a response with a "next," we want to request the "next" as well and then keep both of them.
  2. Call the callback with response when all of the work is done.

I would change #2 so you just return the promise and fulfill it with response.

A key thing about promises is that then returns a new promise, which will be resolved based on what you return: if you return a non-thenable value, the promise is fulfilled with that value; if you return a thenable, the promise is resolved to the thenable you return. That means that if you have a source of promises ($.get, in this case), you almost never need to use new Promise; just use the promises you create with then. (And catch.)

(If the term "thenable" isn't familiar, or you're not clear on the distinction between "fulfill" and "resolve", I go into promise terminology in this post on my blog.)

See comments:

function testCase(urls) {
    // Return a promise that will be settled when the various `$.get` calls are
    // done.
    return Promise.all(urls.map(function(url) {
        // Return a promise for this `$.get`.
        return $.get(url)
            .then(function(response) {
                if (response.meta && response.meta.next) {
                    // This `$.get` has a "next", so return a promise waiting
                    // for the "next" which we ultimately fulfill (via `return`)
                    // with an array with both the original response and the
                    // "next". Note that by returning a thenable, we resolve the
                    // promise created by `then` to the thenable we return.
                    return $.get(url + "&offset=" + response.meta.next)
                        .then(function(nextResponse) {
                            return [response, nextResponse];
                        });
                } else {
                    // This `$.get` didn't have a "next", so resolve this promise
                    // directly (via `return`) with an array (to be consistent
                    // with the above) with just the one response in it. Since
                    // what we're returning isn't thenable, the promise `then`
                    // returns is resolved with it.
                    return [response];
                }
            });
    })).then(function(responses) {
        // `responses` is now an array of arrays, where some of those will be one
        // entry long, and others will be two (original response and next).
        // Flatten it, and return it, which will settle he overall promise with
        // the flattened array.
        var flat = [];
        responses.forEach(function(responseArray) {
            // Push all promises from `responseArray` into `flat`.
            flat.push.apply(flat, responseArray);
        });
        return flat;
    });
}

Note how we never use catch there; we defer error handling to the caller.

Usage:

testCase(["url1", "url2", "etc."])
    .then(function(responses) {
        // Use `responses` here
    })
    .catch(function(error) {
        // Handle error here
    });

The testCase function looks really long, but that's just because of the comments. Here it is without them:

function testCase(urls) {
    return Promise.all(urls.map(function(url) {
        return $.get(url)
            .then(function(response) {
                if (response.meta && response.meta.next) {
                    return $.get(url + "&offset=" + response.meta.next)
                        .then(function(nextResponse) {
                            return [response, nextResponse];
                        });
                } else {
                    return [response];
                }
            });
    })).then(function(responses) {
        var flat = [];
        responses.forEach(function(responseArray) {
            flat.push.apply(flat, responseArray);
        });
        return flat;
    });
}

...and it'd be even more concise if we were using ES2015's arrow functions. :-)


In a comment you've asked:

Could this handle if there was a next next? Like a page 3 of results?

We can do that by encapsulating that logic into a function we use instead of $.get, which we can use recursively:

function getToEnd(url, target, offset) {
    // If we don't have a target array to fill in yet, create it
    if (!target) {
        target = [];
    }
    return $.get(url + (offset ? "&offset=" + offset : ""))
        .then(function(response) {
            target.push(response);
            if (response.meta && response.meta.next) {
                // Keep going, recursively
                return getToEnd(url, target, response.meta.next);
            } else {
                // Done, return the target
                return target;
            }
        });
}

Then our main testCase is simpler:

function testCase(urls) {
    return Promise.all(urls.map(function(url) {
        return getToEnd(url);
    })).then(function(responses) {
        var flat = [];
        responses.forEach(function(responseArray) {
            flat.push.apply(flat, responseArray);
        });
        return flat;
    });
}
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Could this handle if there was a next `next`? Like a page 3 of results? – Josiah Feb 13 '17 at 18:18
  • @Josiah: It *could*, yes. We'd have to change the logic. We'd probably want to have a function that handled a given URL, and returned a promise that would be resolved with an array of responses of all of the "nexts", and use it recursively. You'd want to pass it a URL and an offset or similar. – T.J. Crowder Feb 13 '17 at 18:20
  • @Josiah: Turns out to be quite simple to do that, I've added it to the end of the answer. – T.J. Crowder Feb 13 '17 at 18:37
  • In my old non-promisey version I was using a recursive function to fetch the information. I thought I could do it using promise-generating callbacks, but I think this is probably cleaner. Thanks very much! – Josiah Feb 13 '17 at 20:08
  • @Josiah: No worries! `getToEnd` is recursive, note. :-) – T.J. Crowder Feb 14 '17 at 07:51
1

Assuming you are using jQuery v3+ you can use the promises returned by $.ajax to pass to Promise.all().

What you are missing is returning the second request as a promise instead of trying to push it to the promises array

Simplified example

var promises = urls.map(function(url) {
  // return promise returned by `$.ajax`
  return $.get(url).then(function(response) {
    if (response.meta) {
      // return a new promise
      return $.get('special-data.json').then(function(innerResponse) {
        // return innerResponse to resolve promise chain
        return innerResponse;
      });

    } else {
      // or resolve with first response
      return response;
    }
  });

})

Promise.all(promises).then(function(data) {
  console.dir(data)
}).catch(function(e) {
  console.log(e);
});

DEMO

charlietfl
  • 170,828
  • 13
  • 121
  • 150