Merge pull request #1546 from pre-commit/error_logging_read_only_filesystem

better error handling when Store is readonly

pre_commit/error_handler.py

@@ -18,10 +18,17 @@ class FatalError(RuntimeError):
 def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None:
     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}')
 
-    with open(log_path, 'wb') as log:
+    storedir = Store().directory
+    log_path = os.path.join(storedir, 'pre-commit.log')
+    with contextlib.ExitStack() as ctx:
+        if os.access(storedir, os.W_OK):
+            output.write_line(f'Check the log at {log_path}')
+            log = ctx.enter_context(open(log_path, 'wb'))
+        else:  # pragma: win32 no cover
+            output.write_line(f'Failed to write to log at {log_path}')
+            log = sys.stdout.buffer
+
         _log_line = functools.partial(output.write_line, stream=log)
         _log_line_b = functools.partial(output.write_line_b, stream=log)
 

tests/error_handler_test.py

@@ -1,13 +1,16 @@
 import os.path
 import re
+import stat
 import sys
 from unittest import mock
 
 import pytest
 
 from pre_commit import error_handler
+from pre_commit.store import Store
 from pre_commit.util import CalledProcessError
 from testing.util import cmd_output_mocked_pre_commit_home
+from testing.util import xfailif_windows
 
 
 @pytest.fixture
@@ -168,3 +171,29 @@ def test_error_handler_no_tty(tempdir_factory):
     out_lines = out.splitlines()
     assert out_lines[-2] == 'An unexpected error has occurred: ValueError: ☃'
     assert out_lines[-1] == f'Check the log at {log_file}'
+
+
+@xfailif_windows  # pragma: win32 no cover
+def test_error_handler_read_only_filesystem(mock_store_dir, cap_out, capsys):
+    # a better scenario would be if even the Store crash would be handled
+    # but realistically we're only targetting systems where the Store has
+    # already been set up
+    Store()
+
+    write = (stat.S_IWGRP | stat.S_IWOTH | stat.S_IWUSR)
+    os.chmod(mock_store_dir, os.stat(mock_store_dir).st_mode & ~write)
+
+    with pytest.raises(SystemExit):
+        with error_handler.error_handler():
+            raise ValueError('ohai')
+
+    output = cap_out.get()
+    assert output.startswith(
+        'An unexpected error has occurred: ValueError: ohai\n'
+        'Failed to write to log at ',
+    )
+
+    # our cap_out mock is imperfect so the rest of the output goes to capsys
+    out, _ = capsys.readouterr()
+    # the things that normally go to the log file will end up here
+    assert '### version information' in out