From ddad64a4bfc2cb34edd82093194b8f2318f83765 Mon Sep 17 00:00:00 2001 From: RUiNtheExtinct Date: Sun, 28 Dec 2025 13:36:59 +0530 Subject: [PATCH] fix(logger): prevent duplicate logs when callback raises exception Clear _logs_since_flush before iterating callbacks by capturing logs into a local variable first. This prevents duplicate logs if a callback raises an exception, since the instance variable is already cleared before any callback runs. Add test to verify logs are cleared even when callbacks raise. --- app/logger.py | 5 +++-- tests-unit/app_test/test_logger.py | 36 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/app/logger.py b/app/logger.py index da7ff908d..b007bf8cf 100644 --- a/app/logger.py +++ b/app/logger.py @@ -39,9 +39,10 @@ class LogInterceptor(io.TextIOWrapper): # This is safe to ignore as write() already succeeded if e.errno != 22: raise - for cb in self._flush_callbacks: - cb(self._logs_since_flush) + logs_to_send = self._logs_since_flush self._logs_since_flush = [] + for cb in self._flush_callbacks: + cb(logs_to_send) 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 e749c2a2a..8f1ff8d65 100644 --- a/tests-unit/app_test/test_logger.py +++ b/tests-unit/app_test/test_logger.py @@ -132,6 +132,42 @@ 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.""" + 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 a callback that raises + def raising_callback(logs): + raise ValueError("Callback error") + + interceptor.on_flush(raising_callback) + + # 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() + + # Logs should be cleared to prevent duplicates on next flush + assert interceptor._logs_since_flush == [] + class TestLogInterceptorWrite: """Test that LogInterceptor.write() works correctly."""