Repository: python-cmd2/cmd2

Issue:
Version 1.0.2 changed the way SystemExit exceptions raised from do_command methods are handled
I have a cmd2 application which does it's own argparsing inside of the `do_command()` methods, i.e. it does not use any of the cmd2 built-in arg parsing functionality. This is done by calling `argparse.parse_args()` as one of the first lines of code inside the `do_command()` method.

In 1.0.1 if a `do_command()` method raises a `SystemExit` exception, as `parse_args()` does if given the '-h' option or if a syntax error occurs, that exception would be swallowed by `cmd2` and the command prompt would be printed again.

In 1.0.2, that exception is not swallowed and the `cmd2` app terminates.

You can replicate this issue by adding a `raise SystemExit` to the `do_speak()` method in `examples/first_app.py`, and running it with the two versions of `cmd2`.

The behavior of other raised exceptions does not appear to have changed between 1.0.1 and 1.0.2. For example if you raise a ValueError, you get:
```
EXCEPTION of type 'ValueError' occurred with message: ''
To enable full traceback, run the following command: 'set debug true'
```
in both versions of `cmd2`.

This change in behavior is caused by the change in the logic on the last two lines of the `onecmd_plus_hooks()` method made between 1.0.1 and 1.0.2.

In the short term, I request that we quickly revert this change in behavior. After that, I suggest we discuss the desired behavior is for `cmd2` when a `SystemExit` exception is raised, and then intentionally implement and document that behavior.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6fbd466..31daa079 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,4 @@
-## 1.0.3 (TBD, 2020)
+## 1.1.0 (TBD, 2020)
 * Bug Fixes
     * Fixed issue where subcommand usage text could contain a subcommand alias instead of the actual name
 * Enhancements
@@ -14,6 +14,15 @@
           documentation for an overview.
         * See [table_creation.py](https://github.com/python-cmd2/cmd2/blob/master/examples/table_creation.py)
           for an example.
+    * Added the following exceptions to the public API
+        * `SkipPostcommandHooks` - Custom exception class for when a command has a failure bad enough to skip
+          post command hooks, but not bad enough to print the exception to the user.
+        * `Cmd2ArgparseError` - A `SkipPostcommandHooks` exception for when a command fails to parse its arguments.
+          Normally argparse raises a `SystemExit` exception in these cases. To avoid stopping the command
+          loop, catch the `SystemExit` and raise this instead. If you still need to run post command hooks
+          after parsing fails, just return instead of raising an exception.
+    * Added explicit handling of `SystemExit`. If a command raises this exception, the command loop will be
+      gracefully stopped.
     
 ## 1.0.2 (April 06, 2020)
 * Bug Fixes
diff --git a/cmd2/__init__.py b/cmd2/__init__.py
index eb5c275d..d49427f2 100644
--- a/cmd2/__init__.py
+++ b/cmd2/__init__.py
@@ -25,6 +25,7 @@ from .argparse_custom import DEFAULT_ARGUMENT_PARSER
 from .cmd2 import Cmd
 from .constants import COMMAND_NAME, DEFAULT_SHORTCUTS
 from .decorators import with_argument_list, with_argparser, with_argparser_and_unknown_args, with_category
+from .exceptions import Cmd2ArgparseError, SkipPostcommandHooks
 from .parsing import Statement
 from .py_bridge import CommandResult
 from .utils import categorize, CompletionError, Settable
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 49c181f1..73f7aa79 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -46,7 +46,7 @@ from . import ansi, constants, plugin, utils
 from .argparse_custom import DEFAULT_ARGUMENT_PARSER, CompletionItem
 from .clipboard import can_clip, get_paste_buffer, write_to_paste_buffer
 from .decorators import with_argparser
-from .exceptions import Cmd2ArgparseError, Cmd2ShlexError, EmbeddedConsoleExit, EmptyStatement, RedirectionError
+from .exceptions import Cmd2ShlexError, EmbeddedConsoleExit, EmptyStatement, RedirectionError, SkipPostcommandHooks
 from .history import History, HistoryItem
 from .parsing import Macro, MacroArg, Statement, StatementParser, shlex_split
 from .rl_utils import RlType, rl_get_point, rl_make_safe_prompt, rl_set_prompt, rl_type, rl_warning, vt100_support
@@ -1587,9 +1587,9 @@ class Cmd(cmd.Cmd):
 
         :param line: command line to run
         :param add_to_history: If True, then add this command to history. Defaults to True.
-        :param raise_keyboard_interrupt: if True, then KeyboardInterrupt exceptions will be raised. This is used when
-                                         running commands in a loop to be able to stop the whole loop and not just
-                                         the current command. Defaults to False.
+        :param raise_keyboard_interrupt: if True, then KeyboardInterrupt exceptions will be raised if stop isn't already
+                                         True. This is used when running commands in a loop to be able to stop the whole
+                                         loop and not just the current command. Defaults to False.
         :param py_bridge_call: This should only ever be set to True by PyBridge to signify the beginning
                                of an app() call from Python. It is used to enable/disable the storage of the
                                command's stdout.
@@ -1667,26 +1667,35 @@ class Cmd(cmd.Cmd):
                     if py_bridge_call:
                         # Stop saving command's stdout before command finalization hooks run
                         self.stdout.pause_storage = True
-        except KeyboardInterrupt as ex:
-            if raise_keyboard_interrupt:
-                raise ex
-        except (Cmd2ArgparseError, EmptyStatement):
+        except (SkipPostcommandHooks, EmptyStatement):
             # Don't do anything, but do allow command finalization hooks to run
             pass
         except Cmd2ShlexError as ex:
             self.perror("Invalid syntax: {}".format(ex))
         except RedirectionError as ex:
             self.perror(ex)
+        except KeyboardInterrupt as ex:
+            if raise_keyboard_interrupt and not stop:
+                raise ex
+        except SystemExit:
+            stop = True
         except Exception as ex:
             self.pexcept(ex)
         finally:
-            stop = self._run_cmdfinalization_hooks(stop, statement)
+            try:
+                stop = self._run_cmdfinalization_hooks(stop, statement)
+            except KeyboardInterrupt as ex:
+                if raise_keyboard_interrupt and not stop:
+                    raise ex
+            except SystemExit:
+                stop = True
+            except Exception as ex:
+                self.pexcept(ex)
 
         return stop
 
     def _run_cmdfinalization_hooks(self, stop: bool, statement: Optional[Statement]) -> bool:
         """Run the command finalization hooks"""
-
         with self.sigint_protection:
             if not sys.platform.startswith('win') and self.stdin.isatty():
                 # Before the next command runs, fix any terminal problems like those
@@ -1695,15 +1704,12 @@ class Cmd(cmd.Cmd):
                 proc = subprocess.Popen(['stty', 'sane'])
                 proc.communicate()
 
-        try:
-            data = plugin.CommandFinalizationData(stop, statement)
-            for func in self._cmdfinalization_hooks:
-                data = func(data)
-            # retrieve the final value of stop, ignoring any
-            # modifications to the statement
-            return data.stop
-        except Exception as ex:
-            self.pexcept(ex)
+        data = plugin.CommandFinalizationData(stop, statement)
+        for func in self._cmdfinalization_hooks:
+            data = func(data)
+        # retrieve the final value of stop, ignoring any
+        # modifications to the statement
+        return data.stop
 
     def runcmds_plus_hooks(self, cmds: List[Union[HistoryItem, str]], *, add_to_history: bool = True,
                            stop_on_keyboard_interrupt: bool = True) -> bool:
@@ -3894,7 +3900,7 @@ class Cmd(cmd.Cmd):
 
         IMPORTANT: This function will not print an alert unless it can acquire self.terminal_lock to ensure
                    a prompt is onscreen.  Therefore it is best to acquire the lock before calling this function
-                   to guarantee the alert prints.
+                   to guarantee the alert prints and to avoid raising a RuntimeError.
 
         :param alert_msg: the message to display to the user
         :param new_prompt: if you also want to change the prompt that is displayed, then include it here
@@ -3956,7 +3962,7 @@ class Cmd(cmd.Cmd):
 
         IMPORTANT: This function will not update the prompt unless it can acquire self.terminal_lock to ensure
                    a prompt is onscreen.  Therefore it is best to acquire the lock before calling this function
-                   to guarantee the prompt changes.
+                   to guarantee the prompt changes and to avoid raising a RuntimeError.
 
                    If user is at a continuation prompt while entering a multiline command, the onscreen prompt will
                    not change. However self.prompt will still be updated and display immediately after the multiline
@@ -3971,9 +3977,9 @@ class Cmd(cmd.Cmd):
 
         Raises a `RuntimeError` if called while another thread holds `terminal_lock`.
 
-        IMPORTANT: This function will not set the title unless it can acquire self.terminal_lock to avoid
-                   writing to stderr while a command is running. Therefore it is best to acquire the lock
-                   before calling this function to guarantee the title changes.
+        IMPORTANT: This function will not set the title unless it can acquire self.terminal_lock to avoid writing
+                   to stderr while a command is running. Therefore it is best to acquire the lock before calling
+                   this function to guarantee the title changes and to avoid raising a RuntimeError.
 
         :param title: the new window title
         """
diff --git a/cmd2/exceptions.py b/cmd2/exceptions.py
index 635192e1..8a7fd81f 100644
--- a/cmd2/exceptions.py
+++ b/cmd2/exceptions.py
@@ -1,16 +1,33 @@
 # coding=utf-8
-"""Custom exceptions for cmd2.  These are NOT part of the public API and are intended for internal use only."""
+"""Custom exceptions for cmd2"""
 
 
-class Cmd2ArgparseError(Exception):
+############################################################################################################
+# The following exceptions are part of the public API
+############################################################################################################
+
+class SkipPostcommandHooks(Exception):
     """
-    Custom exception class for when a command has an error parsing its arguments.
-    This can be raised by argparse decorators or the command functions themselves.
-    The main use of this exception is to tell cmd2 not to run Postcommand hooks.
+    Custom exception class for when a command has a failure bad enough to skip post command
+    hooks, but not bad enough to print the exception to the user.
     """
     pass
 
 
+class Cmd2ArgparseError(SkipPostcommandHooks):
+    """
+    A ``SkipPostcommandHooks`` exception for when a command fails to parse its arguments.
+    Normally argparse raises a SystemExit exception in these cases. To avoid stopping the command
+    loop, catch the SystemExit and raise this instead. If you still need to run post command hooks
+    after parsing fails, just return instead of raising an exception.
+    """
+    pass
+
+
+############################################################################################################
+# The following exceptions are NOT part of the public API and are intended for internal use only.
+############################################################################################################
+
 class Cmd2ShlexError(Exception):
     """Raised when shlex fails to parse a command line string in StatementParser"""
     pass
diff --git a/docs/api/exceptions.rst b/docs/api/exceptions.rst
new file mode 100644
index 00000000..8ef0a61f
--- /dev/null
+++ b/docs/api/exceptions.rst
@@ -0,0 +1,11 @@
+cmd2.exceptions
+===============
+
+Custom cmd2 exceptions
+
+
+.. autoclass:: cmd2.exceptions.SkipPostcommandHooks
+    :members:
+
+.. autoclass:: cmd2.exceptions.Cmd2ArgparseError
+    :members:
diff --git a/docs/api/index.rst b/docs/api/index.rst
index 7b66a684..cc899ba1 100644
--- a/docs/api/index.rst
+++ b/docs/api/index.rst
@@ -19,37 +19,39 @@ This documentation is for ``cmd2`` version |version|.
    :hidden:
 
    cmd
-   decorators
-   parsing
+   ansi
    argparse_completer
    argparse_custom
-   ansi
-   utils
+   constants
+   decorators
+   exceptions
    history
+   parsing
    plugin
    py_bridge
    table_creator
-   constants
+   utils
 
 **Modules**
 
 - :ref:`api/cmd:cmd2.Cmd` - functions and attributes of the main
   class in this library
-- :ref:`api/decorators:cmd2.decorators` - decorators for ``cmd2``
-  commands
-- :ref:`api/parsing:cmd2.parsing` - classes for parsing and storing
-  user input
+- :ref:`api/ansi:cmd2.ansi` - convenience classes and functions for generating
+  ANSI escape sequences to style text in the terminal
 - :ref:`api/argparse_completer:cmd2.argparse_completer` - classes for
   ``argparse``-based tab completion
 - :ref:`api/argparse_custom:cmd2.argparse_custom` - classes and functions
   for extending ``argparse``
-- :ref:`api/ansi:cmd2.ansi` - convenience classes and functions for generating
-  ANSI escape sequences to style text in the terminal
-- :ref:`api/utils:cmd2.utils` - various utility classes and functions
+- :ref:`api/constants:cmd2.constants` - just like it says on the tin
+- :ref:`api/decorators:cmd2.decorators` - decorators for ``cmd2``
+  commands
+- :ref:`api/exceptions:cmd2.exceptions` - custom ``cmd2`` exceptions
 - :ref:`api/history:cmd2.history` - classes for storing the history
   of previously entered commands
+- :ref:`api/parsing:cmd2.parsing` - classes for parsing and storing
+  user input
 - :ref:`api/plugin:cmd2.plugin` - data classes for hook methods
 - :ref:`api/py_bridge:cmd2.py_bridge` - classes for bridging calls from the
   embedded python environment to the host app
 - :ref:`api/table_creator:cmd2.table_creator` - table creation module
-- :ref:`api/constants:cmd2.constants` - just like it says on the tin
+- :ref:`api/utils:cmd2.utils` - various utility classes and functions
diff --git a/docs/features/commands.rst b/docs/features/commands.rst
index 823c3ca6..13a4ac1f 100644
--- a/docs/features/commands.rst
+++ b/docs/features/commands.rst
@@ -129,7 +129,7 @@ it should stop prompting for user input and cleanly exit. ``cmd2`` already
 includes a ``quit`` command, but if you wanted to make another one called
 ``finis`` you could::
 
-    def do_finis(self, line):
+    def do_finish(self, line):
         """Exit the application"""
         return True
 
@@ -186,6 +186,19 @@ catch it and display it for you. The `debug` :ref:`setting
 name and message. If `debug` is `true`, ``cmd2`` will display a traceback, and
 then display the exception name and message.
 
+There are a few exceptions which commands can raise that do not print as
+described above:
+
+- :attr:`cmd2.exceptions.SkipPostcommandHooks` - all postcommand hooks are
+  skipped and no exception prints
+- :attr:`cmd2.exceptions.Cmd2ArgparseError` - behaves like
+  ``SkipPostcommandHooks``
+- ``SystemExit`` - ``stop`` will be set to ``True`` in an attempt to stop the
+  command loop
+- ``KeyboardInterrupt`` - raised if running in a text script and ``stop`` isn't
+  already True to stop the script
+
+All other ``BaseExceptions`` are not caught by ``cmd2`` and will be raised
 
 Disabling or Hiding Commands
 ----------------------------
diff --git a/docs/features/hooks.rst b/docs/features/hooks.rst
index fec8e258..4c615586 100644
--- a/docs/features/hooks.rst
+++ b/docs/features/hooks.rst
@@ -291,6 +291,12 @@ blindly returns ``False``, a prior hook's requst to exit the application will
 not be honored. It's best to return the value you were passed unless you have a
 compelling reason to do otherwise.
 
+To purposefully and silently skip postcommand hooks, commands can raise any of
+of the following exceptions.
+
+- :attr:`cmd2.exceptions.SkipPostcommandHooks`
+- :attr:`cmd2.exceptions.Cmd2ArgparseError`
+
 
 Command Finalization Hooks
 --------------------------


Test patch:
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 33f75c9e..bc0e0a94 100755
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -466,6 +466,17 @@ def test_in_script(request):
 
     assert "WE ARE IN SCRIPT" in out[-1]
 
+def test_system_exit_in_command(base_app, capsys):
+    """Test raising SystemExit from a command"""
+    import types
+
+    def do_system_exit(self, _):
+        raise SystemExit
+    setattr(base_app, 'do_system_exit', types.MethodType(do_system_exit, base_app))
+
+    stop = base_app.onecmd_plus_hooks('system_exit')
+    assert stop
+
 def test_output_redirection(base_app):
     fd, filename = tempfile.mkstemp(prefix='cmd2_test', suffix='.txt')
     os.close(fd)
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index 132361a6..e49cbbfc 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -222,11 +222,24 @@ class Plugin:
         self.called_cmdfinalization += 1
         raise ValueError
 
+    def cmdfinalization_hook_system_exit(self, data: cmd2.plugin.CommandFinalizationData) -> \
+            cmd2.plugin.CommandFinalizationData:
+        """A command finalization hook which raises a SystemExit"""
+        self.called_cmdfinalization += 1
+        raise SystemExit
+
+    def cmdfinalization_hook_keyboard_interrupt(self, data: cmd2.plugin.CommandFinalizationData) -> \
+            cmd2.plugin.CommandFinalizationData:
+        """A command finalization hook which raises a KeyboardInterrupt"""
+        self.called_cmdfinalization += 1
+        raise KeyboardInterrupt
+
     def cmdfinalization_hook_not_enough_parameters(self) -> plugin.CommandFinalizationData:
         """A command finalization hook with no parameters."""
         pass
 
-    def cmdfinalization_hook_too_many_parameters(self, one: plugin.CommandFinalizationData, two: str) -> plugin.CommandFinalizationData:
+    def cmdfinalization_hook_too_many_parameters(self, one: plugin.CommandFinalizationData, two: str) -> \
+            plugin.CommandFinalizationData:
         """A command finalization hook with too many parameters."""
         return one
 
@@ -256,6 +269,10 @@ class PluggedApp(Plugin, cmd2.Cmd):
         """Repeat back the arguments"""
         self.poutput(statement)
 
+    def do_skip_postcmd_hooks(self, _):
+        self.poutput("In do_skip_postcmd_hooks")
+        raise exceptions.SkipPostcommandHooks
+
     parser = Cmd2ArgumentParser(description="Test parser")
     parser.add_argument("my_arg", help="some help text")
 
@@ -847,6 +864,46 @@ def test_cmdfinalization_hook_exception(capsys):
     assert err
     assert app.called_cmdfinalization == 1
 
+def test_cmdfinalization_hook_system_exit(capsys):
+    app = PluggedApp()
+    app.register_cmdfinalization_hook(app.cmdfinalization_hook_system_exit)
+    stop = app.onecmd_plus_hooks('say hello')
+    assert stop
+    assert app.called_cmdfinalization == 1
+
+def test_cmdfinalization_hook_keyboard_interrupt(capsys):
+    app = PluggedApp()
+    app.register_cmdfinalization_hook(app.cmdfinalization_hook_keyboard_interrupt)
+
+    # First make sure KeyboardInterrupt isn't raised unless told to
+    stop = app.onecmd_plus_hooks('say hello', raise_keyboard_interrupt=False)
+    assert not stop
+    assert app.called_cmdfinalization == 1
+
+    # Now enable raising the KeyboardInterrupt
+    app.reset_counters()
+    with pytest.raises(KeyboardInterrupt):
+        stop = app.onecmd_plus_hooks('say hello', raise_keyboard_interrupt=True)
+    assert not stop
+    assert app.called_cmdfinalization == 1
+
+    # Now make sure KeyboardInterrupt isn't raised if stop is already True
+    app.reset_counters()
+    stop = app.onecmd_plus_hooks('quit', raise_keyboard_interrupt=True)
+    assert stop
+    assert app.called_cmdfinalization == 1
+
+def test_skip_postcmd_hooks(capsys):
+    app = PluggedApp()
+    app.register_postcmd_hook(app.postcmd_hook)
+    app.register_cmdfinalization_hook(app.cmdfinalization_hook)
+
+    # Cause a SkipPostcommandHooks exception and verify no postcmd stuff runs but cmdfinalization_hook still does
+    app.onecmd_plus_hooks('skip_postcmd_hooks')
+    out, err = capsys.readouterr()
+    assert "In do_skip_postcmd_hooks" in out
+    assert app.called_postcmd == 0
+    assert app.called_cmdfinalization == 1
 
 def test_cmd2_argparse_exception(capsys):
     """