From 494dce9a36ebb1c0bc810544721372311b1e7d3d Mon Sep 17 00:00:00 2001 From: RUiNtheExtinct Date: Sun, 28 Dec 2025 13:49:09 +0530 Subject: [PATCH] fix(logger): preserve logs for retry when callback raises exception Move log clearing to after all callbacks succeed, not before. This ensures that if any callback raises an exception, the logs remain available for retry on the next flush call instead of being lost. The previous approach cleared logs before iterating callbacks, which meant logs were permanently lost if any callback failed. --- app/logger.py | 5 ++- tests-unit/app_test/test_logger.py | 52 ++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/app/logger.py b/app/logger.py index b007bf8cf..22ee2b11b 100644 --- a/app/logger.py +++ b/app/logger.py @@ -39,10 +39,13 @@ class LogInterceptor(io.TextIOWrapper): # This is safe to ignore as write() already succeeded if e.errno != 22: raise + if not self._logs_since_flush: + return logs_to_send = self._logs_since_flush - 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 + self._logs_since_flush = [] def on_flush(self, callback): self._flush_callbacks.append(callback) diff --git a/tests-unit/app_test/test_logger.py b/tests-unit/app_test/test_logger.py index 8f1ff8d65..b57ba1887 100644 --- a/tests-unit/app_test/test_logger.py +++ b/tests-unit/app_test/test_logger.py @@ -132,8 +132,8 @@ class TestLogInterceptorFlush: assert callback2_results == [3] assert callback3_results == [3] - def test_flush_clears_logs_even_if_callback_raises(self): - """Test that logs are cleared even if a callback raises an exception.""" + def test_flush_preserves_logs_when_callback_raises(self): + """Test that logs are preserved for retry if a callback raises an exception.""" from app.logger import LogInterceptor class MockStream: @@ -155,17 +155,57 @@ class TestLogInterceptorFlush: 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 for retry on next flush + 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 + + 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) + + # Register multiple callbacks + callback1_results = [] + callback2_results = [] + interceptor.on_flush(lambda logs: callback1_results.append(len(logs))) + interceptor.on_flush(lambda logs: callback2_results.append(len(logs))) + # Add some logs interceptor._logs_since_flush = [ {"t": "test", "m": "message1"}, {"t": "test", "m": "message2"} ] - # Flush should raise but logs should still be cleared - with pytest.raises(ValueError, match="Callback error"): - interceptor.flush() + # Flush should succeed + interceptor.flush() - # Logs should be cleared to prevent duplicates on next flush + # All callbacks should have executed + assert callback1_results == [2] + assert callback2_results == [2] + + # Logs should be cleared after success assert interceptor._logs_since_flush == []