fix(tts): fall through to raw import when lazy_deps fails (#53259) - #67314
fix(tts): fall through to raw import when lazy_deps fails (#53259)#67314tusharui wants to merge 2 commits into
Conversation
…ch#53259) Replace aise ImportError(str(e)) with pass in the except Exception handler of _import_edge_tts(), _import_elevenlabs(), and _import_mistral_client() so packages installed via PYTHONPATH or Docker layered filesystems still work when lazy_deps.ensure() raises. Also fix the Mistral STT path in transcription_tools.py which only caught ImportError, not FeatureUnavailable. Adds 6 regression tests using sys.modules fixtures (no builtins.__import__ patching).
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused fallback fix. The premise remains present on upstream main: tools/tts_tool.py:93-95, 114-116, and 137-140 stop before raw imports when lazy installation raises, and tools/transcription_tools.py:1433-1438 only falls through on ImportError.
Problems
- The changed STT branch at
tools/transcription_tools.py:1436has no regression coverage in this PR.tests/tools/test_tts_pythonpath_fallback.pyexercises only the three TTS_import_*helpers. - As the member comment notes, this duplicates active canonical PR #53289. Its verified diff covers the same TTS and Mistral STT paths and includes an STT fallback regression.
Suggested changes
- Add an isolated
_transcribe_mistral()test whereensure("stt.mistral")raisesFeatureUnavailablebut the rawmistralai.client.Mistralimport succeeds. - Reconcile any salvage with #53289 so the canonical implementation and its STT coverage are retained.
Automated hermes-sweeper review.
| from tools.lazy_deps import ensure as _lazy_ensure | ||
| _lazy_ensure("stt.mistral", prompt=False) | ||
| except ImportError: | ||
| except Exception: |
There was a problem hiding this comment.
Please add a regression test for this fall-through. The added test file exercises only the three TTS _import_* helpers, so _transcribe_mistral() is not covered when ensure("stt.mistral") raises but a raw mistralai import is available.
teknium1
added
sweeper:risk-compatibility
…esearch#53259) Add isolated test where ensure('stt.mistral') raises FeatureUnavailable but the raw mistralai.client.Mistral import succeeds, verifying the transcription_tools.py fallthrough path introduced in the same PR.
|
Merged into main via consolidated salvage PR #73510 (merge Your contribution is credited to you in git history. Thank you! Closing this PR as merged-via-salvage. |
3 participants
Problem
When TTS packages (edge-tts, elevenlabs, mistralai) are installed outside the venv but importable on sys.path (e.g. via PYTHONPATH, Docker layered filesystems), the TTS tool fails because
lazy_deps.ensure()failures are re-raised asImportErrorbefore the raw import line runs.Fix
Replace
raise ImportError(str(e))withpassin theexcept Exceptionhandler of all three lazy-import helpers, allowing execution to fall through to the raw import. Also fix the Mistral STT path intranscription_tools.pywhich only caughtImportError, notFeatureUnavailable.Files changed
tools/tts_tool.py— 3 one-line changes (lines 93, 114, 137)tools/transcription_tools.py— 1 one-line change (line 1436)tests/tools/test_tts_pythonpath_fallback.py— new test file (6 tests)Tests
Uses
sys.modulesfixtures (notbuiltins.__import__patching) per reviewer feedback. Covers both fallback-on-FeatureUnavailable and clean-ImportError-when-truly-missing paths for all three providers.Fixes #53259