mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-01-07 21:00:49 +08:00
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.
This commit is contained in:
parent
ddad64a4bf
commit
494dce9a36
@ -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)
|
||||
|
||||
@ -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 == []
|
||||
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user