From 35c27c5741fc2bb7b8328b0327b8480d6f337692 Mon Sep 17 00:00:00 2001 From: Michael H Date: Wed, 8 Jan 2020 13:39:52 -0500 Subject: [PATCH] Be quieter in expected shutdown cases (#3261) * Be quieter in expected cases * lets put this in the log file * inline description use because setuptools entrypoint scripts are dumb * Another setuptools entrypoint related issue * maybe don't crash the bot on tasks * improve the handling a bit more + document some of the lower level bits from the perspective of 'why?' * Adding myself to codeowners on this one * Let's not clobber our exit code * And, there we go * finish that thought * right, I bumped the python version for (part of) this * Update redbot/__main__.py Co-Authored-By: jack1142 <6032823+jack1142@users.noreply.github.com> * Okay, we should be good now * correct exit code enum use * cosmetic * minor fix for linux and ctrl+c Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> --- .github/CODEOWNERS | 2 +- changelog.d/3261.misc.rst | 1 + redbot/__main__.py | 106 ++++++++++++++++++++++++++++---------- redbot/core/bot.py | 28 ++++------ redbot/setup.py | 7 ++- setup.cfg | 2 +- 6 files changed, 97 insertions(+), 49 deletions(-) create mode 100644 changelog.d/3261.misc.rst diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fd8cbe317..85f836a68 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -56,7 +56,7 @@ tests/cogs/downloader/* @jack1142 # Setup, instance setup, and running the bot setup.py @tekulvw redbot/__init__.py @tekulvw -redbot/__main__.py @tekulvw +redbot/__main__.py @tekulvw @mikeshardmind redbot/setup.py @tekulvw # Others diff --git a/changelog.d/3261.misc.rst b/changelog.d/3261.misc.rst new file mode 100644 index 000000000..0487d7519 --- /dev/null +++ b/changelog.d/3261.misc.rst @@ -0,0 +1 @@ +Be just a little less noisy in expected cases on shutdown/restart. diff --git a/redbot/__main__.py b/redbot/__main__.py index e8519fa46..929e97988 100644 --- a/redbot/__main__.py +++ b/redbot/__main__.py @@ -27,7 +27,7 @@ from redbot import _update_event_loop_policy, __version__ _update_event_loop_policy() import redbot.logging -from redbot.core.bot import Red +from redbot.core.bot import Red, ExitCodes from redbot.core.cli import interactive_config, confirm, parse_cli_flags from redbot.setup import get_data_dir, get_name, save_config from redbot.core import data_manager, drivers @@ -335,7 +335,7 @@ def handle_early_exit_flags(cli_flags: Namespace): if cli_flags.list_instances: list_instances() elif cli_flags.version: - print(description) + print("Red V3") print("Current Version: {}".format(__version__)) sys.exit(0) elif cli_flags.debuginfo: @@ -345,31 +345,56 @@ def handle_early_exit_flags(cli_flags: Namespace): sys.exit(1) -async def shutdown_handler(red, signal_type=None): +async def shutdown_handler(red, signal_type=None, exit_code=None): if signal_type: log.info("%s received. Quitting...", signal_type) - exit_code = 0 - else: + # Do not collapse the below line into other logic + # We need to renter this function + # after it interrupts the event loop. + sys.exit(ExitCodes.SHUTDOWN) + elif exit_code is None: log.info("Shutting down from unhandled exception") - exit_code = 1 - await red.logout() - await red.loop.shutdown_asyncgens() - pending = [t for t in asyncio.all_tasks() if t is not asyncio.current_task()] - [task.cancel() for task in pending] - await asyncio.gather(*pending, loop=red.loop, return_exceptions=True) - sys.exit(exit_code) + red._shutdown_mode = ExitCodes.CRITICAL + + if exit_code is not None: + red._shutdown_mode = exit_code + + try: + await red.logout() + finally: + # Then cancels all outstanding tasks other than ourselves + pending = [t for t in asyncio.all_tasks() if t is not asyncio.current_task()] + [task.cancel() for task in pending] + await asyncio.gather(*pending, return_exceptions=True) -def exception_handler(red, loop, context): +def global_exception_handler(red, loop, context): + """ + Logs unhandled exceptions in other tasks + """ msg = context.get("exception", context["message"]) - if isinstance(msg, KeyboardInterrupt): - # Windows support is ugly, I'm sorry - logging.error("Received KeyboardInterrupt, treating as interrupt") - signal_type = signal.SIGINT - else: - logging.critical("Caught fatal exception: %s", msg) - signal_type = None - loop.create_task(shutdown_handler(red, signal_type)) + # These will get handled later when it *also* kills loop.run_forever + if not isinstance(msg, (KeyboardInterrupt, SystemExit)): + log.critical("Caught unhandled exception in task: %s", msg) + + +def red_exception_handler(red, red_task: asyncio.Future): + """ + This is set as a done callback for Red + + must be used with functools.partial + + If the main bot.run dies for some reason, + we don't want to swallow the exception and hang. + """ + try: + red_task.result() + except (SystemExit, KeyboardInterrupt, asyncio.CancelledError): + pass # Handled by the global_exception_handler, or cancellation + except Exception as exc: + log.critical("The main bot task didn't handle an exception and has crashed", exc_info=exc) + log.warning("Attempting to die as gracefully as possible...") + red.loop.create_task(shutdown_handler(red)) def main(): @@ -391,11 +416,11 @@ def main(): data_manager.load_basic_configuration(cli_flags.instance_name) red = Red( - cli_flags=cli_flags, description=description, dm_help=None, fetch_offline_members=True + cli_flags=cli_flags, description="Red V3", dm_help=None, fetch_offline_members=True ) if os.name != "nt": - # None of this works on windows, and we have to catch KeyboardInterrupt in a global handler! + # None of this works on windows. # At least it's not a redundant handler... signals = (signal.SIGHUP, signal.SIGTERM, signal.SIGINT) for s in signals: @@ -403,15 +428,42 @@ def main(): s, lambda s=s: asyncio.create_task(shutdown_handler(red, s)) ) - exc_handler = functools.partial(exception_handler, red) + exc_handler = functools.partial(global_exception_handler, red) loop.set_exception_handler(exc_handler) - # We actually can't use asyncio.run and have graceful cleanup on Windows... - loop.create_task(run_bot(red, cli_flags)) + # We actually can't (just) use asyncio.run here + # We probably could if we didnt support windows, but we might run into + # a scenario where this isn't true if anyone works on RPC more in the future + fut = loop.create_task(run_bot(red, cli_flags)) + r_exc_handler = functools.partial(red_exception_handler, red) + fut.add_done_callback(r_exc_handler) loop.run_forever() + except KeyboardInterrupt: + # We still have to catch this here too. (*joy*) + log.warning("Please do not use Ctrl+C to Shutdown Red! (attempting to die gracefully...)") + log.error("Received KeyboardInterrupt, treating as interrupt") + loop.run_until_complete(shutdown_handler(red, signal.SIGINT)) + except SystemExit as exc: + # We also have to catch this one here. Basically any exception which normally + # Kills the python interpreter (Base Exceptions minus asyncio.cancelled) + # We need to do something with prior to having the loop close + log.info("Shutting down with exit code: %s", exc.code) + loop.run_until_complete(shutdown_handler(red, None, exc.code)) finally: + # Allows transports to close properly, and prevent new ones from being opened. + # Transports may still not be closed correcly on windows, see below + loop.run_until_complete(loop.shutdown_asyncgens()) + if os.name == "nt": + # *we* aren't cleaning up more here, but it prevents + # a runtime error at the event loop on windows + # with resources which require longer to clean up. + # With other event loops, a failure to cleanup prior to here + # results in a resource warning instead and does not break us. + log.info("Please wait, cleaning up a bit more") + loop.run_until_complete(asyncio.sleep(1)) + loop.stop() loop.close() + sys.exit(red._shutdown_mode.value) if __name__ == "__main__": - description = "Red V3" main() diff --git a/redbot/core/bot.py b/redbot/core/bot.py index 546ba293c..4ea8a9055 100644 --- a/redbot/core/bot.py +++ b/redbot/core/bot.py @@ -7,7 +7,7 @@ import shutil import sys from collections import namedtuple from datetime import datetime -from enum import Enum +from enum import IntEnum from importlib.machinery import ModuleSpec from pathlib import Path from typing import Optional, Union, List, Dict, NoReturn @@ -132,10 +132,6 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin): # pylint: d if cli_flags.owner and "owner_id" not in kwargs: kwargs["owner_id"] = cli_flags.owner - if "owner_id" not in kwargs: - loop = asyncio.get_event_loop() - loop.run_until_complete(self._dict_abuse(kwargs)) - if "command_not_found" not in kwargs: kwargs["command_not_found"] = "Command {} not found.\n{}" @@ -408,6 +404,9 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin): # pylint: d init_global_checks(self) init_events(self, cli_flags) + i18n_locale = await self._config.locale() + i18n.set_locale(i18n_locale) + self.add_cog(Core(self)) self.add_cog(CogManagerUI()) self.add_command(license_info_command) @@ -527,17 +526,6 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin): # pylint: d """ return await self._help_formatter.send_help(ctx, help_for) - async def _dict_abuse(self, indict): - """ - Please blame <@269933075037814786> for this. - - :param indict: - :return: - """ - - indict["owner_id"] = await self._config.owner() - i18n.set_locale(await self._config.locale()) - async def embed_requested(self, channel, user, command=None) -> bool: """ Determine if an embed is requested for a response. @@ -654,9 +642,9 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin): # pylint: d """ Sets shared API tokens for a service - In most cases, this should not be used. Users should instead be using the + In most cases, this should not be used. Users should instead be using the ``set api`` command - + This will not clear existing values not specified. Parameters @@ -1098,7 +1086,9 @@ class Red(RedBase, discord.AutoShardedClient): sys.exit(self._shutdown_mode) -class ExitCodes(Enum): +class ExitCodes(IntEnum): + # This needs to be an int enum to be used + # with sys.exit CRITICAL = 1 SHUTDOWN = 0 RESTART = 26 diff --git a/redbot/setup.py b/redbot/setup.py index 6b50def18..5f4e6c8dd 100644 --- a/redbot/setup.py +++ b/redbot/setup.py @@ -418,10 +418,15 @@ def backup(instance: str, destination_folder: Union[str, Path]) -> None: loop.run_until_complete(create_backup(instance, Path(destination_folder))) -if __name__ == "__main__": +def run_cli(): + # Setuptools entry point script stuff... try: cli() # pylint: disable=no-value-for-parameter # click except KeyboardInterrupt: print("Exiting...") else: print("Exiting...") + + +if __name__ == "__main__": + run_cli() diff --git a/setup.cfg b/setup.cfg index 5c39e5738..32408cef0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -109,7 +109,7 @@ test = [options.entry_points] console_scripts = redbot=redbot.__main__:main - redbot-setup=redbot.setup:cli + redbot-setup=redbot.setup:run_cli redbot-launcher=redbot.launcher:main pytest11 = red-discordbot=redbot.pytest