2

Recently I have been having an issue with await not waiting for a function to finish in a for ... of loop. I seem to have poor understanding of what needs to be fulfilled for a await to actually wait for a function to fully resolve. This is the code I'm looking at:

async function updateFromAllSources(){
    try{
        console.log(appLock)
        if(appLock===1){
            throw 'Application is currently running!'
        }
        appLock = 1;
        const sources = await Source.find({}).exec();
        for(const source of sources){
            console.log(`[LOG] Updating ${source.name}`)
            await updateProductsFromCsv(source)
        }
        appLock = 0;
    } catch (e) {
        console.log(`[LOG]${e}`)
    }
    
}

async function updateProductsFromCsv(source){
    try{
        const {body} = await got.get(source.url, {
            responseType: 'text',
        });
        let productActive = 0;
        csv(body.trim(), { columns: true, delimiter: source.delimiter }, async (err, records) => {
            for (const [index, element] of records.entries()) {
                if (element['Stock'] > source.minStock) {
                    productActive = 1;
                }
                console.log(`[LOG] Aktualizacja produktu ${index}/${records.length}`);
                io.emit('logging', `Aktualizacja produktu ${index}/${records.length}`);
                await updateProductByName(element['Article number'], { stock: element['Stock'], active: productActive });
            }
        })
    } catch(error){
        console.error(error);
    }
}

Now: await updateProductsFromCsv(source) resolves immediately without waiting for the loop in updateProductsFromCsv(), but the same construct used in updateProductsFromCsv() where it's supposed to wait for updateProductByName() works as expected, the loop waits for the function to be resolved before iterating further.

What's the difference? How do I make the first for loop wait for the function to resolve?

Anton
  • 137
  • 8
  • 1
    can you show the definition of your `csv` function ? Or from which library it comes from? In order to await a function, it needs to return a promise. But `updateProductsFromCsv` does not have any `await` - it has a callback style so that's why it's not returning a promise – Gonzalo.- Jul 24 '20 at 17:50
  • @Gonzalo.- ``updateProductsFromCsv`` is an async function so it will return a promise. The problem is ``await`` inside for loop which obviously will not work. – Sachin Singh Jul 24 '20 at 17:54
  • 1
    @SachinSingh No, there is nothing wrong with the `await` inside the loop. The problem is that the promise returned by `updateProductsFromCsv` fulfills too early, because it doesn't wait for the asynchronous callback of the `csv` call. – Bergi Jul 24 '20 at 19:10

1 Answers1

2

It's because of your csv function call inside try block. You are passing a resulting callback as an argument and doing await inside for loop after getting the records. The csv function call is still going asynchronously and control won't wait for that call since there is no explicit await for csv.

To achieve the desired result, change your updateProductsFromCsv as below (Wrap it within Promise):

async function updateProductsFromCsv(source) {
    try {
        const {body} = await got.get(source.url, {
            responseType: 'text',
        });
        let productActive = 0;
        const records = await new Promise((resolve, reject) => {
            csv(body.trim(), { columns: true, delimiter: source.delimiter }, (err, data) => {
                if (err) reject(err); // Reject incase of error
                else resolve(data);
            });
        });
        for (const [index, element] of records.entries()) {
            if (element['Stock'] > source.minStock) {
                productActive = 1;
            }
            console.log(`[LOG] Aktualizacja produktu ${index}/${records.length}`);
            io.emit('logging', `Aktualizacja produktu ${index}/${records.length}`);
            await updateProductByName(element['Article number'], { stock: element['Stock'], active: productActive });
        }
    } catch(error) {
        console.error(error);
        throw error;
    }
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
Prathap Reddy
  • 1,688
  • 2
  • 6
  • 18
  • 1
    This doesn't work. You should wrap the `new Promise` *only* about the `csv(…)` call and nothing else, even do the iteration over the records outside of it (after awaiting the promise). – Bergi Jul 24 '20 at 19:08
  • 1
    Hey @Bergi, you are correct. My bad. Will update the answer. Thanks for pointing out the issue – Prathap Reddy Jul 24 '20 at 19:13
  • Hope the updated answer solve the problem. Thanks @Bergi. Will try to be more cautious while posting my answers – Prathap Reddy Jul 24 '20 at 19:26
  • 1
    I took the liberty to fix the error handling - feel free to rollback the edit if you don't like it – Bergi Jul 24 '20 at 19:28
  • Looks ideal. Thanks for updating it. – Prathap Reddy Jul 24 '20 at 19:29
  • @Bergi this seems alright almost, I havent tried it yet, but doesn't it resolve the promise after taking the records? I want it to resolve after the for loop with updateProductByName – Anton Jul 24 '20 at 19:39
  • @Anton There are many promises here that fulfill at different times. The one returned by `updateProductsFromCsv()` will resolve when the body of the function returns, after having `await`ed all the promises created in there. – Bergi Jul 24 '20 at 19:41
  • @Anton, code iniside `for ... of` loop executes synchronously as per your requirement. There is no need of wrapping whole thing inside `Promise`. – Prathap Reddy Jul 24 '20 at 19:43
  • @PrathapReddy I just don't see how the promise is resolving after the loop, that's the intended behaviour – Anton Jul 24 '20 at 20:01
  • Call inside `Promise` alone is not acting as promise in the method. Here is the sequence how it resolves synchronously. 1) got.get (await) call. 2) csv (promise) call. 3) All dynamic promises inside `for ... of` (await `updateProductByName`) based on number of `records` entries. Only after executing all these three steps in same sequence, the function `updateProductsFromCsv` will be resolved. Hope this clear the air for you. – Prathap Reddy Jul 24 '20 at 20:08
  • @PrathapReddy ok that makes it clearer for me. So why did it not work before? Was it because the callback function did not return a promise so the await in the `updateFromAllSources()` thought all the promises were resolved? – Anton Jul 24 '20 at 20:24
  • It didn't work with code posted in question because the `csv` function call is not waiting for any thing (no await before `csv`). We are passing only callback not asking it to wait explicitly. In that case the `callback` execution will be moved to `call back queue` and the main `thread` immediately removes `csv` function thinking that it's done and go to the next step after `csv`. Since there is no code after `csv` the `updateProductsFromCsv` will result that it's done. The `event loop` is responsible to bring back the qued items in `call back queue` to `main` thread again at regular intervals – Prathap Reddy Jul 24 '20 at 20:31
  • Feel free to go through [this video](https://m.youtube.com/watch?vl=en&v=8aGhZQkoFbQ) to understand things explained in my comments even better. – Prathap Reddy Jul 24 '20 at 20:36