[Partner Nodes] add Sonilo Audio nodes#13391
Merged
Kosinkadink merged 3 commits intomasterfrom Apr 14, 2026
Merged
Conversation
bigcat88
requested review from
Kosinkadink,
comfyanonymous and
guill
as code owners
bigcat88
changed the title
📝 WalkthroughWalkthroughAdds a new ComfyUI extension module 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api_nodes/nodes_sonilo.py`:
- Around line 259-260: The return currently uses
next(iter(audio_streams.values())) which picks whichever stream yields first;
change it to deterministically pick stream index 0 when present (or a stable
fallback like the lowest numeric key) before joining bytes so outputs don't vary
with network timing—locate the code in nodes_sonilo.py where
PromptServer.instance.send_progress_text is called and replace the
non-deterministic selection with a lookup for audio_streams[0] if it exists,
otherwise sort the keys and pick the first key consistently, then b"".join(...)
that chosen stream.
- Around line 186-200: Wrap the Sonilo API request/response block (the
aiohttp.ClientSession post call and the resp.content.readline loop inside the
async with) in a try/except that catches asyncio.TimeoutError and
aiohttp.ClientError, and re-raises a clear, consistent Exception (or a
domain-specific exception if used elsewhere) with a user-friendly message that
includes the original exception text; ensure you still call
PromptServer.instance.send_progress_text("Status: Queued", node_id) before the
request, preserve the existing handling for HTTP errors using
_extract_error_message, and keep the
is_processing_interrupted()/ProcessingInterrupted flow intact so cancellation
still works.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8cc53e19-3cae-4dd4-a3b4-43eb89beacd4
📒 Files selected for processing (1)
comfy_api_nodes/nodes_sonilo.py
bigcat88
force-pushed
the
feat/api-nodes/sonilo-nodes
branch
from
fe47dd8 to
06c108c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
comfy_api_nodes/nodes_sonilo.py (1)
186-200:⚠️ Potential issue | 🟠 MajorWrap network/timeout errors in explicit error handling.
The
aiohttprequest can raiseasyncio.TimeoutErrororaiohttp.ClientErrorthat would bubble up as low-level errors rather than user-friendly messages. Consider catching these and raising a clear exception.Proposed fix
+import asyncio import base64timeout = aiohttp.ClientTimeout(total=1200.0, sock_read=300.0) async with aiohttp.ClientSession(timeout=timeout) as session: PromptServer.instance.send_progress_text("Status: Queued", node_id) - async with session.post(url, data=form, headers=headers) as resp: - if resp.status >= 400: - msg = await _extract_error_message(resp) - raise Exception(f"Sonilo API error ({resp.status}): {msg}") - - while True: + try: + async with session.post(url, data=form, headers=headers) as resp: + if resp.status >= 400: + msg = await _extract_error_message(resp) + raise Exception(f"Sonilo API error ({resp.status}): {msg}") + + while True: # ... existing loop code indented one level ... + except asyncio.TimeoutError: + raise Exception("Sonilo API request timed out. Please try again.") from None + except aiohttp.ClientError as e: + raise Exception(f"Sonilo API network error: {e}") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@comfy_api_nodes/nodes_sonilo.py` around lines 186 - 200, Wrap the network call and streaming read in explicit exception handling: around the aiohttp.ClientSession() / session.post(...) and the resp.content.readline() loop in the block using ClientSession, catch asyncio.TimeoutError and aiohttp.ClientError (and optionally Exception for unexpected errors) and raise a clear, user-friendly Exception (or a custom error) that includes context like "Sonilo API request failed" and the original error message; preserve existing behavior for HTTP error responses (the _extract_error_message and raise Exception for resp.status >= 400) and re-raise ProcessingInterrupted unchanged when is_processing_interrupted() is true so cancellation still works.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api_nodes/nodes_sonilo.py`:
- Around line 156-158: The prompt is validated using
validate_string(strip_whitespace=True) but the original unstripped prompt is
added to the FormData; update the code after validation to assign the stripped
value (e.g., prompt = prompt.strip()) before calling form.add_field("prompt",
prompt) so it matches the behavior used in SoniloVideoToMusic and ensures
consistent whitespace handling between validate_string and form.add_field.
- Around line 237-243: In the audio_chunk handling branch (the block that reads
evt_type == "audio_chunk" and uses evt, stream_idx, audio_streams), validate the
presence and type of evt["data"] before calling base64.b64decode: use data =
evt.get("data") and if data is missing or not a str, raise or log a clear error
(e.g., ValueError with context including evt and stream_idx) and skip
processing; also wrap base64.b64decode(...) in a try/except to catch decoding
errors and surface a helpful message rather than letting a bare
KeyError/decoding exception bubble up.
---
Duplicate comments:
In `@comfy_api_nodes/nodes_sonilo.py`:
- Around line 186-200: Wrap the network call and streaming read in explicit
exception handling: around the aiohttp.ClientSession() / session.post(...) and
the resp.content.readline() loop in the block using ClientSession, catch
asyncio.TimeoutError and aiohttp.ClientError (and optionally Exception for
unexpected errors) and raise a clear, user-friendly Exception (or a custom
error) that includes context like "Sonilo API request failed" and the original
error message; preserve existing behavior for HTTP error responses (the
_extract_error_message and raise Exception for resp.status >= 400) and re-raise
ProcessingInterrupted unchanged when is_processing_interrupted() is true so
cancellation still works.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 705610dd-8555-4ead-89e5-21c92956ce4e
📒 Files selected for processing (1)
comfy_api_nodes/nodes_sonilo.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api_nodes/nodes_sonilo.py`:
- Around line 195-201: The loop that reads resp.content.readline() in
nodes_sonilo.py (using is_processing_interrupted() and raising
ProcessingInterrupted) currently treats an unexpected EOF as a successful
completion; change the logic so that if raw_line is empty and you have not yet
seen the terminal "complete" event you raise an appropriate exception (e.g., an
API/StreamError) instead of breaking and returning "Completed"; update both the
occurrence around the readline loop and the similar block at lines 257-265 to
track whether a "complete" event was received (a boolean flag) and if EOF is
reached without that flag set, log/raise a descriptive error so truncated audio
is not reported as success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 29d3efd8-0a05-410a-976b-a2e08d4dbf22
📒 Files selected for processing (1)
comfy_api_nodes/nodes_sonilo.py
Signed-off-by: bigcat88 <bigcat88@icloud.com>
Signed-off-by: bigcat88 <bigcat88@icloud.com>
Signed-off-by: bigcat88 <bigcat88@icloud.com>
bigcat88
force-pushed
the
feat/api-nodes/sonilo-nodes
branch
from
2070c99 to
adda132
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (4)
comfy_api_nodes/nodes_sonilo.py (4)
156-158:⚠️ Potential issue | 🟡 MinorStrip the validated prompt before sending it.
Line 156 validates the stripped value, but Line 158 still posts the original whitespace-padded prompt. That makes
SoniloTextToMusicbehave differently fromSoniloVideoToMusicand can change generation for prompts with leading/trailing spaces.Suggested fix
validate_string(prompt, strip_whitespace=True, min_length=1) + prompt = prompt.strip() form = aiohttp.FormData() form.add_field("prompt", prompt)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@comfy_api_nodes/nodes_sonilo.py` around lines 156 - 158, The code validates a stripped prompt via validate_string(prompt, strip_whitespace=True) but then sends the original unstripped prompt in form.add_field("prompt", prompt), causing inconsistent behavior; update the flow so you use the stripped value when posting (e.g., assign the validated/stripped result back to prompt or to a new variable) before calling form.add_field in the SoniloTextToMusic-related code path in comfy_api_nodes/nodes_sonilo.py.
238-240:⚠️ Potential issue | 🟡 MinorValidate
audio_chunkpayloads before decoding.Line 240 assumes every
audio_chunkincludes a stringdatafield with valid base64. A malformed event currently turns into a bareKeyErroror decode exception instead of a clear Sonilo stream error. Guardevt.get("data")and the decode step so bad chunks fail with a readable Sonilo-specific message.As per coding guidelines,
comfy_api_nodes/**integrations should have proper error handling for API failures (timeouts, rate limits, auth errors).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@comfy_api_nodes/nodes_sonilo.py` around lines 238 - 240, The audio_chunk handling assumes evt["data"] exists and is valid base64; update the block that checks evt_type == "audio_chunk" to validate evt.get("data") is a non-empty string before calling base64.b64decode, handle base64.binascii.Error/TypeError from decoding, and on any validation/decoding failure raise or emit a clear Sonilo-specific error (include stream_index and the invalid payload summary) instead of letting a KeyError/raw decode exception propagate; reference the existing variables evt, stream_idx, and chunk_data when adding the guard and error handling.
199-201:⚠️ Potential issue | 🟠 MajorDon't report a dropped stream as completed.
If the backend disconnects after sending some chunks, Line 200 breaks the loop, Line 263 marks the node completed, and Line 265 returns truncated audio. Track whether a
"complete"event was seen and fail on EOF before that.Suggested fix
last_chunk_status_ts = 0.0 audio_streams: dict[int, list[bytes]] = {} title: str | None = None + saw_complete = False @@ raw_line = await resp.content.readline() if not raw_line: - break + if saw_complete: + break + raise Exception("Sonilo stream ended before completion.") @@ elif evt_type == "complete": + saw_complete = True breakAs per coding guidelines,
comfy_api_nodes/**integrations should have proper error handling for API failures (timeouts, rate limits, auth errors).Also applies to: 257-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@comfy_api_nodes/nodes_sonilo.py` around lines 199 - 201, The code currently breaks the read loop on EOF (raw_line = await resp.content.readline(); if not raw_line: break) and later marks the node as completed and returns audio even if no "complete" event was received; add a boolean (e.g., saw_complete = False) that is set true when the handler sees the "complete" event and only consider the stream successful if saw_complete is true, otherwise raise/propagate an error on EOF and avoid marking the node completed or returning truncated audio. Update the loop that reads resp.content.readline() and the event handling logic that sets completion (the code paths that set completed/return audio) to consult saw_complete, and also add proper error handling for backend failures (timeouts, rate limits, auth) around the resp I/O to surface meaningful exceptions instead of silently returning partial data.
187-190:⚠️ Potential issue | 🟠 MajorHandle Sonilo transport failures explicitly.
Line 190 and Line 199 can raise
aiohttp.ClientErroror timeout exceptions that currently bubble up as raw library errors. Wrap the POST + stream-read block and re-raise a consistent Sonilo request failure instead of leaking low-level transport details.As per coding guidelines,
comfy_api_nodes/**integrations should have proper error handling for API failures (timeouts, rate limits, auth errors).Also applies to: 199-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@comfy_api_nodes/nodes_sonilo.py` around lines 187 - 190, Wrap the POST + stream-read block around session.post(...) and the streaming read that follows in a try/except that catches aiohttp.ClientError and asyncio.TimeoutError (and any aiohttp.ClientResponseError), then re-raise a module-specific exception (e.g., SoniloTransportError or SoniloRequestError) instead of letting low-level errors bubble up; include the original exception as context (raise ... from e), and update the error path to call PromptServer.instance.send_progress_text("Status: Failed: <brief reason>", node_id) so the node surface gets a consistent failure message; add the new exception class to comfy_api_nodes/nodes_sonilo.py if it doesn't exist and use it where the POST/stream code currently lives to standardize Sonilo transport failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@comfy_api_nodes/nodes_sonilo.py`:
- Around line 156-158: The code validates a stripped prompt via
validate_string(prompt, strip_whitespace=True) but then sends the original
unstripped prompt in form.add_field("prompt", prompt), causing inconsistent
behavior; update the flow so you use the stripped value when posting (e.g.,
assign the validated/stripped result back to prompt or to a new variable) before
calling form.add_field in the SoniloTextToMusic-related code path in
comfy_api_nodes/nodes_sonilo.py.
- Around line 238-240: The audio_chunk handling assumes evt["data"] exists and
is valid base64; update the block that checks evt_type == "audio_chunk" to
validate evt.get("data") is a non-empty string before calling base64.b64decode,
handle base64.binascii.Error/TypeError from decoding, and on any
validation/decoding failure raise or emit a clear Sonilo-specific error (include
stream_index and the invalid payload summary) instead of letting a KeyError/raw
decode exception propagate; reference the existing variables evt, stream_idx,
and chunk_data when adding the guard and error handling.
- Around line 199-201: The code currently breaks the read loop on EOF (raw_line
= await resp.content.readline(); if not raw_line: break) and later marks the
node as completed and returns audio even if no "complete" event was received;
add a boolean (e.g., saw_complete = False) that is set true when the handler
sees the "complete" event and only consider the stream successful if
saw_complete is true, otherwise raise/propagate an error on EOF and avoid
marking the node completed or returning truncated audio. Update the loop that
reads resp.content.readline() and the event handling logic that sets completion
(the code paths that set completed/return audio) to consult saw_complete, and
also add proper error handling for backend failures (timeouts, rate limits,
auth) around the resp I/O to surface meaningful exceptions instead of silently
returning partial data.
- Around line 187-190: Wrap the POST + stream-read block around
session.post(...) and the streaming read that follows in a try/except that
catches aiohttp.ClientError and asyncio.TimeoutError (and any
aiohttp.ClientResponseError), then re-raise a module-specific exception (e.g.,
SoniloTransportError or SoniloRequestError) instead of letting low-level errors
bubble up; include the original exception as context (raise ... from e), and
update the error path to call PromptServer.instance.send_progress_text("Status:
Failed: <brief reason>", node_id) so the node surface gets a consistent failure
message; add the new exception class to comfy_api_nodes/nodes_sonilo.py if it
doesn't exist and use it where the POST/stream code currently lives to
standardize Sonilo transport failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 980176dc-9c6b-45d1-966d-0fb6fb3ba4df
📒 Files selected for processing (1)
comfy_api_nodes/nodes_sonilo.py
Kosinkadink
approved these changes
Apr 14, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
API Node PR Checklist
Scope
Pricing & Billing
If Need pricing update:
QA
Comms