I have a set of functions in Node.js that I would like to load in a certain order. I will provide some mockup code abstracted and simplified:

function updateMyApp() {
loadDataToServer()
.then(() => alterData())
.then(() => saveData())
.then(() => { console.log("updateMyApp done") })
}

function loadDataToServer() {
return new Promise( (resolve, reject) {
...preparing data and save file to cloud...
resolve()})
}

function handleDataItem(item) {
// Function that fetches data item from database and updates each data item
console.log("Name", item.name)
}

function saveData() {
// Saves the altered data to some place
}

useData is a bit more complex. In it I would like to, in order:

1. console.log('Starting alterData()')
2. Load data, as json, from the cloud data source
3. Iterate through every item in the json file and do handleDataItem(item) on it.
4. When #2 is done -> console.log('alterData() done')
5. Return a resolved promise back to updateMyApp
6. Go on with saveData() with all data altered.

I want the logs to show:

Starting useData()
Name: Adam
Name: Ben
Name: Casey
useData() done

my take on this is the following:

function useData() {
   console.log('Starting useData()')
   return new Promise( function(resolve, reject) {
      readFromCloudFileserver()
      .then(jsonListFromCloud) => {
         jsonListFromCloud.forEach((item) => {
            handleDataItem(item)
         }
      })
      .then(() => {
         resolve() // I put resolve here because it is not until everything is finished above that this function is finished
         console.log('useData() done')
      }).catch((error) => { console.error(error.message) })
   })
}

which seems to work but, as far as I understand this is not how one is supposed to do it. Also, this seems to do the handleDataItem outside of this chain so the logs look like this:

Starting useData()
useData() done
Name: Adam
Name: Ben
Name: Casey

In other words. It doesn't seem like the handleDataItem() calls are finished when the chain has moved on to the next step (.then()). In other words, I can not be sure all items have been updated when it goes on to the saveData() function?

If this is not a good way to handle it, then how should these functions be written? How do I chain the functions properly to make sure everything is done in the right order (as well as making the log events appear in order)?

CodePudding user response：

You have 3 options to deal with your main issue of async methods in a loop.

1. Instead of forEach, use map and return promises. Then use Promise.all on the returned promises to wait for them to all complete.

2. Use a for/of loop in combination with async/await.

3. Use a for await loop.

CodePudding user response：

It sounds like there's a problem in the implementation of handleDataItem() and the promise that it returns. To help you with that, we need to see the code for that function.

You also need to clean up useData() so that it properly returns a promise that propagates both completion and errors.

And, if handleDataItem() returns a promise that is accurate, then you need to change how you do that in a loop here also.

Change from this:

function useData() {
   console.log('Starting useData()')
   return new Promise( function(resolve, reject) {
      readFromCloudFileserver()
      .then(jsonListFromCloud) => {
         jsonListFromCloud.forEach((item) => {
            handleDataItem(item)
         }
      })
      .then(() => {
         resolve() // I put resolve here because it is not until everything is finished above that this function is finished
         console.log('useData() done')
      }).catch((error) => { console.error(error.message) })
   })
}

to this:

async function useData() {
    try {
        console.log('Starting useData()')
        const jsonListFromCloud = await readFromCloudFileserver();
        for (let item of jsonListFromCloud) {
            await handleDataItem(item);
        }
        console.log('useData() done');
    } catch (error) {
        // log error and rethrow so caller gets the error
        console.error(error.message)
        throw error;
    }
}

The structural changes here are:

1. Switch to use async/await to more easily handle the asynchronous items in a loop
2. Remove the promise anti-pattern that wraps new Promise() around an existing promise - no need for that AND you weren't capturing or propagating rejections from readFromCloudFileServer() which is a common mistake when using that anti-pattern.
3. rethrow the error inside your catch after logging the error so the error gets propagated back to the caller