Skip to content

Commit

Permalink
Don't crash out on OSErrors in subprocess calls
Browse files Browse the repository at this point in the history
  • Loading branch information
asottile committed Mar 12, 2020
1 parent 58a16bc commit 03617b2
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 14 deletions.
12 changes: 2 additions & 10 deletions pre_commit/error_handler.py
Expand Up @@ -8,23 +8,15 @@
import pre_commit.constants as C
from pre_commit import output
from pre_commit.store import Store
from pre_commit.util import force_bytes


class FatalError(RuntimeError):
pass


def _exception_to_bytes(exc: BaseException) -> bytes:
with contextlib.suppress(TypeError):
return bytes(exc) # type: ignore
with contextlib.suppress(Exception):
return str(exc).encode()
return f'<unprintable {type(exc).__name__} object>'.encode()


def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None:
error_msg = f'{msg}: {type(exc).__name__}: '.encode()
error_msg += _exception_to_bytes(exc)
error_msg = f'{msg}: {type(exc).__name__}: '.encode() + force_bytes(exc)
output.write_line_b(error_msg)
log_path = os.path.join(Store().directory, 'pre-commit.log')
output.write_line(f'Check the log at {log_path}')
Expand Down
28 changes: 24 additions & 4 deletions pre_commit/util.py
Expand Up @@ -43,6 +43,14 @@ def yaml_dump(o: Any) -> str:
)


def force_bytes(exc: Any) -> bytes:
with contextlib.suppress(TypeError):
return bytes(exc)
with contextlib.suppress(Exception):
return str(exc).encode()
return f'<unprintable {type(exc).__name__} object>'.encode()


@contextlib.contextmanager
def clean_path_on_failure(path: str) -> Generator[None, None, None]:
"""Cleans up the directory on an exceptional failure."""
Expand Down Expand Up @@ -120,6 +128,10 @@ def _setdefault_kwargs(kwargs: Dict[str, Any]) -> None:
kwargs.setdefault(arg, subprocess.PIPE)


def _oserror_to_output(e: OSError) -> Tuple[int, bytes, None]:
return 1, force_bytes(e).rstrip(b'\n') + b'\n', None


def cmd_output_b(
*cmd: str,
retcode: Optional[int] = 0,
Expand All @@ -132,9 +144,13 @@ def cmd_output_b(
except parse_shebang.ExecutableNotFoundError as e:
returncode, stdout_b, stderr_b = e.to_output()
else:
proc = subprocess.Popen(cmd, **kwargs)
stdout_b, stderr_b = proc.communicate()
returncode = proc.returncode
try:
proc = subprocess.Popen(cmd, **kwargs)
except OSError as e:
returncode, stdout_b, stderr_b = _oserror_to_output(e)
else:
stdout_b, stderr_b = proc.communicate()
returncode = proc.returncode

if retcode is not None and retcode != returncode:
raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)
Expand Down Expand Up @@ -205,7 +221,11 @@ def cmd_output_p(
with open(os.devnull) as devnull, Pty() as pty:
assert pty.r is not None
kwargs.update({'stdin': devnull, 'stdout': pty.w, 'stderr': pty.w})
proc = subprocess.Popen(cmd, **kwargs)
try:
proc = subprocess.Popen(cmd, **kwargs)
except OSError as e:
return _oserror_to_output(e)

pty.close_w()

buf = b''
Expand Down
13 changes: 13 additions & 0 deletions tests/util_test.py
Expand Up @@ -9,6 +9,7 @@
from pre_commit.util import cmd_output
from pre_commit.util import cmd_output_b
from pre_commit.util import cmd_output_p
from pre_commit.util import make_executable
from pre_commit.util import parse_version
from pre_commit.util import rmtree
from pre_commit.util import tmpdir
Expand Down Expand Up @@ -92,6 +93,18 @@ def test_cmd_output_exe_not_found_bytes(fn):
assert out == b'Executable `dne` not found'


@pytest.mark.parametrize('fn', (cmd_output_b, cmd_output_p))
def test_cmd_output_no_shebang(tmpdir, fn):
f = tmpdir.join('f').ensure()
make_executable(f)

# previously this raised `OSError` -- the output is platform specific
ret, out, _ = fn(str(f), retcode=None, stderr=subprocess.STDOUT)
assert ret == 1
assert isinstance(out, bytes)
assert out.endswith(b'\n')


def test_parse_version():
assert parse_version('0.0') == parse_version('0.0')
assert parse_version('0.1') > parse_version('0.0')
Expand Down

0 comments on commit 03617b2

Please sign in to comment.