From 8c0f498a23f4f16fb93f00094038580f965c31ae Mon Sep 17 00:00:00 2001 From: RUiNtheExtinct Date: Sun, 28 Dec 2025 14:45:13 +0530 Subject: [PATCH] fix(logger): copy logs before passing to callbacks to prevent mutation If a callback modifies the logs list (e.g., clear()) and a subsequent callback raises an exception, the preserved logs for retry would have been corrupted. Now passes a shallow copy to callbacks. --- app/logger.py | 3 ++- tests-unit/app_test/test_logger.py | 41 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/app/logger.py b/app/logger.py index 3c317882e..2c89dc061 100644 --- a/app/logger.py +++ b/app/logger.py @@ -48,7 +48,8 @@ class LogInterceptor(io.TextIOWrapper): raise if not self._logs_since_flush: return - logs_to_send = self._logs_since_flush + # Copy to prevent callback mutations from affecting retry on failure + logs_to_send = list(self._logs_since_flush) for cb in self._flush_callbacks: cb(logs_to_send) # Only clear after all callbacks succeed - if any raises, logs remain for retry diff --git a/tests-unit/app_test/test_logger.py b/tests-unit/app_test/test_logger.py index 52e1522b1..c31cedf9b 100644 --- a/tests-unit/app_test/test_logger.py +++ b/tests-unit/app_test/test_logger.py @@ -169,6 +169,47 @@ class TestLogInterceptorFlush: # Logs should be preserved for retry on next flush assert interceptor._logs_since_flush == original_logs + def test_flush_protects_logs_from_callback_mutation(self): + """Test that callback mutations don't affect preserved logs on failure.""" + from app.logger import LogInterceptor + + class MockStream: + def __init__(self): + self._buffer = io.BytesIO() + self.encoding = 'utf-8' + self.line_buffering = False + + @property + def buffer(self): + return self._buffer + + mock_stream = MockStream() + interceptor = LogInterceptor(mock_stream) + + # First callback mutates the list, second raises + def mutating_callback(logs): + logs.clear() # Mutate the passed list + + def raising_callback(logs): + raise ValueError("Callback error") + + interceptor.on_flush(mutating_callback) + interceptor.on_flush(raising_callback) + + # Add some logs + original_logs = [ + {"t": "test", "m": "message1"}, + {"t": "test", "m": "message2"} + ] + interceptor._logs_since_flush = original_logs.copy() + + # Flush should raise + with pytest.raises(ValueError, match="Callback error"): + interceptor.flush() + + # Logs should be preserved despite mutation by first callback + assert interceptor._logs_since_flush == original_logs + def test_flush_clears_logs_after_all_callbacks_succeed(self): """Test that logs are cleared only after all callbacks execute successfully.""" from app.logger import LogInterceptor