Skip to content

Use error_embed for errors, tidy code, show cmd signature for user errors.#332

Merged
ikuyarihS merged 3 commits into
masterfrom
errorhandler-refine
Dec 15, 2019
Merged

Use error_embed for errors, tidy code, show cmd signature for user errors.#332
ikuyarihS merged 3 commits into
masterfrom
errorhandler-refine

Conversation

@scragly
Copy link
Copy Markdown
Contributor

@scragly scragly commented Dec 12, 2019

Related Issues

Closes #307

Description of Changes

  • CRLF (Windows) line separators changed to LF (Unix)
  • Create error_embed classmethod to build error embeds consistently
  • Make all handled error types use error_embed method for any user responses
  • Remove error-specific debug log messages and have a generic debug log at the top
  • Log unhandled errors with log.exception instead of using logging.warning
  • Have full traceback shown in stderr with log.exception by passing exc_info
  • Use math.ceil on retry_after instead of within f-string to be cleaner
  • Have the command signature sent to show how to use it when users invoke it incorrectly (UserInputError, BadArgument)
  • Make channel check errors delete themselves after 7.5 seconds (as per suggestion from lemon)

Reasoning

CRLF to LF

Our projects use LF line separators and it seems sometime in the past CRLF was accidentally committed. This was brought back inline for consistency.

Error message consistency

Some errors shown an embed, others didn't. This brings everything back together to be the same and to make use of the random error titles with ERROR_REPLIES used by default and NEGATIVE_REPLIES able to be used for errors caused by permissions/checks/cooldowns.

Error specific logs

If all are logged, there's really no need for each to have one.

Using log.exception with full traceback

Previously errors looked like this during development:

seasonalbot    | Ignoring exception in command roll:
seasonalbot    | 12/12/19 02:55:20 - root WARNING: Scragly#5146 called the command 'roll' however the command failed to run with the error:-------------
seasonalbot    | this is a test exception

To be honest, it's pretty useless. It doesn't tell us where the error is located, on what line and what lead up to the event.

With the changes we have instead:

seasonalbot    | 12/12/19 02:58:09 - root DEBUG: Error Encountered: OSError - this is a test exception, Command: roll, Author: Scragly#5146, Channel: bot
seasonalbot    | 12/12/19 02:58:09 - bot.seasons.evergreen.error_handler ERROR: Unhandled command error: this is a test exception
seasonalbot    | Traceback (most recent call last):
seasonalbot    |   File "/usr/local/lib/python3.7/site-packages/discord/ext/commands/core.py", line 79, in wrapped
seasonalbot    |     ret = await coro(*args, **kwargs)
seasonalbot    |   File "/bot/bot/seasons/evergreen/fun.py", line 38, in roll
seasonalbot    |     raise OSError("this is a test exception")
seasonalbot    | OSError: this is a test exception

Which is far more useful during development or even when just needing to look back on log output generally for unhandled cases.

Use math.ceil on retry_after

Doing so reduced the code from:

remaining_minutes, remaining_seconds = divmod(error.retry_after, 60)
return await ctx.send(
    "This command is on cooldown, please retry in "
    f"{int(remaining_minutes)} minutes {math.ceil(remaining_seconds)} seconds."
)

to the alternative which shortens the var names and removes the unnecessary int cast:

mins, secs = divmod(math.ceil(error.retry_after), 60)
embed = self.error_embed(
    f"This command is on cooldown:\nPlease retry in {mins} minutes {secs} seconds.",
    NEGATIVE_REPLIES
)

Show command signature on user invoke error

The current output when a command is used incorrectly is:
image

This isn't too useful to new users, as then they'd likely have to manually use .help commandname which is just another step, or they'd keep guessing.

Instead this was changed to show the proper usage:
image

Did you:

  • Join the PythonDiscord Community?
  • Make changes in the Pipenv environment?
  • Lint your code and make sure it passes? (pipenv run lint)

@ikuyarihS ikuyarihS merged commit 39fa525 into master Dec 15, 2019
@ikuyarihS ikuyarihS deleted the errorhandler-refine branch December 15, 2019 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Autodelete error messages

3 participants