0

I wrote a function in Discord.js v13 that expects user input. If the input is invalid, it needs to re-prompt them again, and the original promise should no longer be considered.

If the user takes longer than timeoutSeconds, then it throws an InputTimeout error.

What happens in practice, which I don't understand, is this:

  1. getResponse(...) is executed
  2. User is prompted for input
  3. User supplies invalid input
  4. User is informed of invalid input, and is re-prompted
  5. If no input is supplied, an InputTimeout error is thrown from the first request and the second request

What I wish to happen is:

  1. getResponse(...) is executed
  2. User is prompted for input
  3. User supplies invalid input
  4. User is informed of invalid input, and is re-prompted
  5. If no input is supplied, an InputTimeout error is thrown for the new request only

Here is what I think is happening... The second prompt is throwing the InputTimeout error which is causing the catch() handler from the first Promise to fire off. If that's true, then I am confused, because I thought if then() runs, then it's because no error was thrown. So why is catch() catching errors from the then() function? Should put catch() first, before then()? Is this an issue with the order I've chained the functions?

async getResponse(user, timeoutSeconds, channel = null) {
        if (channel == null) {
            const msg = await user.send(this.promptMessage);
            channel = msg.channel;
        }
        return await channel
            .awaitMessages({
                max: 1,
                time: timeoutSeconds * 1000,
                errors: ["time"],
            })
            .then(async (messages) => {
                if (messages.size == 1) {
                    const message = messages.values().next().value;
                    if (this.validator(message)) {
                        await user.send(this.successMessage);
                        return this.formatter(message);
                    } else {
                        await user.send(this.errorMessage);
                        await this.getResponse(
                            user,
                            timeoutSeconds,
                            channel
                        );
                    }
                }
            })
            .catch(async (e) => {
                console.error(e);
                await channel.send(this.timeoutMessage);
                throw new InputTimeout();
            });
    }
gwizzo
  • 41
  • 7
  • 4
    Avoid [mixing `await` with `.then(…)` syntax](https://stackoverflow.com/a/54387912/1048572)! – Bergi Mar 27 '22 at 18:54

1 Answers1

3

The issue was in fact the order of the .then() and .catch(). Since I was only concerned with catching errors from the awaitMessages() function, I should've chained it to that first, followed by the .then().

By putting catch() after then(), I was catching errors from my then() function.

Doh!

New solution, which also gets rid of .then() and .catch() as suggested by community:
try {
    let messages = await channel.awaitMessages({
        max: 1,
        time: timeoutSeconds * 1000,
        errors: ["time"],
    });
    messages = Array.from(messages.values());
    if (messages.length == 1) {
        const message = messages[0];
        if (message.author.bot) return;
        if (this.validator(message)) {
            await user.send(this.successMessage);
            return this.formatter(message);
        } else {
            await user.send(this.errorMessage);
            return this.getResponse( // note I removed "await" here to prevent nested errors
                user,
                timeoutSeconds,
                channel
            );
        }
    }
} catch (e) {
    console.error(e);
    await channel.send(this.timeoutMessage);
    throw new InputTimeout();
}
gwizzo
  • 41
  • 7
  • 3
    Mixing `await` with `.then()` is an error-prone practice. – Pointy Mar 27 '22 at 18:50
  • 2
    You also might want to [use `.then(…, …)` instead of `.then(…).catch(…)`](https://stackoverflow.com/q/24662289/1048572). But yeah, if your `catch` handler always re-throws an error, `.catch(…).then(…)` will also work fine. – Bergi Mar 27 '22 at 18:58
  • @Pointy By that you mean the `return await` part, or the `await` within the `.then()` function? Or both? – gwizzo Mar 29 '22 at 17:06
  • The whole point of `async` functions and `await` is to make using Promise objects easier. Instead of `.then()` callbacks, you can use `await`, which will essentially turn the rest of your function into a `.then()` function body. – Pointy Mar 29 '22 at 17:15
  • @Pointy Ok, I am following. I got confused as to why they both existed but it looks like it's just two of the same thing, just different syntax. I prefer `try / catch` so I just updated the function to use that across the board. Thanks! – gwizzo Mar 29 '22 at 19:39