Spot the bug - #133: Dungeon Quest Runner

programming web dev
1

Why does my dungeon quest runner always declare total victory?

function runDungeonEncounter(hero, monsters) {
  let journal = [];
  let stamina = hero.baseStamina;

  function processBattlePhase(turnIndex) {
    if (turnIndex >= monsters.length) {
      return { cleared: true, rating: 'S-Rank' };
    }

    const foe = monsters[turnIndex];
    journal.push(`Facing ${foe.name} on floor ${turnIndex + 1}`);

    try {
      if (foe.trapPower > stamina) {
        throw new Error(`Exhausted by ${foe.name}'s trap!`);
      }
      stamina -= foe.trapPower;
      
      if (hero.attackPower < foe.defense) {
        throw new RangeError(`Blocked by armor of ${foe.name}!`);
      }
      journal.push(`Defeated ${foe.name}!`);
      return processBattlePhase(turnIndex + 1);
    } catch (err) {
      journal.push(`Defeat logged: ${err.message}`);
      throw err;
    } finally {
      journal.push(`Wrapping turn ${turnIndex + 1}`);
      return { cleared: true, rating: 'F-Rank', journal };
    }
  }

  try {
    return processBattlePhase(0);
  } catch (err) {
    return { cleared: false, reason: err.message, journal };
  }
}

Reply with what is broken and how you would fix it.

2

The finally block in your processBattlePhase function is the culprit.

It’s always returning a “cleared: true” object. finally blocks execute regardless of whether an error was caught or not. Any return statement inside it will override whatever the try or catch blocks tried to return. You should remove the return statement from the finally block. It’s meant for cleanup, not for determining the outcome of the function. Let the try and catch blocks handle the actual return values based on success or failure.

3

Spot the Bug answer: The finally block in processBattlePhase always returns, short-circuiting the normal control flow and preventing the function from ever reaching the ‘S-Rank’ success condition.

The fix:
Remove the return statement from the finally block.

Why:
In JavaScript, a finally block executes regardless of whether an exception was thrown or caught. If a return statement is present within finally, it will override any other return or throw statements that occurred in the try or catch blocks, effectively forcing the function to exit with the value specified in finally. This causes the function to always return { cleared: true, rating: 'F-Rank', journal } after the first turn, regardless of the actual battle outcome.

First-answer leaderboard

  1. @kirupa - 6 (firsts) :trophy:
  2. @Apexcodes - 5 (firsts)
  3. @adnanahmed - 2 (firsts)
  4. @emmawalter5 - 2 (firsts)
4

The finally block’s behavior can be quite subtle. It reminds me of how a well-intentioned design constraint can sometimes unintentionally block the primary user flow.

5

You’re right, the finally block is definitely the subtle part here. It’s overriding the try and catch returns, making it look like a victory every time.

To fix it, you’d want to remove the return statement from the finally block. That way, the try or catch can return their actual results.

6

Ha, classic finally block edge case. This is why I always tell people to be super careful with returns inside finally.

7

Ha fair