Skip to content

Suppress 403 error when sending DEFCON reject DM#1711

Merged
MarkKoz merged 2 commits into
mainfrom
bug/mod/bot-137/defcon-reject-dm-403
Aug 4, 2021
Merged

Suppress 403 error when sending DEFCON reject DM#1711
MarkKoz merged 2 commits into
mainfrom
bug/mod/bot-137/defcon-reject-dm-403

Conversation

@MarkKoz
Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz commented Aug 3, 2021

Sentry Issue: BOT-37

403 occurs if the user has DMs disabled. The specific code is 50007, but I don't think 403 would be raised for any other reason here, so I didn't check the code explicitly. If someone has more insight on this, please comment.

403 occurs if the user has DMs disabled.

Fixes BOT-137
@MarkKoz MarkKoz added a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority t: bug Something isn't working labels Aug 3, 2021
@NIRDERIi
Copy link
Copy Markdown
Contributor

NIRDERIi commented Aug 3, 2021

Wouldn't it require to handle discord.HTTPException? Or I totally misunderstood you

@wookie184
Copy link
Copy Markdown
Contributor

Wouldn't it require to handle discord.HTTPException? Or I totally misunderstood you

discord.Forbidden is a subclass of discord.HTTPException, httpexception could be any response from the api saying it didn't work, e.g. a random error with the Discord API, while Forbidden is more specific, and as in that part of the code we only want to catch disabled DMs, I believe Forbidden is correct.

If you were instead referring to replacing the general Exception with discord.HTTPException, then yeah, that should probably work from what I can see, although the logger logs an exception in that instance anyway, and as the comment says, sending the DM is less important than kicking the user, so we want to avoid the code failing at that point as much as possible.

Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of any other reason the bot should get a Forbidden error here. lgtm

@mbaruh
Copy link
Copy Markdown
Member

mbaruh commented Aug 4, 2021

Just for my understanding, the issue here is that log.exception opens an issue on Sentry? as functionality-wise it seems to be working fine already.

@MarkKoz
Copy link
Copy Markdown
Contributor Author

MarkKoz commented Aug 4, 2021

Just for my understanding, the issue here is that log.exception opens an issue on Sentry? as functionality-wise it seems to be working fine already.

The issue being fixed is 403 Forbidden errors being logged as exceptions and creating issues on Sentry.

Copy link
Copy Markdown
Member

@mbaruh mbaruh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Looks good to me

@MarkKoz MarkKoz enabled auto-merge August 4, 2021 15:47
@MarkKoz MarkKoz disabled auto-merge August 4, 2021 15:47
@MarkKoz MarkKoz merged commit c390ca4 into main Aug 4, 2021
@MarkKoz MarkKoz deleted the bug/mod/bot-137/defcon-reject-dm-403 branch August 4, 2021 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority t: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants