mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-01-07 21:00:49 +08:00
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.
This commit is contained in:
parent
1a410446e3
commit
8c0f498a23
@ -48,7 +48,8 @@ class LogInterceptor(io.TextIOWrapper):
|
|||||||
raise
|
raise
|
||||||
if not self._logs_since_flush:
|
if not self._logs_since_flush:
|
||||||
return
|
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:
|
for cb in self._flush_callbacks:
|
||||||
cb(logs_to_send)
|
cb(logs_to_send)
|
||||||
# Only clear after all callbacks succeed - if any raises, logs remain for retry
|
# Only clear after all callbacks succeed - if any raises, logs remain for retry
|
||||||
|
|||||||
@ -169,6 +169,47 @@ class TestLogInterceptorFlush:
|
|||||||
# Logs should be preserved for retry on next flush
|
# Logs should be preserved for retry on next flush
|
||||||
assert interceptor._logs_since_flush == original_logs
|
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):
|
def test_flush_clears_logs_after_all_callbacks_succeed(self):
|
||||||
"""Test that logs are cleared only after all callbacks execute successfully."""
|
"""Test that logs are cleared only after all callbacks execute successfully."""
|
||||||
from app.logger import LogInterceptor
|
from app.logger import LogInterceptor
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user