0

Code:

var promiseResolve, promiseReject;    

function iterateRules(rules, params, i = 0) {

    CWevents.putRule(rules[i], function(err, data) {    // This is an async function with a callback
        if (err) console.log(err); promiseReject(err)

        responses++
        params['Lambda_invoke']['SourceArns'].push(data.RuleArn)

        if(responses == rules.length){ promiseResolve("Success"); console.log("Resolved")} 
        // After two responses are confirmed, this if does run and I get the "Resolved"
    })
    i++
    if(i < rules.length){
        setTimeout( function(){
            iterateRules(params['CW_rules']['Rules'], params, i)
        }, 50)
    }
}

new Promise((resolve, reject) => {
    resolve() 
// This part is added solely for the code to make sense, it's taken out of a 
// bigger script and lots of unnecessary data is removed 
})
.then((Item) => {
            return new Promise((resolve, reject) => {
                promiseReject = reject;
                promiseResolve = resolve;
                iterateRules(params['CW_rules']['Rules'], params)
            })
}) .then((res) => {
        console.log("This ran")
   })

The iterateRules function is supposed to run an asynchronous function multiple times, and resolve the Promise it's called in when the last response is acquired. The if(responses == rules.length) does run as expected and logs the "Resolved" in the console. Everything is successful, no errors.

Beneath is the context of this code, the iterateRules function is executed, but the .then that comes after, isn't. If I put a resolve() inside the promise directly, it does execute. What might be wrong with this script? I tried running a simple version of it that looks like this separately:

var res, rej;

function dude(){
    res()
}

new Promise((resolve, reject) => {
    res = resolve;
    dude()
}).then((dude) => {
    console.log("resolved")
})

And it does in fact work, so I'm very confused. What might cause the problem?

Cœur
  • 37,241
  • 25
  • 195
  • 267
almarc
  • 1,598
  • 3
  • 11
  • 31
  • 2
    First thing I notice is you need to not use the [Promise constructor antipattern](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) – Liam Oct 25 '19 at 14:23
  • 3
    this line `new Promise((resolve, reject) => { resolve()})` makes zero sense? Why are you making a promise then immediately resolving it? – Liam Oct 25 '19 at 14:24
  • Because it's a piece of a 300 line script and it contains sensitive data, lots of non-default functions and would be unnecessary for the case. So I replaced it with a simple `resolve()` for the script to make sense. – almarc Oct 25 '19 at 14:25
  • Wow, this code is an async nightmare...I'm not sure where to begin. is `iterateRules` supposed to return a promise? – LostJon Oct 25 '19 at 14:40
  • No, `iterateRules` just resolves the promise it's called in when all the requests are resolved. It should at least. – almarc Oct 25 '19 at 14:41
  • is there a reason why you cant make iterateRules return a promise? passing a global scope var like `promiseResolve` is not a good practice – LostJon Oct 25 '19 at 14:59
  • I'm pretty new to promises so I'm not entirely sure what you're talking about. Can you please clarify? – almarc Oct 25 '19 at 15:04
  • see my answer below....hard to explain in a comment – LostJon Oct 25 '19 at 16:32

1 Answers1

1

I would make iterateRules() return a promise (since it is asynchronous). I would also promisify the CWevents.putRule() function so you can accomplish some code like the below:

function iterateRules(rules, params) {
    return Promise.all(rules.map(rule => {
        return CWevents.putRule(rule)
    })).then((data) => {
        params['Lambda_invoke']['SourceArns'].push(data.RuleArn)
    })
}

Then your handler for iterateRules would become:

iterateRules(rules,params).then(()=>{
    // Do something...
})
LostJon
  • 2,287
  • 11
  • 20
  • and for the purists, yes i know there is a shorthand for `CWevents.putRule()`, but it is less readable if you dont know about `(val) => {return val}` and `(val) => val` – LostJon Oct 25 '19 at 16:34
  • 1
    Thanks. I've implemented it a bit differently using `Promise.all()`, but it fixed the problem and looks much more neat. – almarc Oct 26 '19 at 16:44