Promise not waiting until request is finished

Question

I'm trying to chain promises together to make a series of requests. This is one of the few times I've used promises, so I'm not too sure what I'm doing. Hoping that someone can catch whats going on wrong. Basically, my first promise is working correctly, then the second function runs after the first promise is resolved, however, the third does not wait so the information I'm requesting is incomplete.

I can see that my functions are returning the correct information. So mainly my question is how do I format my promises so they each wait for each other to complete before running?

function getPaths(folderpath) {
  return new Promise(function(resolve, reject) {
    db.getPaths(folderpath, function(response) {
      //get paths for files and folders
      for (let i = 0; i < response.entries.length; i++) {
        let entries = response.entries[i];
        if (entries[".tag"] == "folder") {
          folderPaths.push(response.entries[i].path_lower);
        }
        if (entries[".tag"] == "file") {
          filePaths.push(response.entries[i].path_lower);
        }
      }
      resolve();
    });
  });
}

function openFolders(folders) {
  return new Promise(function(resolve, reject) {
    for (let i = 0; i < folders.length; i++) {
      db.getPaths(folders[i], function(response) {
        for (let j = 0; j < response.entries.length; j++) {
          let entries = response.entries[j];
          if (entries[".tag"] == "file") {
            filePaths.push(response.entries[j].path_lower);
          }
        }
        //console.log(filePaths); //returns correct information
      });
    }
    resolve();
  });
}

getPaths("/path").then(function() {
  openFolders(folderPaths);
}).then(function() {
  console.log(filePaths); //returns incomplete information
});

where is `folderPaths` defined? `folderPaths.push` seems wrong in the async handler- it looks like you're trying to coordinate synchronous code to read from a global var that is modified by asynchronous code - definitely not the right way to do whatever it is you're trying to do. — Mulan, Jun 22 '17 at 21:14
Possibly related: [How do I return the response from an asynchronous call?](https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call) — Mulan, Jun 22 '17 at 21:16

Mulan · Accepted Answer · 2017-06-23T21:07:15.913

You have a big problem here in bold

function openFolders(folders) {
  return new Promise(function(resolve, reject) {
    for (let i = 0; i < folders.length; i++) {
      db.getPaths(folders[i], function(response) {
        // ...
      });
    }
    resolve();
  });
}

You're running a synchrounous for loop around an asynchronous function call. The loop finishes iterating synchronously and then immediately calls resolve() - the code in the async handler (eg, filePaths.push(...)) does not run before the Promise is resolved

Your problems don't stop there though. You're using Promises unconventionally by modifying global state and then resolving and empty Promise - resolve() vs resolve(someValue). @FrankModica's answer has more to say about that.

As for remedies, I'd recommend you take a look at Promise.all - and in general, using Promises in a more conventional way

In the snippet below, I've mocked your db function to read from a fake in-memory database, fakedb. Then, I made a getPaths function which wraps db.getPaths but returns a Promise instead. Then I rewrite openFolders to create an array of Promises, pass them to Promise.all, then finally resolve the values I want

Worth noting: Promise.all will run your array of Promises in parallel - if this undesirable, you could run them in serial using .reduce and .then.

const db = {
  getPaths: (path, k) => {
    setTimeout(k, 50, fakedb[path])
  }
}

function getPaths (path) {
  return new Promise((resolve, reject) =>
    db.getPaths(path, response => resolve(response)))
}

function openFolders (folders) {
  return Promise.all(folders.map(getPaths))
    .then(responses =>
      responses.reduce((acc, {entries}) =>
        acc.concat(entries.filter(x => x['.tag'] === 'file').map(x => x.path_lower)), []))
}

const fakedb = {
  'cat': {
    'entries': [
        { '.tag': 'dir', 'path_lower': './cat/.' },
        { '.tag': 'dir', 'path_lower': './cat/..' },
        { '.tag': 'file', 'path_lower': './cat/a' },
        { '.tag': 'file', 'path_lower': './cat/b' },
        { '.tag': 'file', 'path_lower': './cat/c' }
      ]
  },
  'dog': {
    'entries': [
        { '.tag': 'dir', 'path_lower': './dog/.' },
        { '.tag': 'dir', 'path_lower': './dog/..' },
        { '.tag': 'file', 'path_lower': './dog/a' },
        { '.tag': 'file', 'path_lower': './dog/b' },
        { '.tag': 'file', 'path_lower': './dog/c' }
      ]
  }
}

openFolders(['cat','dog']).then(console.log, console.error)

The operation I'm performing in openFolders might feel a little complex because it handles all transformations of the responses in a single .then handler - you could (optionally) separate the work into multiple .then calls which might result in a lighter cognitive load

function openFolders (folders) {
  return Promise.all(folders.map(getPaths))
    // get `.entries` of each response
    .then(responses =>
      responses.map(x => x.entries))
    // flatten array of entries arrays into a single array
    .then(arrOfEntries =>
      arrOfEntries.reduce((acc, entries) =>
        acc.concat(entries), []))
    // only keep entries where `.tag` of each entry is `'file'`
    .then(entries =>
      entries.filter(x => x['.tag'] === 'file'))
    // return `path_lower` of the resulting entries
    .then(entries =>
      entries.map(x => x.path_lower))
}

How do you know db.getPaths is asynchronous? It is not the same function that he is declaring. — Nick Wyman, Jun 22 '17 at 21:25
How would I go about running getPaths for each item in folders? If I were to uncommented the print statement in that function, the correct information is in fact being added? — 23k, Jun 22 '17 at 21:30
I didn't notice the potentially async call inside the loop. Can you update your answer with a way to fix it? — Frank Modica, Jun 22 '17 at 21:36
@NickWyman I don't *know*, I just strongly inferred it based on the function name being prefixed with `db` and the signature continuation passing style (callback) — Mulan, Jun 22 '17 at 21:40
@23k it was a bit of a challenge, but I was able to create a runnable code snippet in my updated answer. I think you'll be happy with the results. — Mulan, Jun 22 '17 at 21:58
@FrankModica the updated answer offers *one* possible way it could be done — Mulan, Jun 22 '17 at 21:58

Frank Modica · Answer 2 · 2017-06-22T21:36:04.837

If I understand correctly, you are using some variables from the outer scope (folderPaths and filePaths). I'm not sure that's the best way to go about this, but I believe it will start working once you return the promise from openFolders. This way you wait until openFolders completes:

getPaths("/path").then(function() {
  return openFolders(folderPaths);
}).then(function() {
  console.log(filePaths);
});

But I'd recommend resolving the information required for the next call:

resolve(folderPaths);

And:

resolve(filePaths);

So your code can look more like:

getPaths("/path").then(function(folderPaths) {
  return openFolders(folderPaths);
}).then(function(filePaths) {
  console.log(filePaths); 
});

Edit: Looks like you may have another issue mentioned by @naomik. If db.getPaths is async, in your openFolders function you may be resolving before all of the async calls in the loop complete. Unfortunately I don't have time right now to show the solution. Hopefully @naomik can!

Promise not waiting until request is finished

2 Answers2