diff --git a/CHANGES.rst b/CHANGES.rst index 73ff5f2e..43a6b5e7 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,6 +12,8 @@ Unreleased - Passing ``script_info`` to app factory functions is deprecated. This was not portable outside the ``flask`` command. Use ``click.get_current_context().obj`` if it's needed. :issue:`3552` +- The CLI shows better error messages when the app failed to load + when looking up commands. :issue:`2741` - Add :meth:`sessions.SessionInterface.get_cookie_name` to allow setting the session cookie name dynamically. :pr:`3369` - Add :meth:`Config.from_file` to load config using arbitrary file diff --git a/src/flask/cli.py b/src/flask/cli.py index caf0dfea..e73f7b78 100644 --- a/src/flask/cli.py +++ b/src/flask/cli.py @@ -536,43 +536,41 @@ class FlaskGroup(AppGroup): def get_command(self, ctx, name): self._load_plugin_commands() + # Look up built-in and plugin commands, which should be + # available even if the app fails to load. + rv = super().get_command(ctx, name) - # We load built-in commands first as these should always be the - # same no matter what the app does. If the app does want to - # override this it needs to make a custom instance of this group - # and not attach the default commands. - # - # This also means that the script stays functional in case the - # application completely fails. - rv = AppGroup.get_command(self, ctx, name) if rv is not None: return rv info = ctx.ensure_object(ScriptInfo) + + # Look up commands provided by the app, showing an error and + # continuing if the app couldn't be loaded. try: - rv = info.load_app().cli.get_command(ctx, name) - if rv is not None: - return rv - except NoAppException: - pass + return info.load_app().cli.get_command(ctx, name) + except NoAppException as e: + click.secho(f"Error: {e.format_message()}\n", err=True, fg="red") def list_commands(self, ctx): self._load_plugin_commands() - - # The commands available is the list of both the application (if - # available) plus the builtin commands. - rv = set(click.Group.list_commands(self, ctx)) + # Start with the built-in and plugin commands. + rv = set(super().list_commands(ctx)) info = ctx.ensure_object(ScriptInfo) + + # Add commands provided by the app, showing an error and + # continuing if the app couldn't be loaded. try: rv.update(info.load_app().cli.list_commands(ctx)) + except NoAppException as e: + # When an app couldn't be loaded, show the error message + # without the traceback. + click.secho(f"Error: {e.format_message()}\n", err=True, fg="red") except Exception: - # Here we intentionally swallow all exceptions as we don't - # want the help page to break if the app does not exist. - # If someone attempts to use the command we try to create - # the app again and this will give us the error. - # However, we will not do so silently because that would confuse - # users. - traceback.print_exc() + # When any other errors occurred during loading, show the + # full traceback. + click.secho(f"{traceback.format_exc()}\n", err=True, fg="red") + return sorted(rv) def main(self, *args, **kwargs): diff --git a/tests/conftest.py b/tests/conftest.py index d7a54a66..17ff2f3d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -73,9 +73,15 @@ def client(app): @pytest.fixture def test_apps(monkeypatch): - monkeypatch.syspath_prepend( - os.path.abspath(os.path.join(os.path.dirname(__file__), "test_apps")) - ) + monkeypatch.syspath_prepend(os.path.join(os.path.dirname(__file__), "test_apps")) + original_modules = set(sys.modules.keys()) + + yield + + # Remove any imports cached during the test. Otherwise "import app" + # will work in the next test even though it's no longer on the path. + for key in sys.modules.keys() - original_modules: + sys.modules.pop(key) @pytest.fixture(autouse=True) diff --git a/tests/test_cli.py b/tests/test_cli.py index f3f8aca1..5fb114a4 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -239,7 +239,7 @@ def test_locate_app_raises(test_apps, iname, aname): locate_app(info, iname, aname) -def test_locate_app_suppress_raise(): +def test_locate_app_suppress_raise(test_apps): info = ScriptInfo() app = locate_app(info, "notanapp.py", None, raise_if_not_found=False) assert app is None @@ -396,21 +396,36 @@ def test_flaskgroup_debug(runner, set_debug_flag): assert result.output == f"{not set_debug_flag}\n" -def test_print_exceptions(runner): - """Print the stacktrace if the CLI.""" +def test_no_command_echo_loading_error(): + from flask.cli import cli - def create_app(): - raise Exception("oh no") - return Flask("flaskgroup") + runner = CliRunner(mix_stderr=False) + result = runner.invoke(cli, ["missing"]) + assert result.exit_code == 2 + assert "FLASK_APP" in result.stderr + assert "Usage:" in result.stderr - @click.group(cls=FlaskGroup, create_app=create_app) - def cli(**params): - pass +def test_help_echo_loading_error(): + from flask.cli import cli + + runner = CliRunner(mix_stderr=False) result = runner.invoke(cli, ["--help"]) assert result.exit_code == 0 - assert "Exception: oh no" in result.output - assert "Traceback" in result.output + assert "FLASK_APP" in result.stderr + assert "Usage:" in result.stdout + + +def test_help_echo_exception(): + def create_app(): + raise Exception("oh no") + + cli = FlaskGroup(create_app=create_app) + runner = CliRunner(mix_stderr=False) + result = runner.invoke(cli, ["--help"]) + assert result.exit_code == 0 + assert "Exception: oh no" in result.stderr + assert "Usage:" in result.stdout class TestRoutes: