From 9a017dcbe9db007957873ddcec8360918a5fd01a Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 28 Jul 2014 11:02:36 -0700 Subject: [PATCH] Add error_handler and use it. --- pre_commit/error_handler.py | 42 +++++++++++++ pre_commit/errors.py | 6 ++ pre_commit/five.py | 1 - pre_commit/jsonschema_extensions.py | 12 ++-- pre_commit/main.py | 42 ++++++------- pre_commit/output.py | 14 ++--- pylintrc | 2 +- tests/error_handler_test.py | 93 +++++++++++++++++++++++++++++ 8 files changed, 177 insertions(+), 35 deletions(-) create mode 100644 pre_commit/error_handler.py create mode 100644 pre_commit/errors.py create mode 100644 tests/error_handler_test.py diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py new file mode 100644 index 00000000..c8d2bfc8 --- /dev/null +++ b/pre_commit/error_handler.py @@ -0,0 +1,42 @@ +from __future__ import absolute_import +from __future__ import print_function +from __future__ import unicode_literals + +import contextlib +import io +import os.path +import traceback + +from pre_commit.errors import FatalError +from pre_commit.store import Store + + +# For testing purposes +class PreCommitSystemExit(SystemExit): + pass + + +def _log_and_exit(msg, exc, formatted, print_fn=print): + error_msg = '{0}: {1}: {2}'.format(msg, type(exc).__name__, exc) + print_fn(error_msg) + print_fn('Check the log at ~/.pre-commit/pre-commit.log') + store = Store() + store.require_created() + with io.open(os.path.join(store.directory, 'pre-commit.log'), 'w') as log: + log.write(error_msg + '\n') + log.write(formatted + '\n') + raise PreCommitSystemExit(1) + + +@contextlib.contextmanager +def error_handler(): + try: + yield + except FatalError as e: + _log_and_exit('An error has occurred', e, traceback.format_exc()) + except Exception as e: + _log_and_exit( + 'An unexpected error has occurred', + e, + traceback.format_exc(), + ) diff --git a/pre_commit/errors.py b/pre_commit/errors.py new file mode 100644 index 00000000..4dedbfc2 --- /dev/null +++ b/pre_commit/errors.py @@ -0,0 +1,6 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + + +class FatalError(RuntimeError): + pass diff --git a/pre_commit/five.py b/pre_commit/five.py index 525a6de7..a129d1db 100644 --- a/pre_commit/five.py +++ b/pre_commit/five.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -"""five: six, redux""" # pylint:disable=invalid-name PY2 = str is bytes PY3 = str is not bytes diff --git a/pre_commit/jsonschema_extensions.py b/pre_commit/jsonschema_extensions.py index f7608135..4fcbe981 100644 --- a/pre_commit/jsonschema_extensions.py +++ b/pre_commit/jsonschema_extensions.py @@ -20,20 +20,20 @@ def extend_validator_cls(validator_cls, modify): def default_values(properties, instance): - for property, subschema in properties.items(): + for prop, subschema in properties.items(): if 'default' in subschema: instance.setdefault( - property, copy.deepcopy(subschema['default']), + prop, copy.deepcopy(subschema['default']), ) def remove_default_values(properties, instance): - for property, subschema in properties.items(): + for prop, subschema in properties.items(): if ( - 'default' in subschema and - instance.get(property) == subschema['default'] + 'default' in subschema and + instance.get(prop) == subschema['default'] ): - del instance[property] + del instance[prop] _AddDefaultsValidator = extend_validator_cls( diff --git a/pre_commit/main.py b/pre_commit/main.py index 90c60eb0..2d69fb81 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -9,6 +9,7 @@ from pre_commit.commands.clean import clean from pre_commit.commands.install_uninstall import install from pre_commit.commands.install_uninstall import uninstall from pre_commit.commands.run import run +from pre_commit.error_handler import error_handler from pre_commit.runner import Runner from pre_commit.util import entry @@ -83,28 +84,29 @@ def main(argv): else: parser.parse_args(['--help']) - runner = Runner.create() + with error_handler(): + runner = Runner.create() - if args.command == 'install': - return install( - runner, overwrite=args.overwrite, hooks=args.install_hooks, - ) - elif args.command == 'uninstall': - return uninstall(runner) - elif args.command == 'clean': - return clean(runner) - elif args.command == 'autoupdate': - return autoupdate(runner) - elif args.command == 'run': - return run(runner, args) - else: - raise NotImplementedError( - 'Command {0} not implemented.'.format(args.command) - ) + if args.command == 'install': + return install( + runner, overwrite=args.overwrite, hooks=args.install_hooks, + ) + elif args.command == 'uninstall': + return uninstall(runner) + elif args.command == 'clean': + return clean(runner) + elif args.command == 'autoupdate': + return autoupdate(runner) + elif args.command == 'run': + return run(runner, args) + else: + raise NotImplementedError( + 'Command {0} not implemented.'.format(args.command) + ) - raise AssertionError( - 'Command {0} failed to exit with a returncode'.format(args.command) - ) + raise AssertionError( + 'Command {0} failed to exit with a returncode'.format(args.command) + ) if __name__ == '__main__': diff --git a/pre_commit/output.py b/pre_commit/output.py index eabb253f..0c0e9ed1 100644 --- a/pre_commit/output.py +++ b/pre_commit/output.py @@ -16,13 +16,13 @@ COLS = int( def get_hook_message( - start, - postfix='', - end_msg=None, - end_len=0, - end_color=None, - use_color=None, - cols=COLS, + start, + postfix='', + end_msg=None, + end_len=0, + end_color=None, + use_color=None, + cols=COLS, ): """Prints a message for running a hook. diff --git a/pylintrc b/pylintrc index f82bfd76..e57af29d 100644 --- a/pylintrc +++ b/pylintrc @@ -1,5 +1,5 @@ [MESSAGES CONTROL] -disable=missing-docstring,abstract-method,redefined-builtin,useless-else-on-loop,redefined-outer-name,invalid-name +disable=locally-disabled,fixme,missing-docstring,abstract-method,useless-else-on-loop,invalid-name [REPORTS] output-format=colorized diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py new file mode 100644 index 00000000..2b50fb36 --- /dev/null +++ b/tests/error_handler_test.py @@ -0,0 +1,93 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +import io +import os.path +import mock +import pytest +import re + +from pre_commit import error_handler +from pre_commit.errors import FatalError + + +@pytest.yield_fixture +def mocked_log_and_exit(): + with mock.patch.object(error_handler, '_log_and_exit') as log_and_exit: + yield log_and_exit + + +def test_error_handler_no_exception(mocked_log_and_exit): + with error_handler.error_handler(): + pass + assert mocked_log_and_exit.call_count == 0 + + +def test_error_handler_fatal_error(mocked_log_and_exit): + exc = FatalError('just a test') + with error_handler.error_handler(): + raise exc + + mocked_log_and_exit.assert_called_once_with( + 'An error has occurred', + exc, + # Tested below + mock.ANY, + ) + + assert re.match( + 'Traceback \(most recent call last\):\n' + ' File ".+/pre_commit/error_handler.py", line \d+, in error_handler\n' + ' yield\n' + ' File ".+/tests/error_handler_test.py", line \d+, ' + 'in test_error_handler_fatal_error\n' + ' raise exc\n' + '(pre_commit\.errors\.)?FatalError: just a test\n', + mocked_log_and_exit.call_args[0][2], + ) + + +def test_error_handler_uncaught_error(mocked_log_and_exit): + exc = ValueError('another test') + with error_handler.error_handler(): + raise exc + + mocked_log_and_exit.assert_called_once_with( + 'An unexpected error has occurred', + exc, + # Tested below + mock.ANY, + ) + assert re.match( + 'Traceback \(most recent call last\):\n' + ' File ".+/pre_commit/error_handler.py", line \d+, in error_handler\n' + ' yield\n' + ' File ".+/tests/error_handler_test.py", line \d+, ' + 'in test_error_handler_uncaught_error\n' + ' raise exc\n' + 'ValueError: another test\n', + mocked_log_and_exit.call_args[0][2], + ) + + +def test_log_and_exit(mock_out_store_directory): + mocked_print = mock.Mock() + with pytest.raises(error_handler.PreCommitSystemExit): + error_handler._log_and_exit( + 'msg', FatalError('hai'), "I'm a stacktrace", + print_fn=mocked_print, + ) + + printed = '\n'.join(call[0][0] for call in mocked_print.call_args_list) + assert printed == ( + 'msg: FatalError: hai\n' + 'Check the log at ~/.pre-commit/pre-commit.log' + ) + + log_file = os.path.join(mock_out_store_directory, 'pre-commit.log') + assert os.path.exists(log_file) + contents = io.open(log_file).read() + assert contents == ( + 'msg: FatalError: hai\n' + "I'm a stacktrace\n" + )