From fe7475d6c5b8ede7fb4e41ffc320f4cbbf5b1624 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 25 Aug 2020 16:31:05 -0400 Subject: [PATCH] Exit gracefully on Ctrl-C If we receive SIGINT, exit gracefully: run the clean_up method; don't print a stacktrace; exit with error code 130 (128 + SIGINT). Change-Id: I77687133d5482912523814a28e42f4f3a1a146d5 Story: 2008124 Task: 40846 --- cliff/app.py | 35 +++++++++++++++++++---------------- cliff/tests/test_app.py | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/cliff/app.py b/cliff/app.py index 8b8217ce..5fae2252 100644 --- a/cliff/app.py +++ b/cliff/app.py @@ -276,11 +276,16 @@ class App(object): else: self.LOG.error(err) return 1 + except KeyboardInterrupt: + return 130 result = 1 if self.interactive_mode: result = self.interact() else: - result = self.run_subcommand(remainder) + try: + result = self.run_subcommand(remainder) + except KeyboardInterrupt: + return 130 return result # FIXME(dhellmann): Consider moving these command handling methods @@ -391,6 +396,7 @@ class App(object): kwargs['cmd_name'] = cmd_name cmd = cmd_factory(self, self.options, **kwargs) result = 1 + err = None try: self.prepare_to_run_command(cmd) full_name = (cmd_name @@ -398,17 +404,24 @@ class App(object): else ' '.join([self.NAME, cmd_name]) ) cmd_parser = cmd.get_parser(full_name) - parsed_args = cmd_parser.parse_args(sub_argv) + try: + parsed_args = cmd_parser.parse_args(sub_argv) + except SystemExit as ex: + raise cmd2.exceptions.Cmd2ArgparseError from ex result = cmd.run(parsed_args) except help.HelpExit: result = 0 - except SystemExit as ex: - raise cmd2.exceptions.Cmd2ArgparseError from ex - except Exception as err: + except Exception as err1: + err = err1 if self.options.debug: self.LOG.exception(err) else: self.LOG.error(err) + except KeyboardInterrupt as err1: + result = 130 + err = err1 + raise + finally: try: self.clean_up(cmd, result, err) except Exception as err2: @@ -416,15 +429,5 @@ class App(object): self.LOG.exception(err2) else: self.LOG.error('Could not clean up: %s', err2) - if self.options.debug: - # 'raise' here gets caught and does not exit like we want - return result - else: - try: - self.clean_up(cmd, result, None) - except Exception as err3: - if self.options.debug: - self.LOG.exception(err3) - else: - self.LOG.error('Could not clean up: %s', err3) + del err return result diff --git a/cliff/tests/test_app.py b/cliff/tests/test_app.py index d24c62b5..d31cd3da 100644 --- a/cliff/tests/test_app.py +++ b/cliff/tests/test_app.py @@ -51,6 +51,15 @@ def make_app(**kwargs): err_command.return_value = err_command_inst cmd_mgr.add_command('error', err_command) + # Register a command that is interrrupted + interrupt_command = mock.Mock(name='interrupt_command', spec=c_cmd.Command) + interrupt_command_inst = mock.Mock(spec=c_cmd.Command) + interrupt_command_inst.run = mock.Mock( + side_effect=KeyboardInterrupt + ) + interrupt_command.return_value = interrupt_command_inst + cmd_mgr.add_command('interrupt', interrupt_command) + app = application.App('testing interactive mode', '1', cmd_mgr, @@ -113,6 +122,11 @@ class TestInitAndCleanup(base.TestBase): app.run(['mock']) app.prepare_to_run_command.assert_called_once_with(command()) + def test_interrupt_command(self): + app, command = make_app() + result = app.run(['interrupt']) + self.assertEqual(result, 130) + def test_clean_up_success(self): app, command = make_app() app.clean_up = mock.MagicMock(name='clean_up') @@ -148,6 +162,19 @@ class TestInitAndCleanup(base.TestBase): self.assertIsInstance(args[2], RuntimeError) self.assertEqual(('test exception',), args[2].args) + def test_clean_up_interrupt(self): + app, command = make_app() + + app.clean_up = mock.MagicMock(name='clean_up') + ret = app.run(['interrupt']) + self.assertNotEqual(ret, 0) + + app.clean_up.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY) + call_args = app.clean_up.call_args_list[0] + self.assertEqual(mock.call(mock.ANY, 130, mock.ANY), call_args) + args, kwargs = call_args + self.assertIsInstance(args[2], KeyboardInterrupt) + def test_error_handling_clean_up_raises_exception(self): app, command = make_app() @@ -322,6 +349,19 @@ class TestHelpHandling(base.TestBase): def test_deferred_help(self): self._test_help(True) + def _test_interrupted_help(self, deferred_help): + app, _ = make_app(deferred_help=deferred_help) + with mock.patch('cliff.help.HelpAction.__call__', + side_effect=KeyboardInterrupt): + result = app.run(['--help']) + self.assertEqual(result, 130) + + def test_interrupted_help(self): + self._test_interrupted_help(False) + + def test_interrupted_deferred_help(self): + self._test_interrupted_help(True) + def test_subcommand_help(self): app, _ = make_app(deferred_help=False)