Iteration normally works with Map(), Set().. but breaking with promises

Question

Steps provide to reproduce..

const map = new Map()
let success = new Set()

let failed = new Set()
map.set('success', success)
map.set('failed', failed)
success.add(123)
failed.add(55)
success.add(456)

var iter = map.entries()
console.log(
  'printing map output ==> ',
  map,
  'map.entries() ==> ',
  map.entries(),
  'iter.next().value ==> ',
  iter.next().value,
  'iter.next().value ==> ',
  iter.next().value
)

for (const [key, value] of map.entries()) {
  for (const val of value) {
    console.log(key, val)
  }
}

above code outputs me following results which are expected..

So now, I use the same code using a promise,

$(button).submit(updateTags(enteredTags, tagName,contentIDs).then(
  (resultMap) => {
     console.log("result after promise ==> ", resultMap); // I get proper output here, please see the below image for this

       for (const [key, value] of resultMap.entries()) {
           for (const val of value) {
               console.log("key ==> ", key,"value ==> ", val)// I don't get all the results here.. I am hurting my head with this, did I do any mistake ?
           }
       }
  }));


  let updateTags = (newTags, tagName, contentIds) => {
    let deferred = Q.defer(),
        resultMap = new Map(),
        success_ids = [],
        failed_ids = [],
        counter = 1;
    resultMap.set("success", success_ids);
    resultMap.set("failed", failed_ids);

     contentIds.forEach(function (contentId) {
        var tags = [];
        osapi.jive.core.get({
            v: "v3",
            href: "/contents/" + contentId + ""
        }).execute(function (content) {
            tags = content.tags;
            content["tags"] = _.union(tags, newTags);
            osapi.jive.core.put({
                v: "v3",
                href: "/contents/" + contentId + "",
                body: content
            }).execute(function (response) {
                    if (response.error) {
                        failed_ids.push(contentId);
                    } else {
                        success_ids.push(contentId);
                    }
                    if (counter === contentIds.length) {
                        deferred.resolve(resultMap);
                        $("#spinner").hide()
                    }
                    counter++;
                    deferred.resolve(resultMap);
                }, function (error) {
                    failed_ids.push(contentId);
                    if (counter === contentIds.length) {
                        deferred.resolve(resultMap);
                        $("#spinner").hide()
                    }
                    counter++;
                }
            );
        })
    });
    return deferred.promise;
};

output image of this code below:

Finally, this second code snippet "with promise" should display results as follows:

I have now added all the code of mine, the promise resolves only once and I get the resultMap to check on the success and failedIDs.. I want to iterate them to again dump them into loggers..

I am hurting my head from 4 hours.. any pointers would be appreciated.. — ggorantala, Jan 04 '19 at 11:43
Please post the actual code to make reproducible example, not `//some Math logic, code...`. Why are you even using promises at all here? There's nothing asynchronous in the code you posted. And why do you `resolve` inside a `forEach` loop? That doesn't make sense — Bergi, Jan 04 '19 at 11:51
Why are you passing a promise to `$(button).submit(…)`, what do you expect this to do? — Bergi, Jan 04 '19 at 11:51
if you look at the console statement you see success is an array of 0 and failed is an array of 1 but when you expand it then success is 7 and failed is 4. In updateTags you are resolving multiple times but your `then` function is called only the first time you call resolve and has only one fail value. The for loop continues and mutates the map but at this time your then function is already called. Maybe try with Promise.all — HMR, Jan 04 '19 at 11:58
@Bergi: Please see I am using a product specific API call which is "osapi.jive.core.put".. I am using this to update Tags in BULK, returning the resolved promise to check for the success and failed contents(contentID).. — ggorantala, Jan 04 '19 at 12:00
Please update your question with a [mcve] demonstrating the problem, ideally a **runnable** one using Stack Snippets (the `[<>]` toolbar button; [here's how to do one](https://meta.stackoverflow.com/questions/358992/)). — T.J. Crowder, Jan 04 '19 at 12:01
@gopigorantala [You still should use `Promise.all` for that](https://stackoverflow.com/questions/31424561/wait-until-all-es6-promises-complete-even-rejected-promises) — Bergi, Jan 04 '19 at 12:01
@Bergi: $(button).submit(…) => this click triggers the Bulk update of contents with new TAGS entered by user — ggorantala, Jan 04 '19 at 12:03
For the success and fail functions try to increment the counter first and only resolve when counter is `contentIds.length` You are currently still resolving when counter is not `contentIds.length` (under counter++) — HMR, Jan 04 '19 at 12:13
@T.J.Crowder: updated it with proper code.. can you check and let me know the pointers — ggorantala, Jan 04 '19 at 12:13
@gopigorantala - Please review the MCVE link above again. Your example misses out the C. — T.J. Crowder, Jan 04 '19 at 12:21
@HMR, the counter gets increased irrespective of the success and fail.. and then when the counter value is equal to the contentIDs.length, then the promise resolves — ggorantala, Jan 04 '19 at 12:28
Your last .execute takes 2 functions, the first will resolve the promise no matter what (you can only resolve a promise once, see comment on your console output what happens). Both of the functions should be: `counter++ set failed or success if(couner === someLength){resolve}` The first function passed to your last execute does not do this. — HMR, Jan 04 '19 at 12:37

HMR · Accepted Answer · 2019-01-04T13:13:11.223

In the code currently in your question you are resolving the promise too soon:

let updateTags = (newTags, tagName, contentIds) => {
     //... removed some code that does not show where you go wrong
     contentIds.forEach(function (contentId) {
        osapi.jive.core.get()
        .execute(function (content) {
            }).execute(function (response) {
                    if (response.error) {
                        failed_ids.push(contentId);
                    } else {
                        success_ids.push(contentId);
                    }
                    if (counter === contentIds.length) {
                        deferred.resolve(resultMap);
                        $("#spinner").hide()
                    }
                    counter++;
                    //here you are always resolving the promise
                    deferred.resolve(resultMap);

I think the following should work:

let updateTags = (newTags, tagName, contentIds) => {
    let deferred = Q.defer(),
        resultMap = new Map(),
        success_ids = [],
        failed_ids = [],
        counter = 1;
    resultMap.set("success", success_ids);
    resultMap.set("failed", failed_ids);
    //promise will never be resolved if you have no content ids
    if(contentIds.length === 0){
      //not sure if Q.resolve exist but here you return a resolved
      //  promise with the empty success and failed arrays
      $("#spinner").hide();
      return Promise.resolve(resultMap);//resolve with empty value
    }

     contentIds.forEach(function (contentId) {
        var tags = [];
        osapi.jive.core.get({
            v: "v3",
            href: "/contents/" + contentId + ""
        }).execute(function (content) {
            tags = content.tags;
            content["tags"] = _.union(tags, newTags);
            osapi.jive.core.put({
                v: "v3",
                href: "/contents/" + contentId + "",
                body: content
            }).execute(function (response) {
                    counter++;
                    if (response.error) {
                        failed_ids.push(contentId);
                    } else {
                        success_ids.push(contentId);
                    }
                    if (counter === contentIds.length) {
                        deferred.resolve(resultMap);
                        $("#spinner").hide()
                    }
                }, function (error) {
                    //I assume execute will either call one or the other function
                    // that is passed to it so increment counter here
                    counter++;
                    failed_ids.push(contentId);
                    if (counter === contentIds.length) {
                        deferred.resolve(resultMap);
                        $("#spinner").hide()
                    }
                }
            );
        })
    });
    return deferred.promise;
};

You are right.. I am resolving the promise early.. This works and I get the desired output.. Thanks HMR — ggorantala, Jan 04 '19 at 12:58
I am spinning the "spinning wheel" without coming out of it to think of possibilities.. — ggorantala, Jan 04 '19 at 13:00
@gopigorantala I have added a `$("#spinner").hide()` in the early returned promise when content id's is empty. I'm not sure if there are other cases this could happen. Maybe add some console.logs to see what contentIds is and how many of them come back in the execute callbacks. — HMR, Jan 04 '19 at 13:14
no worries, I have taken care of the spinner.. your answer gave me some more clues and I fixed my code.. Appreciate your help ;) — ggorantala, Jan 04 '19 at 13:22

Iteration normally works with Map(), Set().. but breaking with promises

1 Answers1