NeuroAssassin / Toxic-Cogs

A collection of utility, moderation and fun plugins to Red - Discord Bot.
MIT License
48 stars 38 forks source link

updatechecker doesn't appear to handle team ownership #84

Closed tigattack closed 1 year ago

tigattack commented 1 year ago

I have one bot owned by a team and another owned by a single user, the former fails to function and the latter works as expected. Both have identical configurations as far as this cog goes.

Here's an example of how such a situation is handled in Red's code https://github.com/Cog-Creators/Red-DiscordBot/blob/V3/develop/redbot/core/core_commands.py#L419-L423
This is for owner/team name, rather than messaging, but maybe a handy reference nonetheless.

Traceback (most recent call last):
  File "/redbot/data/cogs/CogManager/cogs/updatechecker/updatechecker.py", line 141, in bg_task
    await channel.send(embed=e)
  File "/home/redbot/.local/lib/python3.11/site-packages/discord/abc.py", line 1561, in send
    data = await state.http.send_message(channel.id, params=params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/redbot/.local/lib/python3.11/site-packages/discord/http.py", line 738, in request
    raise Forbidden(response, data)
discord.errors.Forbidden: 403 Forbidden (error code: 50007): Cannot send messages to this user

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/redbot/data/cogs/CogManager/cogs/updatechecker/updatechecker.py", line 185, in bg_task
    await owner.send(
  File "/home/redbot/.local/lib/python3.11/site-packages/discord/abc.py", line 1561, in send
    data = await state.http.send_message(channel.id, params=params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/redbot/.local/lib/python3.11/site-packages/discord/http.py", line 738, in request
    raise Forbidden(response, data)
discord.errors.Forbidden: 403 Forbidden (error code: 50007): Cannot send messages to this user

image

tigattack commented 1 year ago

Perhaps deprecating explicit DM notifications in favour of the Red.send_to_owners function would be more appropriate. Certain cleaner to handle in the code.

I'm happy to implement a fix for this, just let me know how which approach you'd prefer @NeuroAssassin.

  1. Fix the current functionality to support team ownership
  2. Swap the DM update channel option to use the bot's configured owner notification channel (defaults to DMing the owner(s)) instead, with help message/documentation changes to match of course.

The latter would be my personal preference, but it's your cog :)

Flame442 commented 1 year ago

The particular error you posted is because the cog does not properly handle cases where the bot cannot send a message to a particular channel. For users, that happens if the bot is blocked or if the user has "Allow direct messages" set or not. For channels, that is based on the guild permissions.

That being said, the cog also does not properly handle any custom Red ownership settings, instead always defaulting to the application owner.

Addressed in #85.

tigattack commented 1 year ago

That's curious since I allow DMs and of course haven't blocked the bot. PR resolved my issue though, thanks!