From e8c044a9bf6f1178e52c4bd5301a3bddfd965f4b Mon Sep 17 00:00:00 2001 From: Jakub Kuczys Date: Sun, 25 Dec 2022 22:27:07 +0100 Subject: [PATCH] Use different exit codes for critical errors vs configuration errors (#5674) Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> --- docs/autostart_systemd.rst | 5 +++-- redbot/__init__.py | 2 +- redbot/__main__.py | 28 ++++++++++++++-------------- redbot/core/bot.py | 12 ++---------- redbot/core/cli.py | 23 +++++++++++++++++++++-- redbot/core/data_manager.py | 5 +++-- redbot/core/rpc.py | 4 +++- redbot/launcher.py | 4 ++-- redbot/setup.py | 21 +++++++++++---------- 9 files changed, 60 insertions(+), 44 deletions(-) diff --git a/docs/autostart_systemd.rst b/docs/autostart_systemd.rst index 229eb95f1..45e2b018d 100644 --- a/docs/autostart_systemd.rst +++ b/docs/autostart_systemd.rst @@ -52,9 +52,10 @@ Paste the following in the file, and replace all instances of :code:`username` w User=username Group=username Type=idle - Restart=always + Restart=on-abnormal RestartSec=15 - RestartPreventExitStatus=0 + RestartForceExitStatus=1 + RestartForceExitStatus=26 TimeoutStopSec=10 [Install] diff --git a/redbot/__init__.py b/redbot/__init__.py index 1a8690f04..04727a960 100644 --- a/redbot/__init__.py +++ b/redbot/__init__.py @@ -27,7 +27,7 @@ if _sys.version_info < MIN_PYTHON_VERSION: f"Python {'.'.join(map(str, MIN_PYTHON_VERSION))} is required to run Red, but you have " f"{_sys.version}! Please update Python." ) - _sys.exit(1) + _sys.exit(78) class VersionInfo: diff --git a/redbot/__main__.py b/redbot/__main__.py index f9de75e03..ddd6eef85 100644 --- a/redbot/__main__.py +++ b/redbot/__main__.py @@ -53,13 +53,13 @@ def list_instances(): "No instances have been configured! Configure one " "using `redbot-setup` before trying to run the bot!" ) - sys.exit(1) + sys.exit(ExitCodes.CONFIGURATION_ERROR) else: text = "Configured Instances:\n\n" for instance_name in _get_instance_names(): text += "{}\n".format(instance_name) print(text) - sys.exit(0) + sys.exit(ExitCodes.SHUTDOWN) async def debug_info(*args: Any) -> None: @@ -80,10 +80,10 @@ async def edit_instance(red, cli_flags): if data_path is None and copy_data: print("--copy-data can't be used without --edit-data-path argument") - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) if new_name is None and confirm_overwrite: print("--overwrite-existing-instance can't be used without --edit-instance-name argument") - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) if ( no_prompt and all(to_change is None for to_change in (token, owner, new_name, data_path)) @@ -94,7 +94,7 @@ async def edit_instance(red, cli_flags): " Available arguments (check help for more information):" " --edit-instance-name, --edit-data-path, --copy-data, --owner, --token, --prefix" ) - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) await _edit_token(red, token, no_prompt) await _edit_prefix(red, prefix, no_prompt) @@ -357,10 +357,10 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None: token = new_token else: log.critical("Token and prefix must be set in order to login.") - sys.exit(1) + sys.exit(ExitCodes.CONFIGURATION_ERROR) if cli_flags.dry_run: - sys.exit(0) + sys.exit(ExitCodes.SHUTDOWN) try: # `async with red:` is unnecessary here because we call red.close() in shutdown handler await red.start(token) @@ -371,8 +371,8 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None: if confirm("\nDo you want to reset the token?"): await red._config.token.set("") print("Token has been reset.") - sys.exit(0) - sys.exit(1) + sys.exit(ExitCodes.SHUTDOWN) + sys.exit(ExitCodes.CONFIGURATION_ERROR) except discord.PrivilegedIntentsRequired: console = rich.get_console() console.print( @@ -381,7 +381,7 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None: "https://docs.discord.red/en/stable/bot_application_guide.html#enabling-privileged-intents", style="red", ) - sys.exit(1) + sys.exit(ExitCodes.CONFIGURATION_ERROR) except _NoOwnerSet: print( "Bot doesn't have any owner set!\n" @@ -399,7 +399,7 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None: "c) pass owner ID(s) when launching Red with --owner" " (and --co-owner if you need more than one) flag\n" ) - sys.exit(1) + sys.exit(ExitCodes.CONFIGURATION_ERROR) return None @@ -410,12 +410,12 @@ def handle_early_exit_flags(cli_flags: Namespace): elif cli_flags.version: print("Red V3") print("Current Version: {}".format(__version__)) - sys.exit(0) + sys.exit(ExitCodes.SHUTDOWN) elif cli_flags.debuginfo: early_exit_runner(cli_flags, debug_info) elif not cli_flags.instance_name and (not cli_flags.no_instance or cli_flags.edit): print("Error: No instance name was provided!") - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) async def shutdown_handler(red, signal_type=None, exit_code=None): @@ -553,7 +553,7 @@ def main(): asyncio.set_event_loop(None) loop.stop() loop.close() - exit_code = red._shutdown_mode if red is not None else 1 + exit_code = red._shutdown_mode if red is not None else ExitCodes.CRITICAL sys.exit(exit_code) diff --git a/redbot/core/bot.py b/redbot/core/bot.py index 03e336693..a055d9617 100644 --- a/redbot/core/bot.py +++ b/redbot/core/bot.py @@ -11,7 +11,6 @@ import weakref import functools from collections import namedtuple, OrderedDict from datetime import datetime -from enum import IntEnum from importlib.machinery import ModuleSpec from pathlib import Path from typing import ( @@ -39,6 +38,7 @@ from discord.ext import commands as dpy_commands from discord.ext.commands import when_mentioned_or from . import Config, i18n, commands, errors, drivers, modlog, bank +from .cli import ExitCodes from .cog_manager import CogManager, CogManagerUI from .core_commands import Core from .data_manager import cog_data_path @@ -69,7 +69,7 @@ SHARED_API_TOKENS = "SHARED_API_TOKENS" log = logging.getLogger("red") -__all__ = ("Red", "ExitCodes") +__all__ = ("Red",) NotMessage = namedtuple("NotMessage", "guild") @@ -2190,11 +2190,3 @@ class Red( failed_cogs=failures["cog"], unhandled=failures["unhandled"], ) - - -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/core/cli.py b/redbot/core/cli.py index bf37f3a8a..169f2eb79 100644 --- a/redbot/core/cli.py +++ b/redbot/core/cli.py @@ -2,6 +2,7 @@ import argparse import asyncio import logging import sys +from enum import IntEnum from typing import Optional import discord @@ -10,6 +11,21 @@ from discord import __version__ as discord_version from redbot.core.utils._internal_utils import cli_level_to_log_level +# This needs to be an int enum to be used +# with sys.exit +class ExitCodes(IntEnum): + #: Clean shutdown (through signals, keyboard interrupt, [p]shutdown, etc.). + SHUTDOWN = 0 + #: An unrecoverable error occurred during application's runtime. + CRITICAL = 1 + #: The CLI command was used incorrectly, such as when the wrong number of arguments are given. + INVALID_CLI_USAGE = 2 + #: Restart was requested by the bot owner (probably through [p]restart command). + RESTART = 26 + #: Some kind of configuration error occurred. + CONFIGURATION_ERROR = 78 # Exit code borrowed from os.EX_CONFIG. + + def confirm(text: str, default: Optional[bool] = None) -> bool: if default is None: options = "y/n" @@ -23,9 +39,12 @@ def confirm(text: str, default: Optional[bool] = None) -> bool: while True: try: value = input(f"{text}: [{options}] ").lower().strip() - except (KeyboardInterrupt, EOFError): + except KeyboardInterrupt: print("\nAborted!") - sys.exit(1) + sys.exit(ExitCodes.SHUTDOWN) + except EOFError: + print("\nAborted!") + sys.exit(ExitCodes.INVALID_CLI_USAGE) if value in ("y", "yes"): return True if value in ("n", "no"): diff --git a/redbot/core/data_manager.py b/redbot/core/data_manager.py index 655dd138f..762a99a71 100644 --- a/redbot/core/data_manager.py +++ b/redbot/core/data_manager.py @@ -12,6 +12,7 @@ import appdirs from discord.utils import deprecated from . import commands +from .cli import ExitCodes __all__ = [ "create_temp_config", @@ -118,7 +119,7 @@ def load_basic_configuration(instance_name_: str): "You need to configure the bot instance using `redbot-setup`" " prior to running the bot." ) - sys.exit(1) + sys.exit(ExitCodes.CONFIGURATION_ERROR) try: basic_config = config[instance_name] except KeyError: @@ -126,7 +127,7 @@ def load_basic_configuration(instance_name_: str): "Instance with this name doesn't exist." " You can create new instance using `redbot-setup` prior to running the bot." ) - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) def _base_data_path() -> Path: diff --git a/redbot/core/rpc.py b/redbot/core/rpc.py index 6308f9cb0..b2ee0eb4a 100644 --- a/redbot/core/rpc.py +++ b/redbot/core/rpc.py @@ -8,6 +8,8 @@ from aiohttp_json_rpc.rpc import JsonRpcMethod import logging +from redbot.core.cli import ExitCodes + log = logging.getLogger("red.rpc") __all__ = ["RPC", "RPCMixin", "get_name"] @@ -89,7 +91,7 @@ class RPC: ) except Exception as exc: log.exception("RPC setup failure", exc_info=exc) - sys.exit(1) + sys.exit(ExitCodes.CRITICAL) else: await self._site.start() log.debug("Created RPC server listener on port %s", port) diff --git a/redbot/launcher.py b/redbot/launcher.py index f726590a5..31b753c9e 100644 --- a/redbot/launcher.py +++ b/redbot/launcher.py @@ -20,7 +20,7 @@ from redbot.setup import ( create_backup, ) from redbot.core import __version__, version_info as red_version_info, VersionInfo -from redbot.core.cli import confirm +from redbot.core.cli import ExitCodes, confirm from redbot.core.data_manager import load_existing_config if sys.platform == "linux": @@ -155,7 +155,7 @@ def main(): req_ver=".".join(map(str, MIN_PYTHON_VERSION)), sys_ver=sys.version ) ) # Don't make an f-string, these may not exist on the python version being rejected! - sys.exit(1) + sys.exit(ExitCodes.CONFIGURATION_ERROR) if INTERACTIVE_MODE: main_menu(flags_to_pass) diff --git a/redbot/setup.py b/redbot/setup.py index b4af36a80..0c2618a40 100644 --- a/redbot/setup.py +++ b/redbot/setup.py @@ -21,6 +21,7 @@ from redbot.core.utils._internal_utils import ( cli_level_to_log_level, ) from redbot.core import config, data_manager, drivers +from redbot.core.cli import ExitCodes from redbot.core.data_manager import appdir, config_dir, config_file from redbot.core.drivers import BackendType, IdentifierData @@ -30,7 +31,7 @@ try: config_dir.mkdir(parents=True, exist_ok=True) except PermissionError: print("You don't have permission to write to '{}'\nExiting...".format(config_dir)) - sys.exit(1) + sys.exit(ExitCodes.CONFIGURATION_ERROR) instance_data = data_manager.load_existing_config() if instance_data is None: @@ -77,7 +78,7 @@ def get_data_dir(*, instance_name: str, data_path: Optional[Path], interactive: "We were unable to check your chosen directory." " Provided path may contain an invalid character." ) - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) if not exists: try: @@ -85,15 +86,15 @@ def get_data_dir(*, instance_name: str, data_path: Optional[Path], interactive: except OSError: print( "We were unable to create your chosen directory." - " You may need to restart this process with admin" - " privileges." + " You may need to create the directory and set proper permissions" + " for it manually before it can be used as the data directory." ) - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) print("You have chosen {} to be your data directory.".format(data_path)) if not click.confirm("Please confirm", default=True): print("Please start the process over.") - sys.exit(0) + sys.exit(ExitCodes.CRITICAL) return str(data_path.resolve()) @@ -143,7 +144,7 @@ def get_name(name: str) -> str: " and can only include characters A-z, numbers," " and non-consecutive underscores (_) and periods (.)." ) - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) return name while len(name) == 0: @@ -191,7 +192,7 @@ def basic_setup( "Providing instance name through --instance-name is required" " when using non-interactive mode." ) - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) if interactive: print( @@ -225,14 +226,14 @@ def basic_setup( "Are you absolutely certain you want to continue?", default=False ): print("Not continuing") - sys.exit(0) + sys.exit(ExitCodes.SHUTDOWN) else: print( "An instance with this name already exists.\n" "If you want to remove the existing instance and replace it with this one," " run this command with --overwrite-existing-instance flag." ) - sys.exit(1) + sys.exit(ExitCodes.INVALID_CLI_USAGE) save_config(name, default_dirs) if interactive: