fix(logger): handle OSError errno 22 on flush for Windows piped streams #11539
+342
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When running ComfyUI in API mode on Windows, print() statements from custom nodes can crash with "OSError: [Errno 22] Invalid argument" during flush. This occurs because piped/redirected stdout streams on Windows may fail to flush even after successful writes. This fix catches OSError with errno 22 (EINVAL) specifically in LogInterceptor.flush(), allowing the flush callbacks to still execute. The error is safe to ignore since write() already succeeded. Fixes comfyanonymous#11367
RUiNtheExtinct
requested review from
Kosinkadink,
comfyanonymous and
guill
as code owners
Move _logs_since_flush reset outside the callback loop so all registered callbacks receive the same log data instead of only the first callback getting logs while subsequent ones get an empty list. Add test to verify multiple callbacks all receive the same logs.
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.
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.
If flush callbacks persistently fail (e.g., network issues), logs would accumulate indefinitely in _logs_since_flush, potentially causing OOM on long-running servers. Added MAX_PENDING_LOGS (10000) limit - when exceeded, oldest logs are dropped. This is similar to how the global logs deque uses maxlen.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
print()in API mode on WindowsOSErrorwith errno 22 (EINVAL) inLogInterceptor.flush()that can occur with piped/redirected stdoutProblem
When running ComfyUI in API mode on Windows, print statements from custom nodes crash with:
This occurs because the log interceptor's
flush()method fails on piped/redirected streams, even though thewrite()operation succeeded.Solution
Wrap the
super().flush()call in a try-except block that specifically catches errno 22. This error is safe to ignore because:write()already succeededTest plan
tests-unit/app_test/test_logger.pypython -m pytest tests-unit/app_test/test_logger.py -v)Fixes #11367