Why is .then() not executing at all for my Promise?

Question

I have a promise chain that I want to run, and when it finishes, I resolve the Promise (please see code below for more context), and expect my .then() block to run... But it does not. Here is my code:


function notifications_for_each_in_array(iteration, the_array, error_array) {
    
    return new Promise(function(resolve, reject) {

        if(!iteration) iteration = 0;
        var error_array = error_array || [];

        var user_id = the_array[iteration];

        $.ajax({
            url: ".....my_url.....",
            type: "PUT",
            data: JSON.stringify({test: [user_id]}),
            success: function() {
                // ...
            }

        }).done(function(rez) {
            error_array.push(rez);

            iteration++;

            console.log("will stop: " + (the_array[iteration] == undefined));

            if(the_array[iteration] == undefined) { // reached the end
                console.log("Resolving...");
                resolve(error_array);
            } else {
                if(the_array[iteration] != undefined) {
                    console.log("Next: " + iteration);
                    notifications_for_each_in_array(iteration, the_array, error_array);
                } 
            }
            
            
        }).fail(function(err) {
            console.error(err);
        });
    });
}

The above function works, but when I call it with a .then(), the .then() block does not run. In this example, I never get alerted (PS: I also tried defining notifications_for_each_in_array as an async function, and using await, but I get the same result):

notifications_for_each_in_array(0, [0,1,2,3], [])
.then(function(res) {
    alert("here is then()"); // I never get alerted!
});

Add additional `.catch(function (txt) {console.log(txt)})`. Maybe your `notifications_for_each_in_array` fails somewhere inside `.done`? — Justinas, Jan 20 '21 at 14:40
You call `notifications_for_each_in_array(iteration, the_array, error_array)` recursively but don’t `resolve` it. — Sebastian Simon, Jan 20 '21 at 14:40
**Warning**: `$.ajax` returns a thennable, you have the [explicit constructor antipattern here](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) — Quentin, Jan 20 '21 at 14:41
You probably don't want to have both a `success` callback and a `done` callback — Heretic Monkey, Jan 20 '21 at 14:41
@SebastianSimon is correct. In addition, I'm not sure there is need for recursion here. Seems like this code is more complex than it needs to be. — VLAZ, Jan 20 '21 at 14:42
@SebastianSimon in the console, the function logs "Resolving..." only once, when (the_array[iteration] == undefined) == true — Krokodil, Jan 20 '21 at 14:42
@AlphaHowl: It may log that to the console, but does it log that to the console on the *very first* invocation of this function? If not then the promise was never resolved. — David, Jan 20 '21 at 14:44
@AlphaHowl but that's resolving a *different* promise, not the top level one. — VLAZ, Jan 20 '21 at 14:44
@HereticMonkey, sorry for the confusion, I don't actually do anything in the success func, I just added it for completion... — Krokodil, Jan 20 '21 at 14:44
@AlphaHowl So? If the inner call `notifications_for_each_in_array(iteration, the_array, error_array)` resolves at `resolve(error_array)`, where does the resolved value go? It is discarded, because nothing propagates the result of that call back to the original call. — Sebastian Simon, Jan 20 '21 at 14:45
Your're right, thank you. But how would I resolve the outer-layer Promise? — Krokodil, Jan 20 '21 at 14:46
Why are you trying to do this recursively, when a simple loop will do? — Jamiec, Jan 20 '21 at 14:47
@AlphaHowl: I'd take a step back and question if the recursion is even necessary. But at a glance something like this might work: `notifications_for_each_in_array(iteration, the_array, error_array).then(resolve)` It's worth noting however that there are still unresolved/unrejected code paths. — David, Jan 20 '21 at 14:47
With the recursion, each time, the function generates a different URL — Krokodil, Jan 20 '21 at 14:48
@AlphaHowl how does that require recursion - why don't you include thelogic for generating that URL - I guarantee you theres nothing special about recursion that makes it necessary — Jamiec, Jan 20 '21 at 14:54
@David the reference to resolve has to be within the outer promise and outside the inner promise, but i only want to resolve the outer promise when i get the result from the inner promise... So that is what I'm struggling with — Krokodil, Jan 20 '21 at 14:55
@Jamiec it simply adds `"/user-"+the_array[iteration]` to the end — Krokodil, Jan 20 '21 at 14:57
@AlphaHowl: The promise would need to be resolved at every level for that resolution to continue up the recursive stack. Not just the "outer promise" but all of them. So when you recursively call the function and that creates a new promise, you need to "await" that promise and resolve the current one when it's done. That's what chaining `.then()` to that created promise is meant to accomplish. — David, Jan 20 '21 at 14:57
@AlphaHowl - exactly! That doesnt need recursion if you're using a loop. See my answer — Jamiec, Jan 20 '21 at 14:58
@Jamiec - Yes, that works! I did complicate my code far too much, thank you! — Krokodil, Jan 20 '21 at 15:11

Jamiec · Accepted Answer · 2021-01-20T15:20:24.463

Your code is needlessly complicated. Essentially all you should need to do is loop through the input array and use async/await to run them all.

Here is a demo that demonstrates just how easy this should be to do in a loop. Your $.ajax call supports async/await as shown by my fake call to httpbin. Note also that I never explicitly create a new Promise this is all done for you by declaring the function as async.

async function notifications_for_each_in_array(arr) {
  const errors = [];
  for(var i=0;i<arr.length;i++) {  
     var result = await makeAjaxCall(arr[i]);
     errors.push("response from:" + result.args.id);
  }
  return errors;
}

const makeAjaxCall = (data) => {
  return $.ajax({
    url:"https://httpbin.org/get?id=" + data,
    method:"GET",
    type:"JSONP"
  })
}

const demo = async () => {
  var result = await notifications_for_each_in_array([0,1,2,3]);
  console.log(result);
}

demo()

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

You should also note that there is actually no need to await each http call before doing the next. You could make them all in parallel!

async function notifications_for_each_in_array(arr) {
  const errors = await Promise.all(
    arr.map(async data => {    
      const res = await makeAjaxCall(data)
      return "response from: " + res.args.id;    
    })
  );
  
  return errors;
}

const makeAjaxCall = (data) => {
  return $.ajax({
    url:"https://httpbin.org/get?id=" + data,
    method:"GET",
    type:"JSONP"
  })
}

const demo = async () => {
  var result = await notifications_for_each_in_array([0,1,2,3]);
  console.log(result);
}

demo()

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

Yes, that works! I did complicate my code far too much, thank you! — Krokodil, Jan 20 '21 at 15:11
@AlphaHowl you're welcome. Another thing to note is that you dont need to make each http call in series, you could do them all in parallel. — Jamiec, Jan 20 '21 at 15:18

Why is .then() not executing at all for my Promise?

1 Answers1