JavaScript: Recursive function fails to handle Promises correctly

Question

I'm writint a simple game in Node. Player makes a move, which is issued as a promise. Next the move is checked for legality:

1. If legal - great, execute the next ".then"
2. If not - call the same function recursively to get a new move

What works: I manage to break out of the promises chain using "reject" + "catch" and recursively call the function.

What doesn't work: when _makeMove is called the second time, it's supposed to ask the player for a new move, pausing until they reply. What actually happens is the function just runs away to Step 3, not waiting for the player's entry.

I'm taking input through terminal, and the window is still at "please enter: ", but the code already ran away.. (ultimately hitting an "undefined" error, which is natural, as the player hasn't had a chance to enter a new move).

Code (simplified):

const _makeMove = (activePlayer) => {

    //player makes a move, which is returned as a promise
    activePlayer.proposeMove()

      //Step 1 - check if legal
      .then(proposedMove => {
        if (!gameBoard.checkLegal(proposedMove)) {
          return Promise.reject("bad entry");
        }
        return proposedMove;
      })

      //Step 2 - record the move
      .then(proposedMove => {
        activePlayer.recordMove(proposedMove);
        return proposedMove;
      }) //unless step 1 fails...
      .catch(err => {
        console.log('oops bad entry!')
        //in which case let's ask the player to move again...
        //by calling the function RECURSIVELY
        _makeMove(activePlayer);
      })

      //Step 3 - game goes on...
      .then(proposedMove => {
        //more stuff
      })
  }

I'm completely puzzled. Why is the recursive call not working as expected?

Answer 1

Thanks for the suggestions above. After tinkering, I ended up doing 2 things that solved it:

1. I rewrote my promise chain from a bunch of .then methods using async/await. This solved the original issue of the recursive call falling through to Step 3 and not pausing at Step 2 (which I still find magical..)
2. This however, created a new problem. Initially I chained a single .catch to the end of the async function, but that only triggered once. Ie 2nd, 3rd, 4th recursive calls would lead to Unhandled exception errors. So I instead placed all the code inside of a try block inside the function, and put a catch block also inside the function. This works beautifully

Final code:

const _makeMove = async (activePlayer) => {
    try {
      //Step 1 - take input
      const proposedMove = await activePlayer.proposeMove();

      //Step 2 - check if legal
      if (!gameBoard.checkLegal(proposedMove)) {
        throw new Error("bad entry");
      }

      //Step 3 - etc
      //Step 4 - etc
      //Step 5 - etc

    } catch (e) {
      console.log('oops bad entry!')
      console.log("how it works: type 2 numbers ONLY, each between 1 and 3 (no spaces), to signify your move")
      console.log("eg to place a mark into bottom left corner type 33. First cell = 11. Bang in the center = 22. You get it.")
      console.log('lets try again...')
      return _makeMove(activePlayer);
    }
  };

Answer 2

In order for your promise chain to take the value from _makeMove, you have to return the result of _makeMove. Don't worry about the value being a promise; it will be automatically resolved before the next then in the chain is invoked.

      .catch(err => {
        console.log('oops bad entry!')
        return _makeMove(activePlayer);
        //  ^ return here
      })

However, the _makeMove result here will complete before the then is returned, which would likely make your recordMove call return twice. You might need to split into _makeMove and _recordMove functions so the recursive call to _makeMove would not record a move.

Though you could theoretically run out of stack, for a reasonable number of move attempts it wouldn't affect the correctness. The two errors above would.

Answer 3

I'm not sure you should use recursive call here. And if you get a very dump player, you could reach a Stack Overflow error ;)

const _makeMove = async (activePlayer) => {
  let proposedMove = null;

  while (1) {
    proposedMove = await activePlayer.proposeMove();
    if (gameBoard.checkLegal(proposedMove)) {
       break;
    }
  }
  activePlayer.recordMove(proposedMove);
  // do more stuff
}