Fix a race condition in which borgmatic sometimes swallows Borg error output without logging it (#1256).

tests/integration/test_execute.py

@@ -557,6 +557,43 @@ def test_log_outputs_with_unfinished_process_re_polls():
     )
 
 
+def test_log_outputs_includes_error_output_when_output_spans_multiple_chunks():
+    flexmock(module.logger).should_receive('log')
+    flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.ERROR)
+    flexmock(module).should_receive('command_for_process').and_return('python')
+
+    process = subprocess.Popen(
+        [
+            sys.executable,
+            '-c',
+            (
+                'import os, sys; '
+                f'os.write(sys.stdout.fileno(), b"x" * {module.READ_CHUNK_SIZE + 10}); '
+                'os.write(sys.stdout.fileno(), b"\\nERROR: critical failure"); '
+                'os.close(1); '
+                'os._exit(2)'
+            ),
+        ],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+    )
+    flexmock(module).should_receive('output_buffers_for_process').and_return((process.stdout,))
+
+    with pytest.raises(subprocess.CalledProcessError) as error:
+        tuple(
+            module.log_outputs(
+                (process,),
+                exclude_stdouts=(),
+                output_log_level=logging.INFO,
+                borg_local_path='borg',
+                borg_exit_codes=None,
+            )
+        )
+
+    assert error.value.output
+    assert 'ERROR: critical failure' in error.value.output
+
+
 def test_read_lines_uses_system_locale_when_decoding_output():
     flexmock(module.locale).should_receive('getpreferredencoding').and_return('ISO-8859-1')
     process = subprocess.Popen(['echo', b'\xc4pple'], stdout=subprocess.PIPE)

tests/unit/test_execute.py

@@ -860,6 +860,7 @@ def test_raise_for_process_errors_with_error_process_raises():
         process: module.Process_metadata(last_lines=['hi', 'there'], capture=False)
     }
     flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.ERROR)
+    flexmock(module).should_receive('log_remaining_buffer_lines').and_return(())
     command = flexmock()
     flexmock(module).should_receive('command_for_process').and_return(command)
 
@@ -890,6 +891,7 @@ def test_raise_for_process_errors_with_success_process_and_warning_process_retur
     flexmock(module).should_receive('interpret_exit_code').with_args(
         object, 1, object, object
     ).and_return(module.Exit_status.WARNING)
+    flexmock(module).should_receive('log_remaining_buffer_lines').and_return(())
     flexmock(module).should_receive('command_for_process').never()
 
     assert (
@@ -917,6 +919,7 @@ def test_raise_for_process_errors_with_warning_process_and_error_process_raises(
     flexmock(module).should_receive('interpret_exit_code').with_args(
         object, 3, object, object
     ).and_return(module.Exit_status.ERROR)
+    flexmock(module).should_receive('log_remaining_buffer_lines').and_return(())
     command = flexmock()
     flexmock(module).should_receive('command_for_process').and_return(command)
 
@@ -947,6 +950,7 @@ def test_raise_for_process_errors_with_warning_process_and_running_process_kills
     flexmock(module).should_receive('interpret_exit_code').with_args(
         object, 1, object, object
     ).and_return(module.Exit_status.WARNING)
+    flexmock(module).should_receive('log_remaining_buffer_lines').and_return(())
     command = flexmock()
     flexmock(module).should_receive('command_for_process').and_return(command)
 
@@ -975,6 +979,7 @@ def test_raise_for_process_errors_with_error_process_and_running_process_kills_a
     flexmock(module).should_receive('interpret_exit_code').with_args(
         object, 3, object, object
     ).and_return(module.Exit_status.ERROR)
+    flexmock(module).should_receive('log_remaining_buffer_lines').and_return(())
     command = flexmock()
     flexmock(module).should_receive('command_for_process').and_return(command)
 
@@ -998,6 +1003,7 @@ def test_raise_for_process_errors_with_warning_process_and_long_output_raises_wi
         process: module.Process_metadata(last_lines=['hi', 'there'], capture=False)
     }
     flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.ERROR)
+    flexmock(module).should_receive('log_remaining_buffer_lines').and_return(())
     command = flexmock()
     flexmock(module).should_receive('command_for_process').and_return(command)