Promise inside of a Promise creates an endless loop

Question

I have a Promise setup to make an Http call like so (All code has been aggregated):

  callHttpClient({ method, endpoint, payload }) {
    return new Promise((resolve, reject) => {
      axios({
        method: method,
        url: endpoint,
        data: payload,
      }).then(
        (response) => {
          if (response.status == httpStatusCodes.OK) {
            if (response.data == undefined) {
              reject(response);
            } else {
              logEventMixIn.methods.pushEvent('ApiResponse','Success', true);
              resolve(response);
            }
          }
        },
      );
    });
  },

The true flag on the pushEvent will trigger another call inside of the logEventMixin.js file which will also end up returning a Promise:

pushEvent(category, action, shouldLogToDatabase) {
  var payload = {
    userId: getUserId(),
    sessionKey: getSessionKey(),
    pageName: getPageName(),
    sessionId: getSessionId(),
  };

  if(shouldLogToDatabase) {

  httpHelperMixIn.methods.callEndpoint("https://pushEventEndpoint.com", payload); 
   // ** ^^ This returns a Promise ^^ **

  }

  ....
},

The problem is when I pass my flag as true to this method, I actually end up in an endless loop calling the same API endpoint thousands of times. I think it has something to do with the Promise chain, since if I place another Axios call directly in the logEventMixin.js, I don't get the issue anymore (like so)

pushEvent(category, action, shouldLogToDatabase) {
  var payload = {
    userId: getUserId(),
    sessionKey: getSessionKey(),
    pageName: getPageName(),
    sessionId: getSessionId(),
  };

  if(shouldLogToDatabase) {

  axios({
    method: "POST",
    url: "https://pushEventEndpoint.com",
    data: payload,
  }).then((response) => {
    Promise.resolve(response);
  }

What am I doing wrong?

(It's probably not the issue, but see [*What is the explicit promise construction antipattern and how do I avoid it?*](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it)) — T.J. Crowder, Apr 29 '22 at 14:41
I don't see anything here that would be causing it, it's likely to be further up the call stack, i.e. whatever is calling `pushEvent` — andy mccullough, Apr 29 '22 at 14:43
@andymccullough My callstack would be something like so: `httpHelperMixIn.methods.callEndpoint()` --> returns the Promise from `callHttpClient` ie the `return new Promise((resolve, reject) => ...` — 845614720, Apr 29 '22 at 14:45
In order to loop, there must either be a looping construct like `while(somethingAlwaysTrue)`, or a recursion: like A calls A, or (the pernicious kind you probably have) an indirect recursion, where A calls B calls C calls A. The functions in the OP code don't refer to each other at all. The only thing visible in the code is an ill-advised promise wrapping a promise. — danh, Apr 29 '22 at 15:37
If I had to wager a guess, I'd say that `httpHelperMixIn.methods.callEndpoint` is making some other API call (wager: for auth) which indirectly circles back on `pushEvent`. Step through, or console log at the top of methods and you might find a cycle. — danh, Apr 29 '22 at 15:40
@danh `callEndpoint` calls `callHttpClient()` which then calls `pushEvent()` — 845614720, Apr 29 '22 at 16:31
More specifically because this is a Vue app. CallEndpoint calls `store.dispatch()` (Vuex) which returns `callHttpClient()` which returns the promise all the back up the chain (presumably). — 845614720, Apr 29 '22 at 16:44
I didn't see the pushEvent in callHttpClient! Isn't that the answer? callEndpoint calls callHttpClient() which then calls pushEvent() WHICH THEN CALLS callEndpoint (forgive the caps :-)) isn't that it? — danh, Apr 29 '22 at 16:52
Yes! I guess now the question is how do I do this without the infinite loop so that all Promises resolve? — 845614720, Apr 29 '22 at 16:55
So the cycle is this: call an api, then call an api to log the completion of the api, then call to log the completion of that last one, and so on. That's what we must fix. `logEventMixIn.methods.pushEvent('ApiResponse','Success', **false**);` might do it, but this is a logic problem, not a promise problem. (though, to repeat, the code's use of promises is ill-advised and should also be fixed) — danh, Apr 29 '22 at 17:26
@danh Right, exactly what I was thinking (using a bool). what is your advice on the Promise to fix it? — 845614720, Apr 29 '22 at 17:30
@845614720 - posted a summary of the logic issue and a suggestion about promise style, with a sort of hand wave at error handling. — danh, Apr 29 '22 at 18:23

score 0 · Answer 1 · answered Apr 29 '22 at 14:53

I think you are not handling the Promise fully, try -

callHttpClient({ method, endpoint, payload }) {
    return new Promise((resolve, reject) => {
      axios({
        method: method,
        url: endpoint,
        data: payload,
      }).then(
        (response) => {
          if (response.status == httpStatusCodes.OK) {
            if (response.data == undefined) {
              reject(response);
            } else {
              logEventMixIn.methods.pushEvent('ApiResponse','Success', true).then(() => {
                 resolve(response); // <!-- you should be resolving only when `pushEvent` resolves
              });
              
            }
          }
        },
      );
    });
  },

and also -

pushEvent(category, action, shouldLogToDatabase) {
  var payload = {
    userId: getUserId(),
    sessionKey: getSessionKey(),
    pageName: getPageName(),
    sessionId: getSessionId(),
  };

  if(shouldLogToDatabase) {
     return httpHelperMixIn.methods.callEndpoint("https://pushEventEndpoint.com", payload); 
   // you need to return the above Promise....
  }

  // also return a Promise here for your other code...
  ....
},

Thanks for the response! The only problem here is that I don't care about the response of the Promise from `return httpHelperMixIn.methods.callEndpoint("https://pushEventEndpoint.com", payload); `. It's a logging call that is meant to be just fire and forget. — 845614720, Apr 29 '22 at 14:58

score 0 · Answer 2 · answered Apr 29 '22 at 18:22

We've determined in comments that the source of the loop is indirect recursion, wherein the making a remote call to log a successful remote call generates a loop. The fix is to not log the logs.

The issues with promise code are mostly stylistic, but important because at minimum they obscure the bugs. The following is roughly equivalent to the OP code, but more concise and stylistically correct...

async callHttpClient({ method, endpoint, payload }) {
  const response = await axios({
    method: method,
    url: endpoint,
    data: payload,
  });
  logEventMixIn.methods.pushEvent('ApiResponse','Success', false); // "fire and forget" don't await, and don't log the log
  return response;
}

async pushEvent(category, action, shouldLogToDatabase) {
  var payload = {
    userId: getUserId(),
    sessionKey: getSessionKey(),
    pageName: getPageName(),
    sessionId: getSessionId(),
  };

  if (shouldLogToDatabase) {
    return httpHelperMixIn.methods.callEndpoint("https://pushEventEndpoint.com", payload); 
  } else {
    // do something else with payload
    return Promise.resolve();
  }
}

Note that it makes no mention of error handling. The OP code doesn't either, but that shortcoming is disguised in the OP code with the presence of a reject here or there.

The better pattern on error handling is to throw when an error is found, and catch when something can be done about it, generally earlier in the call chain.

Promise inside of a Promise creates an endless loop

2 Answers2