Skip to content

Gracefully handle non-existing infractions#1388

Merged
MarkKoz merged 3 commits into
python-discord:masterfrom
ChrisLovering:deal-with-unknown-infraction-IDs
Jan 29, 2021
Merged

Gracefully handle non-existing infractions#1388
MarkKoz merged 3 commits into
python-discord:masterfrom
ChrisLovering:deal-with-unknown-infraction-IDs

Conversation

@ChrisLovering
Copy link
Copy Markdown
Member

This closes #1375.

Catches the response code error and handles by raising a bad argument error

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 28, 2021

Coverage Status

Coverage decreased (-0.04%) to 56.593% when pulling acc8bcf on ChrisLovering:deal-with-unknown-infraction-IDs into 3d014d9 on python-discord:master.

@Xithrius Xithrius added a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) needs 2 approvals p: 2 - normal Normal Priority t: feature New feature or request labels Jan 29, 2021
Copy link
Copy Markdown
Contributor

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

Tested, looks good to me.

@ChrisLovering
Copy link
Copy Markdown
Member Author

Following discussions in #dev-contrib I moved this handling into error_handler.py. I copied a similar pattern to what was above, which should allow us to handle different types of inner errors from ConversionError in future as needed. Please let me know if this wasn't the expected implemention.

@Xithrius
Copy link
Copy Markdown
Contributor

Xithrius commented Jan 29, 2021

I'm pretty sure this is what was wanted in implementation, thanks for the quick change!

I will test a bit more and approve if everything is handled.

@ChrisLovering
Copy link
Copy Markdown
Member Author

My only concern is that it sorta duplicates the check on line 78, but that's from CommandInvokeError. I guess the comment on 95 should be changed to remove ConversionError too, looking at it again.

@Xithrius
Copy link
Copy Markdown
Contributor

I do agree that it does seem to be a little duplicated, but there's nothing really we can do about it to my knowledge since they're two different outer errors.

Yes, that comment should be updated.

Comment thread bot/exts/backend/error_handler.py Outdated
@MarkKoz MarkKoz merged commit 1db6f1d into python-discord:master Jan 29, 2021
@ChrisLovering ChrisLovering deleted the deal-with-unknown-infraction-IDs branch March 6, 2021 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) p: 2 - normal Normal Priority t: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trying to run a command with a non-existing infraction ID result in a traceback

4 participants