Merge pull request #3711 from pallets/cli-loading-error

cleaner message when CLI can't load app

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

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

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)

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: