refactor(#12797): fail-fast on audio/embedding errors, drop fabricated zero/empty outputs - #12971
Conversation
…d zero/empty outputs Fallback-slop sweep for the embeddings/TTS provider group (#12182 family, extends #9324). The parent sweep #12793 already converted the fake-audio/ zero-vector error paths in these three plugins; this closes out the residual unannotated fallback literals and double-log-and-rethrow sludge in plugin-elevenlabs, and annotates the justified keeps. plugin-elevenlabs: - Stop fabricating an empty-string API key: getVoiceSettings/getTranscriptionSettings now report apiKey as `string | undefined` (honest "absent"), never `""`. The real call resolves the key via getElevenLabsClientConfig, which throws when required and absent; the settings field only drives the inline test presence checks (both `""` and `undefined` are falsy there — behavior-preserving). - Remove dead `?? "true"/"false"` on the boolean settings: the 3-arg getSetting overload always returns a string, so the coalescing was unreachable. - Collapse the double log-and-rethrow: the inner fetchSpeech/fetchTranscription helpers no longer catch-relog-rethrow the same error; failures propagate to the single model-slot boundary handler, which logs once and rethrows the original error (preserving stack/type), annotated J1. Never fabricates empty audio/text. - Annotate the untrusted-URL parse catch J3. plugin-embeddings: - Annotate the non-2xx error-body read J2 (context-adding; a body that is itself unreadable must not mask the HTTP error being thrown). plugin-edge-tts: already fail-fast (J6 teardown annotations landed in #12793); no change. Tests: extend the elevenlabs streaming suite with real failure-path cases — TTS/STT SDK rejection and empty/malformed provider response (null/undefined) must throw, never return fabricated empty audio or an empty transcript. 27→32 tests, all green. edge-tts 8/8, embeddings 27/27 unchanged. Closes #12797 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Validated the deterministic failure-path cleanup against fresh
Code review read: the changed ElevenLabs paths remove inner double-log/rethrow blocks, keep the model-slot boundary log, and add null/undefined provider-response assertions so failures throw instead of returning fabricated empty audio/text. Embeddings only adds the J2 annotation around non-2xx body-read context. I did not run live ElevenLabs audio/STT or a live embedding provider trajectory; this validation is limited to deterministic error-path behavior. |
2 participants
Closes #12797
Premise
#12797 (split from #12271) asks the embeddings/TTS provider group —
plugin-embeddings,plugin-elevenlabs,plugin-edge-tts— to throw or return an explicit failure instead of fabricating zero/marker vectors, fake/empty audio, or success-shaped defaults (extends #9324).Premise partly pre-resolved: the parent sweep #12793 (commit
e970e56135) already converted the fake-audio / zero-vector error paths in these three plugins —plugin-edge-ttsteardown was annotated J6,plugin-embeddingswas already fail-fast from #9324, and theplugin-elevenlabsstreaming test suite ("propagates SDK errors instead of returning empty bytes") was added. This PR closes out the residual unannotated fallback literals and double-log-and-rethrow sludge, adds the missing failure-path tests, and annotates the justified keeps.Inventory (fixed vs justified)
plugin-elevenlabs/src/index.ts — fixed
getApiKey(runtime) || "", 2 sites) →apiKey: string | undefined. Honest "absent" instead of a""that reads as "a key is present". The real call resolves the key viagetElevenLabsClientConfig(which throws when required and absent); the settings field only drives the inline test presence checks, where both""andundefinedare falsy — behavior-preserving.?? "true"/"false"on the three boolean settings → removed. The 3-arggetSetting(runtime, key, fallback)overload delegates toresolveSetting(..., { defaultValue }), which always returns a string, so the coalescing operand was unreachable.fetchSpeech/fetchTranscriptioncaught, reduced to message, re-logged, and rethrew the same error, then the model handlers logged it again. Removed the inner-helper catches so failures propagate to the single model-slot boundary handler, which logs once and rethrows the original error (preserving stack/type). Never fabricates empty audio/text.plugin-elevenlabs/src/index.ts — justified (annotated)
validateAudioUrlnew URL()parse catch →// error-policy:J3(untrusted-input sanitizing → explicit typed validation throw).// error-policy:J1(log once at the model edge, rethrow so the runtime planner loop surfaces the failure).plugin-embeddings/src/models/embedding.ts — justified (annotated)
.text().catch(() => "Unknown error")→// error-policy:J2(context-adding; an unreadable body must not mask the HTTP error being thrown). All real error paths already throw; the only synthetic return is the documented boot dimension-probe marker fornullinput — legitimate, unchanged.plugin-edge-tts — no change
Already fail-fast; both teardown catches carry
// error-policy:J6(landed in #12793). No fabricated audio.Verification
All commands run in the worktree against
origin/develop.Tests (real failure-path assertions, local SDK stubs):
New elevenlabs cases assert the throw (never a fabricated empty result) on: TTS SDK rejection, empty TTS stream body (
null/undefined), STT SDK rejection, empty STT response (null/undefined). Logs confirm the J1 boundary fires and the error propagates, e.g.[ElevenLabs] TRANSCRIPTION failed: Empty response from ElevenLabs STT API.Ratchet — no new slop, net neutral (two try/catch blocks removed):
Typecheck + lint (all touched plugins clean):
Evidence
developand remain covered by the existing suite.🤖 Generated with Claude Code