6

I'm in a scenario where I have to get data from the server in parts in sequence, and I would like to do that with the help of Promises. This is what I've tried so far:

function getDataFromServer() {
  return new Promise(function(resolve, reject) {
    var result = [];

    (function fetchData(nextPageToken) {

      server.getData(nextPageToken).then(function(response) {
        result.push(response.data);
        if (response.nextPageToken) {
          fetchData(response.nextPageToken);
        } else {
          resolve(result);
        }
      });

    })(null);

  });
}

getDataFromServer().then(function(result) {
  console.log(result);
});

The first fetch is successful, but subsequent calls to server.getData() does not run. I presume that it has to do with that the first then() is not fulfilled. How should I mitigate this problem?

Tholle
  • 108,070
  • 19
  • 198
  • 189
  • so, console.log(result); is not called / doesn't print anything? – Joel Jul 22 '15 at 22:25
  • @Joel Exactly. If I were to log inside the `getData().then()`, I would get the response for the first part only, not the subsequent ones. – Tholle Jul 22 '15 at 22:29
  • The code looks like it should work. Try adding a second function to your call to 'then' to handle error conditions (at least logging the error for now). I suspect that the second call to server.getData is silently failing. You'll probably need to handle for these types of errors, anyway. – Nimrand Jul 22 '15 at 22:44
  • What's the first "response.nextPageToken" ? Could it be 0 ? In which case, it would stop immediately? (but it would mean the promise is resolved, which you say is not) ; I tried similar code here https://gist.github.com/jotak/1db63cb48f90e86dca74 and it works well, so maybe the issue is on the server side – Joel Jul 22 '15 at 22:50
  • @Nimrand it was indeed failing :) Not used to catching all Promises yet! Thank you very much. – Tholle Jul 22 '15 at 23:22
  • @Joel It sure was a server side issue. Thank you very much! – Tholle Jul 22 '15 at 23:22
  • In that case, I'll post it as an answer when I get back to my Desktop. – Nimrand Jul 22 '15 at 23:26
  • Refactor to avoid the [promise constructor antipattern](http://stackoverflow.com/q/23803743/1048572) and don't forget to `return` promises from your callbacks – Bergi Jul 23 '15 at 01:13

2 Answers2

2

Because your then statement doesn't pass a function to handle error cases, requests to the server for data can fail silently, in which case the promise returned by getDataFromServer will never complete.

To fix this, pass a second function as an argument to then, as below:

function getDataFromServer() {
  return new Promise(function(resolve, reject) {
    var result = [];

    (function fetchData(nextPageToken) {

      server.getData(nextPageToken).then(function(response) {
        result.push(response.data);
        if (response.nextPageToken) {
          fetchData(response.nextPageToken);
        } else {
          resolve(result);
        }
      }).catch(function(error) {
        //Note: Calling console.log here just to make it easy to confirm this
        //was the problem.  You may wish to remove later.
        console.log("Error occurred while retrieving data from server: " + error);
        reject(error);
      });

    })(null);

  });
}
Nimrand
  • 1,748
  • 2
  • 16
  • 29
  • Better completely avoid the [promise constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi Jul 23 '15 at 04:13
  • I considered refactoring the code to avoid this deferred promise construction, but because its recursive and the number of times server.getData will be called is indeterminate, I don't see a way around it. However, I'm new to promises in JavaScript (though I've used them in other languages), so if you know of a way around this, please let me know. – Nimrand Jul 23 '15 at 04:18
  • `reject(error)` is right, but I would let the error bubble up before logging it. i.e. `getDataFromServer().then(result => console.log(result)).catch(e => console.error(e));` - Always return *or* terminate chains with catch, is the rule I use. – jib Jul 23 '15 at 05:19
  • Good point. Using catch here is slightly safer. The difference would be that you'd catch the exception even if an exception occurred within the function passed to `then`, though there is nothing going on there that should be able to fail with an exception. – Nimrand Jul 23 '15 at 05:34
  • @Nimrand: Recursion FTW! See jib's answer – Bergi Jul 23 '15 at 05:55
  • Thank you for all the help! – Tholle Jul 23 '15 at 06:44
2

Nimrand answers your question (missing catch), but here is your code without the promise constructor antipattern:

function getDataFromServer() {
  var result = [];

  function fetchData(nextPageToken) {
    return server.getData(nextPageToken).then(function(response) {
      result.push(response.data);
      if (response.nextPageToken) {
        return fetchData(response.nextPageToken);
      } else {
        return result;
      }
    });
  }
  return fetchData(null);
}

getDataFromServer().then(function(result) {
  console.log(result);
})
.catch(function(e) {
  console.error(e);
});

As you can see, recursion works great with promises.

var console = { log: function(msg) { div.innerHTML += "<p>"+ msg +"</p>"; }};

var responses = [
  { data: 1001, nextPageToken: 1 },
  { data: 1002, nextPageToken: 2 },
  { data: 1003, nextPageToken: 3 },
  { data: 1004, nextPageToken: 4 },
  { data: 1005, nextPageToken: 0 },
];

var server = {
  getData: function(token) {
    return new Promise(function(resolve) { resolve(responses[token]); });
  }
};

function getDataFromServer() {
  var result = [];

  function fetchData(nextPageToken) {
    return server.getData(nextPageToken).then(function(response) {
      result.push(response.data);
      if (response.nextPageToken) {
        return fetchData(response.nextPageToken);
      } else {
        return result;
      }
    });
  }
  return fetchData(0);
}

getDataFromServer().then(function(result) {
  console.log(result);
})
.catch(function(e) { console.log(e); });
<div id="div"></div>
Community
  • 1
  • 1
jib
  • 40,579
  • 17
  • 100
  • 158
  • So, when the function `f` passed to `then` happens to return a promise `a`, does the promise returned by `then` wait for `a` to complete as well? Normally, the promise returned by `then` completes as soon as `f` completes, which would mean this code wouldn't necessarily work. – Nimrand Jul 23 '15 at 06:12
  • This is fantastic. Thank you jib! Liking Promises more and more. :) – Tholle Jul 23 '15 at 06:44
  • 2
    Right, when a promise is *resolved* to another promise that is *pending*, the first promise remains *pending* and wont be *fulfilled* until the second promise *resolves*. In this case, the first promise is both *resolved* and *pending*, but not *fulfilled*, using the terminology in [states and fates](https://github.com/domenic/promises-unwrapping/blob/master/docs/states-and-fates.md). – jib Jul 23 '15 at 15:24
  • Thank you. It works a bit differently in strongly typed languages, like Scala, so that wasn't intuitive to me at first. But, that makes sense. – Nimrand Jul 28 '15 at 05:48