Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinct exit codes for the various error cases #1601

Merged
merged 1 commit into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 11 additions & 6 deletions pre_commit/error_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
from pre_commit.util import force_bytes


def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None:
def _log_and_exit(
msg: str,
ret_code: int,
exc: BaseException,
formatted: str,
) -> None:
error_msg = f'{msg}: {type(exc).__name__}: '.encode() + force_bytes(exc)
output.write_line_b(error_msg)

Expand Down Expand Up @@ -51,7 +56,7 @@ def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None:
_log_line('```')
_log_line(formatted.rstrip())
_log_line('```')
raise SystemExit(1)
raise SystemExit(ret_code)


@contextlib.contextmanager
Expand All @@ -60,9 +65,9 @@ def error_handler() -> Generator[None, None, None]:
yield
except (Exception, KeyboardInterrupt) as e:
if isinstance(e, FatalError):
msg = 'An error has occurred'
msg, ret_code = 'An error has occurred', 1
elif isinstance(e, KeyboardInterrupt):
msg = 'Interrupted (^C)'
msg, ret_code = 'Interrupted (^C)', 130
asottile marked this conversation as resolved.
Show resolved Hide resolved
else:
msg = 'An unexpected error has occurred'
_log_and_exit(msg, e, traceback.format_exc())
msg, ret_code = 'An unexpected error has occurred', 3
_log_and_exit(msg, ret_code, e, traceback.format_exc())
16 changes: 10 additions & 6 deletions tests/error_handler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def test_error_handler_fatal_error(mocked_log_and_exit):

mocked_log_and_exit.assert_called_once_with(
'An error has occurred',
1,
exc,
# Tested below
mock.ANY,
Expand All @@ -47,7 +48,7 @@ def test_error_handler_fatal_error(mocked_log_and_exit):
r' raise exc\n'
r'(pre_commit\.errors\.)?FatalError: just a test\n',
)
pattern.assert_matches(mocked_log_and_exit.call_args[0][2])
pattern.assert_matches(mocked_log_and_exit.call_args[0][3])


def test_error_handler_uncaught_error(mocked_log_and_exit):
Expand All @@ -57,6 +58,7 @@ def test_error_handler_uncaught_error(mocked_log_and_exit):

mocked_log_and_exit.assert_called_once_with(
'An unexpected error has occurred',
3,
exc,
# Tested below
mock.ANY,
Expand All @@ -70,7 +72,7 @@ def test_error_handler_uncaught_error(mocked_log_and_exit):
r' raise exc\n'
r'ValueError: another test\n',
)
pattern.assert_matches(mocked_log_and_exit.call_args[0][2])
pattern.assert_matches(mocked_log_and_exit.call_args[0][3])


def test_error_handler_keyboardinterrupt(mocked_log_and_exit):
Expand All @@ -80,6 +82,7 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit):

mocked_log_and_exit.assert_called_once_with(
'Interrupted (^C)',
130,
exc,
# Tested below
mock.ANY,
Expand All @@ -93,7 +96,7 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit):
r' raise exc\n'
r'KeyboardInterrupt\n',
)
pattern.assert_matches(mocked_log_and_exit.call_args[0][2])
pattern.assert_matches(mocked_log_and_exit.call_args[0][3])


def test_log_and_exit(cap_out, mock_store_dir):
Expand All @@ -103,8 +106,9 @@ def test_log_and_exit(cap_out, mock_store_dir):
'pre_commit.errors.FatalError: hai\n'
)

with pytest.raises(SystemExit):
error_handler._log_and_exit('msg', FatalError('hai'), tb)
with pytest.raises(SystemExit) as excinfo:
error_handler._log_and_exit('msg', 1, FatalError('hai'), tb)
assert excinfo.value.code == 1

printed = cap_out.get()
log_file = os.path.join(mock_store_dir, 'pre-commit.log')
Expand Down Expand Up @@ -170,7 +174,7 @@ def test_error_handler_no_tty(tempdir_factory):
'from pre_commit.error_handler import error_handler\n'
'with error_handler():\n'
' raise ValueError("\\u2603")\n',
retcode=1,
retcode=3,
tempdir_factory=tempdir_factory,
pre_commit_home=pre_commit_home,
)
Expand Down