Error handling -> Targets Issue Better Error Handling #13

[Gregory-Nitch]

Implements

Added cmd_err_handler.py in ./cogs overriding discord.py's on_command_error(). Added new cog to bot.

Removed if not self.game: checks from game_commands.py

Functionality

Here's what I have for some common errors right now. Message format can be changed if need be.

I was still able to get the 'game not loaded' message by forcing the game state to none. (More on this in Unit Test Update)

Further actions

Handle Base Error Case with log?

I was thinking it might be a good idea to still pass unhandled errors to the log, such as ExtensionErrors or others that may not be encountered as of now. elif branches could then be added to handle them as the project grows in scope.

However, I'm having trouble on accessing the log file, currently the base cases just send the error and type to the user. Will look back into it if logging unknown edge cases is something we want.

Add missing Error Types

There's a long list of error types for CommandErrors, I think I've gotten the most important ones, I'll go back through the list but feel free to point out any big ones I've missed.

Update Unit Tests

The unit tests for checking game state are currently failing due to the removal of the if not self.game: checks, I am trying to make replacements but its proving difficult. Still working on it.

Let me know what you think, -Greg

[codecov-commenter]

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (9960066) 82.79% compared to head (7225238) 79.47%.

Files	Patch %	Lines
src/word_debt_bot/cogs/cmd_err_handler.py	67.39%	15 Missing :warning:
src/word_debt_bot/cogs/game_commands.py	75.00%	6 Missing :warning:
src/word_debt_bot/main.py	0.00%	1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## dev #27 +/- ## ========================================== - Coverage 82.79% 79.47% -3.32% ========================================== Files 9 10 +1 Lines 279 307 +28 ========================================== + Hits 231 244 +13 - Misses 48 63 +15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Gregory-Nitch]

Update:

Added a if hasattr(ctx.command, "on_error"): to the begging of the handler, with this if the command has a local handler, it will be called instead.

Updated unit tests, test_submit_words__game() is still a placeholder, need to pass words but invoke() can only take ctx.

Added pytest fixture for the error handler, and added it as a cog for the test bot.

[Gregory-Nitch]

Update:

• Merged commits from Dev -> new buy & info command etc.
• Migrated buy & info command errors to error handler.
• Added ExpectedClosingQuoteError to handler
• Updated test_buy_with_value_error to use workaround like other unit tests needing an invoke error.

Bot looks to be handling buy errors appropriately in testing after the migration to the error handler and all unit tests are passing. I'll label the PR ready for review, I'm not the biggest fan of the unit test workaround either, if anything shows up on the SO question I'll let you know.

Pytest Warnings:

The pytest warning for a few of the unit tests;

RuntimeWarning: coroutine 'AsyncMockMixin._execute_mock_call' was never awaited

is listing view.skip_ws() as the function not awaited. This function is in discord.ext.commands.core. I went in and added await to the function and the warnings went away (however, a different unit test failed). I'm thinking the function's purpose is to skip over whitespaces given to commands. It would be ignorant of me to blame the discord library for the warnings, but it seems like it could be a possibility.

Thanks for your patience on this one.