mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-01-07 21:00:49 +08:00
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.
This commit is contained in:
parent
c06b18a014
commit
ddad64a4bf
@ -39,9 +39,10 @@ class LogInterceptor(io.TextIOWrapper):
|
|||||||
# This is safe to ignore as write() already succeeded
|
# This is safe to ignore as write() already succeeded
|
||||||
if e.errno != 22:
|
if e.errno != 22:
|
||||||
raise
|
raise
|
||||||
for cb in self._flush_callbacks:
|
logs_to_send = self._logs_since_flush
|
||||||
cb(self._logs_since_flush)
|
|
||||||
self._logs_since_flush = []
|
self._logs_since_flush = []
|
||||||
|
for cb in self._flush_callbacks:
|
||||||
|
cb(logs_to_send)
|
||||||
|
|
||||||
def on_flush(self, callback):
|
def on_flush(self, callback):
|
||||||
self._flush_callbacks.append(callback)
|
self._flush_callbacks.append(callback)
|
||||||
|
|||||||
@ -132,6 +132,42 @@ class TestLogInterceptorFlush:
|
|||||||
assert callback2_results == [3]
|
assert callback2_results == [3]
|
||||||
assert callback3_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:
|
class TestLogInterceptorWrite:
|
||||||
"""Test that LogInterceptor.write() works correctly."""
|
"""Test that LogInterceptor.write() works correctly."""
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user