Returning a single resolve() from a loop

Question

I wrote the below code snippet:

var data = require('./readFile.js');
const argv = require('./index.js');
const colors = require('colors');
const rp = require('request-promise').defaults({ encoding: null });
const memeImage = argv.name;
const path = './json/meme.json';

var myData;

data.readFile(path)
.then(function (value) {
    var jsonData = JSON.parse(value);

    const options = {
        resolveWithFullResponse: true
    }
    jsonData.forEach(element => {
        if(element.title == memeImage) {
            rp.get(element.image, options)
            .then(function(response){
                if(response.statusCode == 200) {
                    myData = Buffer.from(response.body).toString('base64');
                    myData.replace('myData:image/png;base64,', '');
                    console.log(myData);

                    resolve(myData);
                }
                else {
                    console.log('Error in retrieving image.  Status code: ', response.statusCode);
                    reject('Error in retrieving image.');
                }
            })
            .catch(function(error){
                console.log('Error in downloading the image via request()');
                reject('Error in downloading the image via request()');
            });
        }
    });
})
.then(function(myData) {
    console.log(myData);
})
.catch(function(err) {
    //Error in reading the file
    console.log(err);
})

module.exports=myData;

Here, function readFile() imported from a different file (readFile.js) returned a promise. readFile.js is as below:

var myData, obj, found=false;

function readFile(path) {
    return new Promise(function(resolve, reject){
        if(!fs.existsSync(path)) {
            console.log('Will throw error now!');
            reject("memes.json file does not exist");
        }
        else {
            //File exists
            console.log('File found');
            var data = fs.readFileSync(path, 'utf8');
            resolve(data);
        }
    })
}

module.exports.readFile = readFile;

Now in the topmost code snippet, my aim is to read a file (that maps a title to a URL) using the readFile.js file and then download the image at the corresponding URL. Once this match is found (if clause above), the promise should be resolved and returned to a new module.

What happens right now is, I get the following output:

node ./lib/getImage.js -n title -t topText -b bottomText
File found
undefined
<... image ... as a base64 string>
Error in downloading the image via request()
Unhandled rejection ReferenceError: reject is not defined ...

My question is, answers such as this and this all talk about returning multiple promises from a loop. In my case, I just want to return a single promise resolve (depending upon the match) and just stop.

How do I achieve this?

jfriend00 · Accepted Answer · 2019-08-14T03:50:38.480

0

You don't just blindly call resolve() and reject() inside a .then() handler. Those functions are ONLY defined in a manually created promise so they don't exist in your context.

If you want to set the resolved value in .then() handler, then just return that value. If you want to reject inside a .then() handler, then either throw someErr or return a rejected promise.

If you're inside a .forEach() callback and you want to stop further processing and set the resolved value of the parent promise, you can't. You're inside a callback function so you can't directly return from the outer function and .forEach() has no way of stopping its iteration. Instead, use a regular for loop and then you can just return value to both end the for loop iteration and to establish the resolved value of your promise chain. A regular for loop gives you significantly more control over the program flow than a .forEach() loop.

FYI, Unhandled rejection ReferenceError: is caused by creating an exception inside your .catch() by attempting to call an non-existing reject() function. Remove that. If you want to log and keep the promise rejected, then throw.

And, then if you use rp.get() inside a for loop, you will either need to use it with await or Promise.all() with it in order to know when everything is done.

edited Aug 14 '19 at 03:50

answered Aug 14 '19 at 03:37

jfriend00

683,504
96
985
979

Oh, but I do have a `.catch()` corresponding to _console.log('Error in downloading the image via request()');_ which is related to `rp.get()`, no? – J. Doe Aug 14 '19 at 03:42
And, could you please elaborate _A regular for loop gives you significantly more control over the program flow than a .forEach() loop_? – J. Doe Aug 14 '19 at 03:44
@J.Doe - Yeah, you do I didn't see that because of your different coding style. But the error happens because you're creating another error in your `.catch()` with `reject('Error in downloading the image via request()');` and there's no `.catch()` after that to catch that new error. Of course, you don't want another `.catch()` - just remove the reject. – jfriend00 Aug 14 '19 at 03:44
@J.Doe - With a regular `for` loop, you can `return` or `break` to stop the iteration. With `.forEach()` there's no way to stop the iteration. With a `for` loop inside a `.then()` handler, you can use `return someValue` to finish the `.then()` handler and set the resolved value of the promise chain and stop the loop. Inside a `.forEach()` callback, you can't do either one of those because returning from inside the `.forEach()` callback just finishes that iteration and the `.forEach()` loop just keep chugging along. – jfriend00 Aug 14 '19 at 03:46
@J.Doe - Frankly with `for/of` and block scope in modern Javascript, there's rarely ever a reason I use `.forEach()` any more. It's just extra overhead and coding complication with little benefit. – jfriend00 Aug 14 '19 at 03:47
Great, thanks, @jfriend00. I appreciate your help! – J. Doe Aug 14 '19 at 03:58

Returning a single resolve() from a loop

1 Answers1