Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tempban logging improvement #2993

Merged
merged 3 commits into from Sep 13, 2019
Merged

Conversation

DevilXD
Copy link
Contributor

@DevilXD DevilXD commented Sep 10, 2019

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

Added proper logging that possibly fixes #2912

@DevilXD DevilXD requested a review from palmtree5 as a code owner Sep 10, 2019
@DevilXD DevilXD requested a review from Twentysix26 as a code owner Sep 10, 2019
@DevilXD
Copy link
Contributor Author

@DevilXD DevilXD commented Sep 10, 2019

The changelog category is misc because this isn't really anything user-facing, just logging improvement.
The changelog entry itself might need a touchup too.

@Flame442 Flame442 added the Type: Enhancement label Sep 10, 2019
Copy link
Contributor

@mikeshardmind mikeshardmind left a comment

Retain the prior catch. you can get a 50013 json response as part of a 403 response as well as part of a 400 response, but it isnt the only possible failure which was being caught by forbidden.

This was also catching a lack of 2FA with 2FA requirement.

@DevilXD
Copy link
Contributor Author

@DevilXD DevilXD commented Sep 13, 2019

Doesn't discord.Forbidden inherit from discord.HTTPException tho? It should catch all of them.

Unless you mean like, the Forbidden should always make it log "due to permissions" occurrences, then I can just add or e.status == 403 to the check.

@mikeshardmind mikeshardmind added this to the 3.2.0 milestone Sep 13, 2019
@mikeshardmind mikeshardmind merged commit 77f1da3 into Cog-Creators:V3/develop Sep 13, 2019
1 check passed
@DevilXD DevilXD deleted the tempban-perm-fix branch Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants