Is a promise within a promise the best solution? asynchronous node file read within for loop

55 views Asked by At

The Node.js function below takes:

  • an object, shop which contains a regular expression
  • an array of filenames

The function will read each csv file listed in the array and test a cell in the first row with a regular expression, returning a new array of matching filenames.

function matchReport(shop, arr) {

    return promise = new Promise(resolve => {
        var newArray = [];
        for(var j=0;j<arr.length;++j) {
            let filename = arr[j];
            csv()
            .fromFile(filename)
            .then(reportData => {
                if (reportData[0]['Work'].match(shop.productRegex)) {
                    newArray.push(filename);
                }
                if (j === arr.length) {
                    resolve(newArray);
                }
            });
        }
    }).then(matches => {
        return {
            'shop' : shop.name,
            'reports' : matches
        }
    }).catch(e => {
        console.log(e);
    });
}

Very rarely the function will return with the correct behavior which is this:

{ shop: 'shop1',
  reports: [ '../artist-sales-report-2020-11-12(1).csv' ] }
{ shop: 'shop2',
  reports:
   [ '../artist-sales-report-2020-12-03.csv',
     '../artist-sales-report-2020-09-01.csv' ] }

More often it returns with reports missing, like below:

{ shop: 'shop1',
  reports: [ '../artist-sales-report-2020-11-12(1).csv' ] }
{ shop: 'shop2',
  reports: [ '../artist-sales-report-2020-12-03.csv' ] }

I understand where the problem is taking place, within the csv reportData block. I understand that it is an asynchronous issue and I have tried to write more elaborate if..then or switch statements as a hack solution with no luck. It seems a little sloppy and cluttered to me to create more promises inside of this promise but I have been unsuccessful at that as well.

1

There are 1 answers

4
somethinghere On BEST ANSWER

Using async/await and your disliked nested promises you could simplify your code to something like this, which should always await all results. I made the assumption that your problem is the fromFile method, which feels like it is itself asynchronous since it uses a then that you are not awaiting.

async function matchReport(shop, arr) {
    
    const matches = await Promise.all(arr.map(async filename => {
       
        const reportData = await csv().fromFile( filename );

        if( reportData[0]['Work'].match(shop.productRegex) ){
        
            return filename;
            
        }
        
    }));
    
    return {
        'shop': shop.name,
        'reports': matches.filter( Boolean )
    };
    
}