-
-
Notifications
You must be signed in to change notification settings - Fork 104
Description
Issue
When using the command /reminder list to show all current pending reminders for the user, the command throws an exception when the user has no pending reminders at the moment.
StackTrace
java.lang.IllegalArgumentException: 1 > 0
at java.base/java.lang.Math.clamp(Math.java:2221)
at org.togetherjava.tjbot.features.reminder.ReminderCommand.createPendingRemindersPage(ReminderCommand.java:223)
at org.togetherjava.tjbot.features.reminder.ReminderCommand.handleListCommand(ReminderCommand.java:207)
at org.togetherjava.tjbot.features.reminder.ReminderCommand.onSlashCommand(ReminderCommand.java:119)
at org.togetherjava.tjbot.features.system.BotCore.lambda$onSlashCommandInteraction$0(BotCore.java:260)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1090)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:614)
at java.base/java.lang.Thread.run(Thread.java:1474)Reproduce
Start the bot locally, do not use /reminder create. Instead, use /reminder list right away. Thats all, the exception should be thrown now.
Bug
The issue is that this code section in ReminderCommand.java:223
calls Math.clamp(x, 1, 0), i.e. min = 1 and max = 0. That makes little sense, hence the crash.
Fix
Funnily enough the code is already prepared to handle the case correctly, just a few lines later it has
if (pendingReminders.isEmpty()) {
remindersEmbed.setDescription("No pending reminders");We just have to make sure that totalPages is at least 1. That can also be justified logically as even with nothing to show, we at least want to show a single page. So we just change
int totalPages = Math.ceilDiv(pendingReminders.size(), REMINDERS_PER_PAGE);into
int totalPages = Math.ceilDiv(pendingReminders.size(), REMINDERS_PER_PAGE);
totalPages = Math.max(1, totalPages);which ensures it is at least 1. The rest of the code should then work out of the box already.
