Asynchronous confusion. Processing an API response and losing my grasp on what is happening in this code

user_601455 0
Viewed 46

I've gotten lost inside my own head about how to best structure this function. This code below is mostly correct, I just stubbed out a few of the functions at the bottom, not important.

What is supposed to happen:

  1. My node app runs at a certain time and calls refreshDevices()
  2. The function fetches a JSON payload from an API
  3. The function loops over elements in that payload and checks if there is a record already
  4. If there is no record, one is created, if there is then it is updated
  5. After this is run a log entry is created logging the number of changes.

My issue is the log entry happens prior to any of the record checks taking place... my log shows 0 added & 0 updated. I've tried re-writing the code to use an inline async/await function but I think that is effectively the same thing as below.

How am I supposed to structure this function to do what I've said. I am finding the mix of async and sync very confusing to wrap my head around.

  • I even tried adding the log entry bit to happen in a .finally() block at the end, but that was executing before the checks happened.
  • I have tried this using both forEach and map.
refreshDevices(_timespan) {  
  const status = {
    added: 0,
    updated: 0,
  };

  // Standard API fetch & json() conversion
  fetch(`https://api.example.com/test&timespan=${timespan}`)
    .then((response) => response.json())
    .then((JSON) => {
      JSON.forEach((device) => {
        findDevice(device._id)
        .then(record => {
          if (!record) {
            addDeviceRecord(device);
            status.added++;
          } else {
            deviceSeen(device._id);
            status.updated++;
          }
        })
        .catch(error => {
          console.log('Error!');
        });
      });

      systemlog.log({
        message: 'Refresh',
        data: {
          success: true,
          note: `Added: ${status.added} & updated: ${status.updated}`,
        },
      });
    })
    .catch((error) => {
      systemlog.log({
        message: 'Refresh',
        type: systemlog.logType.ERROR,
        data: {
          success: false,
          note: error,
        },
      });
    });
}

function findDevice(id) {
  return DB('device').findOne({ id }); // this returns is a promise
}

function addDeviceRecord(record) {
  return DB('device').insertOne(record); // this returns is a promise
}

function deviceSeen(id) {
  return DB('device').updateOne({ id }, { $set: { lastSeen: new Date() } }); // this returns is a promise
}

This question

node.js asynchronous javascript
2 Answers

You could use Async/await to make the code more readable and use for of to iterate over data.

for of loop will wait for the first iteration to complete and then go for the second one and so on.

On the other hand forEach is based on callback and so it will execute and register a callback for each iteration in the callback queue and exit and don't wait for the callbacks to complete.

Try this 

    async refreshDevices(_timespan) {
        const status = {
            added: 0,
            updated: 0,
        };
        // Standard API fetch & json() conversion
        try {
            let response = await fetch(`https://api.example.com/test&timespan=${timespan}`)
            let data= await response.json();
            for (const device of data) {
                try {
                    let record = await findDevice(device._id);
                    if (!record) {
                        addDeviceRecord(device);
                        status.added++;
                    } else {
                        deviceSeen(device._id);
                        status.updated++;
                    }
                } catch (error) {
                    console.log('Error!');
                }
            }
            systemlog.log({
                message: 'Refresh',
                data: {
                    success: true,
                    note: `Added: ${status.added} & updated: ${status.updated}`,
                },
            });
        } catch (error) {
            systemlog.log({
                message: 'Refresh',
                type: systemlog.logType.ERROR,
                data: {
                    success: false,
                    note: error,
                },
            });
        }
    }

function findDevice(id) {
    return DB('device').findOne({ id }); // this returns is a promise
}
function addDeviceRecord(record) {
    return DB('device').insertOne(record); // this returns is a promise
}
function deviceSeen(id) {
    return DB('device').updateOne({ id }, { $set: { lastSeen: new Date() } }); // this returns is a promise
}

Alternatively you could also use promise.all to wait for all promises to be resolved.

 async refreshDevices(_timespan) {
        const status = {
            added: 0,
            updated: 0,
        };
        // Standard API fetch & json() conversion
        try {
            let response = await fetch(`https://api.example.com/test&timespan=${timespan}`)
            let data = await response.json();
            let promises = data.map(device => {
                return new Promise( async (resolve, reject) => {
                    try {
                        let record = await findDevice(device._id);
                        if (!record) {
                            addDeviceRecord(device);
                            status.added++;
                        } else {
                            deviceSeen(device._id);
                            status.updated++;
                        }
                        resolve();
                    } catch (error) {
                        console.log('Error!');
                        reject(error);
                    }
                });
            });
            let result = await Promise.all(promises);
            systemlog.log({
                message: 'Refresh',
                data: {
                    success: true,
                    note: `Added: ${status.added} & updated: ${status.updated}`,
                },
            });
        } catch (error) {
            systemlog.log({
                message: 'Refresh',
                type: systemlog.logType.ERROR,
                data: {
                    success: false,
                    note: error,
                },
            });
        }
    }
function findDevice(id) {
    return DB('device').findOne({ id }); // this returns is a promise
}
function addDeviceRecord(record) {
    return DB('device').insertOne(record); // this returns is a promise
}
function deviceSeen(id) {
    return DB('device').updateOne({ id }, { $set: { lastSeen: new Date() } }); // this returns is a promise
}
user_41489020

As you have identified, your system.log call happens immediately after launching a litany of promises which haven't resolved.

Inside your refreshDevices(_timespan) function add an empty array to add each new promise in the forEach.

Then modify your code like

JSON.forEach((device) => {
            promises.push( // Add this promise to the array
                findDevice(device._id)
                .then(record => {
                    if (!record) {
                        addDeviceRecord(device);
                        status.added++;
                    } else {
                        deviceSeen(device._id);
                        status.updated++;
                    }
                })
                .catch(error => {
                    console.log('Error!');
                });
            ) // end of .push() arg
        });

Then use Promise.all() to wait for all promises to be resolved before calling systemLog.log, sort of like (pseudo-pseudo code :)

Promise.all(promises).then(
            systemlog.log({
                message: 'Refresh',
                data: {
                    success: true,
                    note: `Added: ${status.added} & updated: ${status.updated}`,
                },
            });
        )
user_42861640
Related