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

@@ -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

changelog.d/3261.misc.rst

@@ -0,0 +1 @@
+Be just a little less noisy in expected cases on shutdown/restart.

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
+        # Do not collapse the below line into other logic
-    else:
+        # 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
+        red._shutdown_mode = ExitCodes.CRITICAL
+
+    if exit_code is not None:
+        red._shutdown_mode = exit_code
+
+    try:
         await red.logout()
-    await red.loop.shutdown_asyncgens()
+    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, loop=red.loop, return_exceptions=True)
+        await asyncio.gather(*pending, return_exceptions=True)
-    sys.exit(exit_code)
 
 
-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):
+    # These will get handled later when it *also* kills loop.run_forever
-        # Windows support is ugly, I'm sorry
+    if not isinstance(msg, (KeyboardInterrupt, SystemExit)):
-        logging.error("Received KeyboardInterrupt, treating as interrupt")
+        log.critical("Caught unhandled exception in task: %s", msg)
-        signal_type = signal.SIGINT
+
-    else:
+
-        logging.critical("Caught fatal exception: %s", msg)
+def red_exception_handler(red, red_task: asyncio.Future):
-        signal_type = None
+    """
-    loop.create_task(shutdown_handler(red, signal_type))
+    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...
+        # We actually can't (just) use asyncio.run here
-        loop.create_task(run_bot(red, cli_flags))
+        # 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()

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.
@@ -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

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()

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