Why my promise cant resolve?

Question

I have got two promises. One is not being resolved and I dont know why.

processTradeOffer: chain of promises that tries to procced an object called 'offer'. identifyOffer return a var that could be "valida" | "aceptable" | "denegable". If is 'valida' we need to identify items from offer. So we need another async function that is identifyItems(offer) will be return var 'offerState' "denegable" | "aceptable" and then we can decline or accept offer.

I know that the problem there is not in statement (offerState == 'valida'). Code:

const processTradeOffer = function(offer) {
    return new Promise(function(resolve, reject) {
        identyOffer(offer)
            .then(function(offerState) {
                return finishTradeOffer(offer, offerState);
            }).then(function() {
                console.log('aqui');
                return resolve();
            })
    })
}
const finishTradeOffer = function(offer, offerState) {

    return new Promise(function(resolve, reject) {
        if (offerState == 'aceptable') {
            acceptTradeOffer(offer).then(function() {
                return resolve();
            })
        } else if (offerState == 'denegable') {
            declineTradeOffer(offer).then(function() {
                console.log('here');
                return resolve();
            })
        } else if (offerState == 'valida') {
            identifyItems(offer).then(function(offerState) {
                finishTradeOffer(offer, offerState);
            })
        }
    })
}

Console.log('here') is fired succesfully and Console.log('aqui') dont.

What's supposed to happen if `offerstate == 'valida'`? By the way, `.then(function() { return resolve(); })` is identical to `then(resolve)`, but you shouldn't be doing that anyway. — , Sep 13 '17 at 19:06
It looks like `identyOffer`, `acceptTradeOffer` etc. already return promises since you are calling `then()`. Is that right? — Mark, Sep 13 '17 at 19:08
Please add definitions of `identyOffer` and `finishTradeOffer`. — trincot, Sep 13 '17 at 19:17
I had the same program running fine with callbacks. I just want migrate to promises for improving my knowledge. Should I back to use callbacks? — Tomas Gonzalez, Sep 13 '17 at 19:18
Maybe some of the functions throw an error, but you don't know about that because you haven't `catch` callback. — alexmac, Sep 13 '17 at 19:19

score 2 · Answer 1 · answered Sep 13 '17 at 19:10

2

The problem is that in this block:

    } else if (offerState == 'valida') {
        identifyItems(offer).then(function(offerState) {
            finishTradeOffer(offer, offerState);
        })
    }

You're not calling either resolve() or reject() so the function falls through without calling either callback so, eventually, the "aqui" block never gets called.

answered Sep 13 '17 at 19:10

Prisoner

49,922
7
53
105

@trincot You have to call `resolve` in order to make a promise execute its `then` callback functions. – 4castle Sep 13 '17 at 19:34
I missed the dependency between the two functions. – trincot Sep 13 '17 at 19:37
No is not. Im calling the same promise until offerState = 'aceptable' or 'denegable'. Is that ok? – Tomas Gonzalez Sep 13 '17 at 19:41
While it is "ok", the problem is that when you call the function recursively, you still need to handle the result. The code from @torazaburo works because it returns the Promise that the recursive code returns. If you don't explicitly return something - you'll implicitly fall through and return nothing. – Prisoner Sep 13 '17 at 20:16

score 1 · Answer 2 · 2017-09-14T03:34:52.523

Please write your code as follows.

const processTradeOffer = function(offer) {
  const result = identyOffer(offer)
    .then(offerState => finishTradeOffer(offer, offerState))

  result.then(() =>  console.log('aqui'));

  return result;
 };

const finishTradeOffer = function(offer, offerState) {
  switch(offerState) {
    case 'aceptable': return acceptTradeOffer(offer);

    case 'denegable': {
      const result = declineTradeOffer(offer);
      result.then(() => console.log('here'));
      return result;

    case 'valida':
      return identifyItems(offer)
        .then(offerstate => finishTradeOffer(offer, offerState));

    default: 
      throw "Invalid value for offerstate!!";
  }
};

The basic point is to handle the case of valida in such a way that the promise resolves. In addition, we've gotten rid of the "explicit promise constructor anti-pattern".

score 1 · Accepted Answer · answered Sep 13 '17 at 19:25

1

First of all avoid using Promise constructor antipattern. Your functions are already return promises.

Add catch callback to handle possible errors.

const processTradeOffer = function(offer) {
  return identyOffer(offer)
    .then(function(offerState) {
      return finishTradeOffer(offer, offerState);
    })
    .then(function() {
      console.log('aqui');
    })
    .catch(err => console.log(err));
}

const finishTradeOffer = function(offer, offerState) {
  switch (offerState) {
    case 'aceptable':
      return acceptTradeOffer(offer);
    case 'denegable':
      return declineTradeOffer(offer);
    case 'valida':
      return identifyItems(offer)
        .then(function(offerState) {
          return finishTradeOffer(offer, offerState);
        });
    default:
      return Promise.resolve();
}

answered Sep 13 '17 at 19:25

alexmac

19,087
7
58
69

I'm not too thrilled about this because the value of the promise returned by `processTradeOffer` will be the return value of `console.log`, which will be `undefined`, instead of the return value from the call to `finishTradeoffer`, which is very likely what is desired. And any rejection will be caught by the `catch` and turned into a success, again with the value of `console.log(err)`, which is unlikely to be what you want. – Sep 13 '17 at 19:41
Ok. This is the best solution for me. I definitely need to read more about Promises. Just one more question: acceptTradeOffer and declineTradeOferr are async function that can fail. In that case the error wil be catched? – Tomas Gonzalez Sep 13 '17 at 19:48
@torazaburo I agree that `processTradeOffer` returns a promise with `undefined` value, but it's OP code, maybe he doesn't use this result. The question was about that why _console.log('aqui');_ is not called, I tried to describe why. What about `catch` block, I repeat again, I don't know how `processTradeOffer` function is used. If it's called independently, then this `catch` is valid. – alexmac Sep 13 '17 at 19:50
@TomasGonzalez Of course, all thrown errors will be handled in the next `catch` callback. In my example, this callback is for all functions. – alexmac Sep 13 '17 at 19:52
@alexmac nice answer like always! – Ahmed Can Unbay Sep 13 '17 at 20:35
@TomasGonzalez This seems to be very close to an exact copy of my earlier answer, with the exception that it returns useless promise values due to the way the `console.log` statements are done, and questionable design decisions such as returning an empty resolved promise for an invalid value of `offerState`. – Sep 14 '17 at 03:17

Why my promise cant resolve?

3 Answers3