Skip to content

Stop logging unhandled ComfyUI websocket messages at Warn - #1797

Open
deepratna-awale wants to merge 3 commits into
LykosAI:mainfrom
deepratna-awale:quiet-comfy-websocket-logging
Open

deepratna-awale wants to merge 3 commits into
LykosAI:mainfrom
deepratna-awale:quiet-comfy-websocket-logging

Conversation

@deepratna-awale

Copy link
Copy Markdown

Stacked on #1796. This branch includes #1796's two commits; only the last commit ("Stop logging unhandled ComfyUI websocket messages at Warn") is new here. Once #1796 merges, this PR's diff will show only that commit.

Summary

Follow-up to #1796 for #1790. It addresses what is likely the main source of the log volume reported there.

ComfyClient.HandleTextMessage only acts on executing, status, progress and execution_error. Every other websocket message fell through to:

Logger.Warn($"Unknown message type {json.Type} ({json.Data}), skipping");

That wrote the full pretty-printed JSON payload to app.log at Warn, so no log level setting could hide it. This affected normal ComfyUI traffic (execution_start, execution_cached, executed, execution_success, progress_state) and custom-node broadcasts such as resource monitors that send a message every second. progress_state lists every node in the workflow, so one generation of a large workflow could add thousands of lines.

Changes

  • Added ExecutionSuccess, ExecutionInterrupted and ProgressState to ComfyWebSocketResponseType.
  • Standard ComfyUI message types the client doesn't use are now skipped silently.
  • Any other type, e.g. custom-node broadcasts, is logged at Trace with only its type. The full message is still available at Trace from the existing Received json message line.
  • Invalid JSON is still logged at Warn.
  • HandleTextMessage is now internal so it can be tested (Core already has InternalsVisibleTo for the test project).

Testing

  • New ComfyClientMessageLoggingTests covers each standard type and a custom-node type logging nothing above Trace, the custom type logging its type without the payload, invalid JSON still warning, and the new enum values deserializing. On the previous code, 8 of these tests fail.
  • Replayed one 20-step generation plus 60 seconds of monitor messages (84 messages) through the app's logging setup. Before: 84 Warn entries and 468 lines in app.log. After: none.
  • Full test suite: the only failures are the network-dependent PackageLinkTests (TestLicenseUrl / TestPreviewImageUri), which couldn't reach the internet in my test environment and are unrelated.

The file log was fixed at Debug and the console at Trace, with no way for
users to reduce how much gets written (LykosAI#1790).

Add a Log Level option under Settings > System Settings. It sets the
minimum level of the catch-all file and console rules, is applied when
settings load and whenever it changes, and leaves the per-logger filters
(System.*, Microsoft.*, etc.) untouched. The default stays at Debug so
existing behaviour is unchanged.
Use Info as the default for new and existing settings files, and start the
file and console rules at the same level so messages logged before settings
load match the default.
HandleTextMessage only acts on executing, status, progress and
execution_error. Every other message fell through to a Warn that included
the full pretty-printed JSON payload, so normal ComfyUI traffic
(execution_start, execution_cached, executed, execution_success,
progress_state) and custom node broadcasts such as resource monitors
that send a message every second filled app.log.

Recognise the standard ComfyUI message types the client doesn't use and
skip them silently, and log any other type at Trace with only its type.
The full message is still available at Trace from the existing
"Received json message" line. Invalid JSON is still logged at Warn.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant