Any large update to a program will be accompanied by the discovery of bugs. Here are some that were present in Mirai's 4.11 update, and how they were resolved.
I'll start with a simple one. Permission checks are often present before code that sends requests to Discord. This prevents our logs from being spammed with 401 (Access Denied) errors because of users setting up permissions wrong. Let's use this as an example:
let permissions = channel.permissionsOf(this.bot.user.id);
if (!permissions.has('readMessageHistory') || !permissions.has('sendMessages'))
continue;
This is taken from the MOTD module. It's supposed to check that we can get the past messages in a channel and then send the MOTD message if needed. Despite this, we still get Access Denied. To solve this we need to also check for the readMessages permission.
A lesson you can learn from this is to not make assumptions. I assumed that not having the readMessages permission would mean I also wouldn't have those permissions.
According to the command help, using order:likes should sort the results by likes, but it doesn't. Let's take a look at the line that checks for a order tag.
let sortIndex = tags.findIndex(t => /^sort:(recent|likes|oldest)$/.test(t)),
And there's the problem, we check for sort, not order. This is fixed by updating the command help, but the issue still persists in testing with sort. If we move down to the actual request we see this:
async searchPosts(tags, sort = 'recent', nsfw = false) {
let resp = await axios.post('https://nekos.brussell.me/api/v1/images/search',
{ nsfw, tags: tags.join(','), limit: 20 },
{ headers: { 'User-Agent': "don't copy my code"} });
I must have been really tired, because although a sort parameter is passed, it's never actually used. Adding it to the axios parameters fixes the issue. This shows why you shouldn't rush testing, as it could have been easily noticed before release.
Another example of insufficient testing, the about command included a line of code that was not updated after a breaking change in mirai-bot-core. mirai.commandPlugins was changed from an array to an object. The fix was fairly simple, so I'll just show it below.
- In this version I have **${this.parent.bot.commandPlugins.reduce((t, p) => t ...
+ In this version I have **${Object.values(this.parent.bot.commandPlugins).redu...
There was a problem in the function that gets a list of users and their MAL username to update. Here's the code:
const supporters = (await this.parent.database.models.keys.distinct('ownerId', { ownerId: { $exists: true, $ne: null } }).lean()).map(k => k.ownerId);
if (supporters.length === 0)
return;
let users = await this.parent.database.models.userSettings.find({
id: { $in: supporters },
malUsername: { $exists: true, $nin: null }
}).lean();
There are two issues here. First, the first line doesn't need a lean() or map(), since distinct will already return an array of ownerIds. This was either leftover from a replaced find() query, or I just assumed that it would return results similar to find().
Secondly, a case of copy-pasting code. { $exists: true, $nin: null } should have $ne (not equal to) instead of $nin (not in array). Here's the fixed code.
const supporters = await this.parent.database.models.keys.distinct('ownerId', { ownerId: { $exists: true, $ne: null } });
if (supporters.length === 0)
return;
let users = await this.parent.database.models.userSettings.find({
id: { $in: supporters },
malUsername: { $exists: true, $ne: null }
}).lean();
After looking at the code I figured out this was because a user had failed to enter a valid time after three tries (the maximum). When this happens the askForTime loop exist without having set time to a valid time. Now, there are two places where this loop exists, which I seem to have forgotten when writing the code. In the first occurrence there is a check for tries === 3, but since we used tries++ < 3 this wont work. The loop will see that tries is not less than 3, and then set tries to 4. That check is fixed by changing the condition to tries === 4 and setting the loop to use ++tries < 4 for readability. Looking at the other use of the loop I see that I forgot to actually put the check there. This is an example of why having duplicate code is bad.
This update included a re-write of how the Twitch module checks for updated streams. In the past we used a simple loop. No matter what happened in the update function it would always be called again after a set interval. In the new system the update function queues up the next update when it runs. The logic for this is shown below.
update(index)
...
index + 100 > channels.length?
yes: run update() in (channels / 100 * 1000)ms
no: run update(index + 100) in 1250ms
...
This introduces a problem though. If the update function somehow errors, it is possible for the next update to not be scheduled. To fix this we also schedule the next update when catching an error. You may be seeing the cause now. Say we have an error during the request to the Twitch API. This will throw an error and we'll enter the catch block. The catch block then sets the next update to run and logs the error. The problem is that the next update was already scheduled when the update started. Now we have created an entirely separate update loop that will continue to run and create new update loops. Visualized it looks like this:
Requests get queued faster than the ratelimiter can send them, leading to thousands of unfulfilled promises in memory. The solution I came up with was to assign all timers to a variable, and then run clearTimeout(that variable) before each new timer was created.