The most likely scenario is that you are running more than one of these loops at a time and that creates a potential race condition (because of the asynchronous database call) where one loop deletes a key that the other loop is still working on.
The best fix would be to both avoid ever running two of these at the same time and to also protect against possible race conditions. If you show us the calling code context so we can see how/if it can get called more than once before the other is finished, we can advise on a way to avoid that.
Even without that, you can protect against the race condition like this:
Object.keys(globalObject).forEach(function(key) {
database.query('SELECT * from accounts WHERE `ID` = ' + key + ' LIMIT 1', function(error, rows, fields) {
// if the db says no rows or the key is already gone, then nothing to do
if (rows.length == 0 || !globalObject[key]) return;
console.log('User has ' + globalObject[key].length + ' items')
globalObject[key].forEach(function(item) {
console.log('Item ID is ' + item.id)
});
delete globalObject[key];
});
});
The extra !globalObject[key]
check added to the if
statement protects against the case where someone else has already removed the key during your database calls.
Incidentally, you don't need this if
:
if (Object.keys(globalObject).length != 0) {
because Object.keys(globalObject).forEach()
works fine if the array is empty (it just has nothing to do).
More explanation based on your comments:
The .forEach()
loop runs to completion without getting interrupted. But the database operations in the loop are non-blocking. They get started, but the loop doesn't wait for them to finish. So, the .forEach()
loop will be done, but the callbacks inside the loop have not yet been called.
So, if you have 10 keys, you will start 10 database operations (and none have completed yet). Now, the .forEach()
loop is done. If your setInterval()
calls this code again BEFORE all 10 database operations have finished, then a new .forEach()
loop will run and start some more database operations, based on keys that are about to get deleted by the first loop (when its database operations finally finish). Now, after the second .forEach()
loop has run, the database operations from the first loop start finished and removing keys. Then, after they finish, the database operations from your second .forEach()
loop finish and their callbacks are called and they are trying to use keys that have now been deleted by the callbacks inside the first loop.
The loops themselves don't run at the same time. But, after each loop runs, it sets up database callbacks that run at some indeterminate time in the future and those callbacks (as coded by you) were assuming that a certain set of keys had not been deleted. But, if you didn't wait until all the database callbacks from the first loop were done before starting a second loop, then that assumption falls apart as the second loop will be using keys that the first loop's database calls are going to remove when they finally finish. That will mess up the database callbacks that were started by the second loop.
The .forEach()
calls have nothing to do with events. They are just loops in Javascript and they run synchronously. The database calls just start asynchronous operations. Their callbacks will be triggered by events sometime in the future. Because those database calls are non-blocking asynchronous, the .forEach()
loop finishes and then sometime later the JS interpreter gets events that trigger the database callbacks to get called.