Skip to content

Don't log exception traceback on Forbidden for welcomes.#772

Merged
sco1 merged 1 commit into
masterfrom
dm_failure_fix
Feb 23, 2020
Merged

Don't log exception traceback on Forbidden for welcomes.#772
sco1 merged 1 commit into
masterfrom
dm_failure_fix

Conversation

@scragly
Copy link
Copy Markdown
Contributor

@scragly scragly commented Feb 23, 2020

Closes #763.

Description

Members have a welcome DM sent to them on accepting rules, which may raise a discord.Forbidden exception in the case they have DMs disabled in their privacy settings.

The current implementation catches a broad Exception and uses log.exception to allow the error to be logged fully with traceback as an exception without interrupting the code.

This is an unnecessary log since it's something by design, not an error, and the broad Exception catching is against the more explicit exception catching we prefer.

Changes

The except block only catches discord.Forbidden for the single known case we're intending to handle and suppress.

To ensure the original accept command message is still removed, it's placed in a finally block. This allows unknown errors to raise correctly and propagate to the appropriate error logging and handling.

@scragly scragly requested a review from a team as a code owner February 23, 2020 10:38
@scragly scragly requested review from MrHemlock and jerbob and removed request for a team February 23, 2020 10:38
Copy link
Copy Markdown
Contributor

@SebastiaanZ SebastiaanZ left a comment

Choose a reason for hiding this comment

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

Looks good.

@scragly scragly added a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority status: needs review t: bug Something isn't working labels Feb 23, 2020
@sco1 sco1 merged commit 43cda48 into master Feb 23, 2020
@sco1 sco1 deleted the dm_failure_fix branch February 23, 2020 13:15
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.

Verify command shouldn't log tracebacks for DM failure.

3 participants