From f6ee2a4dc5821fe2e9dcd522270253a4b5b2cff2 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Wed, 7 Apr 2021 14:31:01 +1000 Subject: [PATCH 1/5] [Resolve #988] Stop hiding debug info in helpers Before this, the catch_exceptions function in cli/helpers would catch a range of exceptions and then hide all but the error message from the caller. Over the years, this has caused myself and others quite a lot of lost time, as it is now often quite unclear what caused Sceptre to fail. Simply re-raising the original exception provides valuable information to allow users of Sceptre to debug their failing code. --- sceptre/cli/helpers.py | 5 +++-- tests/test_cli.py | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sceptre/cli/helpers.py b/sceptre/cli/helpers.py index b5749c529..b8f9cbbc0 100644 --- a/sceptre/cli/helpers.py +++ b/sceptre/cli/helpers.py @@ -1,5 +1,6 @@ import logging import sys + from itertools import cycle from functools import partial, wraps @@ -31,14 +32,14 @@ def catch_exceptions(func): def decorated(*args, **kwargs): """ Invokes ``func``, catches expected errors, prints the error message and - exits sceptre with a non-zero exit code. + re-raises. """ try: return func(*args, **kwargs) except (SceptreException, BotoCoreError, ClientError, Boto3Error, TemplateError) as error: write(error) - sys.exit(1) + raise return decorated diff --git a/tests/test_cli.py b/tests/test_cli.py index 1fe02ac21..66f5120da 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -55,14 +55,13 @@ def teardown_method(self, test_method): self.patcher_ConfigReader.stop() self.patcher_StackActions.stop() - @patch("sys.exit") - def test_catch_excecptions(self, mock_exit): + def test_catch_excecptions(self): @catch_exceptions def raises_exception(): raise SceptreException() - raises_exception() - mock_exit.assert_called_once_with(1) + with pytest.raises(SceptreException): + raises_exception() @pytest.mark.parametrize("command,files,output", [ # one --var option From eaf0a1ba4160ac4d59bff4fca8a7518cb9290baf Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Sun, 3 Oct 2021 17:38:01 +1100 Subject: [PATCH 2/5] Refactor according to Jon's suggestion --- sceptre/cli/helpers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sceptre/cli/helpers.py b/sceptre/cli/helpers.py index b8f9cbbc0..479804fc7 100644 --- a/sceptre/cli/helpers.py +++ b/sceptre/cli/helpers.py @@ -1,5 +1,6 @@ import logging import sys +import traceback from itertools import cycle from functools import partial, wraps @@ -38,8 +39,9 @@ def decorated(*args, **kwargs): return func(*args, **kwargs) except (SceptreException, BotoCoreError, ClientError, Boto3Error, TemplateError) as error: - write(error) - raise + if "DEBUG_ERRORS" in os.environ: + traceback.print_exc() + sys.exit(1) return decorated From d0b8e7d14c86b50b91d50e4111075f6f12ea693d Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Sat, 16 Oct 2021 03:16:55 +1100 Subject: [PATCH 3/5] Refactor to implement Craig Hurley's suggestion --- sceptre/cli/helpers.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/sceptre/cli/helpers.py b/sceptre/cli/helpers.py index 479804fc7..fd82a2c16 100644 --- a/sceptre/cli/helpers.py +++ b/sceptre/cli/helpers.py @@ -1,7 +1,5 @@ import logging import sys -import traceback - from itertools import cycle from functools import partial, wraps @@ -29,18 +27,24 @@ def catch_exceptions(func): simplified. :returns: The decorated function. """ + def logging_level(): + logger = logging.getLogger(__name__) + return logger.getEffectiveLevel() + @wraps(func) def decorated(*args, **kwargs): """ Invokes ``func``, catches expected errors, prints the error message and - re-raises. + exits sceptre with a non-zero exit code. In debug mode, the original + exception is re-raised to assist debugging. """ try: return func(*args, **kwargs) except (SceptreException, BotoCoreError, ClientError, Boto3Error, TemplateError) as error: - if "DEBUG_ERRORS" in os.environ: - traceback.print_exc() + if logging_level() == logging.DEBUG: + raise + write(error) sys.exit(1) return decorated From 1764fe371e17962742f64fcf3c22ec11836820ad Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Sat, 16 Oct 2021 03:18:52 +1100 Subject: [PATCH 4/5] Restore master version of tests --- tests/test_cli.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 66f5120da..1fe02ac21 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -55,13 +55,14 @@ def teardown_method(self, test_method): self.patcher_ConfigReader.stop() self.patcher_StackActions.stop() - def test_catch_excecptions(self): + @patch("sys.exit") + def test_catch_excecptions(self, mock_exit): @catch_exceptions def raises_exception(): raise SceptreException() - with pytest.raises(SceptreException): - raises_exception() + raises_exception() + mock_exit.assert_called_once_with(1) @pytest.mark.parametrize("command,files,output", [ # one --var option From f290f684038b329c12402bedb9cb13fbb6e892cc Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Sat, 16 Oct 2021 14:50:29 +1100 Subject: [PATCH 5/5] Add unit tests --- tests/test_cli.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 1fe02ac21..e90aac5fe 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -56,7 +56,7 @@ def teardown_method(self, test_method): self.patcher_StackActions.stop() @patch("sys.exit") - def test_catch_excecptions(self, mock_exit): + def test_catch_exceptions(self, mock_exit): @catch_exceptions def raises_exception(): raise SceptreException() @@ -64,6 +64,17 @@ def raises_exception(): raises_exception() mock_exit.assert_called_once_with(1) + def test_catch_exceptions_debug_mode(self): + @catch_exceptions + def raises_exception(): + raise SceptreException() + + logger = logging.getLogger("sceptre") + logger.setLevel(logging.DEBUG) + + with pytest.raises(SceptreException): + raises_exception() + @pytest.mark.parametrize("command,files,output", [ # one --var option (