From 9c688aeba066f3090f6d4950c7512e3cdb083c03 Mon Sep 17 00:00:00 2001 From: Clodagh Walsh <42538837+clodaghwalsh17@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:42:34 +0100 Subject: [PATCH] [TTS] Improve voice delete error message (#7795) Signed-off-by: Clodagh Walsh --- .../openai_api/test_serving_speech.py | 57 ++++++++++++++++--- vllm_omni/entrypoints/openai/api_server.py | 24 +++++--- vllm_omni/entrypoints/openai/errors.py | 10 ++++ .../entrypoints/openai/serving_speech.py | 16 +++--- 4 files changed, 84 insertions(+), 23 deletions(-) diff --git a/tests/entrypoints/openai_api/test_serving_speech.py b/tests/entrypoints/openai_api/test_serving_speech.py index 46291a88a..fb2e9839d 100644 --- a/tests/entrypoints/openai_api/test_serving_speech.py +++ b/tests/entrypoints/openai_api/test_serving_speech.py @@ -38,6 +38,7 @@ from vllm_omni.entrypoints.omni_base import OmniEngineDeadError from vllm_omni.entrypoints.openai import api_server as api_server_module from vllm_omni.entrypoints.openai import serving_speech as serving_speech_module from vllm_omni.entrypoints.openai.audio_utils_mixin import AudioMixin +from vllm_omni.entrypoints.openai.errors import InvalidPresetVoiceReferenceError, InvalidVoiceReferenceError from vllm_omni.entrypoints.openai.protocol.audio import ( BatchSpeechRequest, CreateAudio, @@ -331,10 +332,12 @@ def test_app(mocker: MockerFixture, tmp_path, monkeypatch): # Add delete_voice endpoint async def delete_voice(name: str): try: - success = await speech_server.delete_voice(name) - if not success: - raise HTTPException(status_code=404, detail=f"Voice '{name}' not found") + await speech_server.delete_voice(name) return {"success": True, "message": f"Voice '{name}' deleted successfully"} + except InvalidPresetVoiceReferenceError as e: + raise HTTPException(status_code=403, detail=str(e)) + except InvalidVoiceReferenceError as e: + raise HTTPException(status_code=404, detail=str(e)) except HTTPException: raise except ValueError as e: @@ -593,7 +596,8 @@ class TestSpeechAPI: def test_upload_voice_with_speaker_description(self, client, tmp_path): """Test voice upload with speaker_description stores and returns the description.""" # Pre-cleanup in case a previous test run left this voice behind - client.delete("/v1/audio/voices/test_voice_vd") + response = client.delete("/v1/audio/voices/test_voice_vd") + assert response.status_code == 404 or response.status_code == 200 audio_content = b"fake audio content" * 1000 files = {"audio_sample": ("test.wav", audio_content, "audio/wav")} @@ -611,7 +615,9 @@ class TestSpeechAPI: def test_upload_voice_speaker_description_in_listing(self, client): """Test that speaker_description survives the upload → list round-trip.""" - client.delete("/v1/audio/voices/test_voice_sd_list") + # Pre-cleanup in case a previous test run left this voice behind + response = client.delete("/v1/audio/voices/test_voice_sd_list") + assert response.status_code == 404 or response.status_code == 200 audio_content = b"fake audio content" * 1000 files = {"audio_sample": ("test.wav", audio_content, "audio/wav")} @@ -4078,7 +4084,7 @@ def test_voice_routes_without_tokenization(mocker: MockerFixture, method: str, h if handler is not None: handler._get_available_voices.return_value = [] handler.uploaded_speakers = {} - handler.delete_voice = mocker.AsyncMock(return_value=False) + handler.delete_voice = mocker.AsyncMock(side_effect=InvalidVoiceReferenceError("Voice 'missing' not found")) app = _make_api_server_request(handler).app app.add_api_route("/v1/audio/voices", api_server_module.list_voices, methods=["GET"]) app.add_api_route("/v1/audio/voices", api_server_module.upload_voice, methods=["POST"]) @@ -4223,7 +4229,7 @@ def test_api_server_delete_voice_value_error_returns_400(mocker: MockerFixture): def test_api_server_delete_voice_not_found_returns_404(mocker: MockerFixture): handler = mocker.MagicMock() - handler.delete_voice = mocker.AsyncMock(return_value=False) + handler.delete_voice = mocker.AsyncMock(side_effect=InvalidVoiceReferenceError("Voice 'missing' not found")) raw_request = _make_api_server_request(handler, method="DELETE", path="/v1/audio/voices/missing") response = asyncio.run(api_server_module.delete_voice("missing", raw_request)) @@ -4236,6 +4242,43 @@ def test_api_server_delete_voice_not_found_returns_404(mocker: MockerFixture): ) +def test_api_server_delete_built_in_voice_returns_403(mocker: MockerFixture): + handler = mocker.MagicMock() + handler.uploaded_speakers = set() + handler._get_available_speakers = mocker.Mock(return_value=set("built-in")) + handler.delete_voice = mocker.AsyncMock( + side_effect=InvalidPresetVoiceReferenceError("Cannot delete built-in voice 'built-in'") + ) + raw_request = _make_api_server_request(handler, method="DELETE", path="/v1/audio/voices/built-in") + + response = asyncio.run(api_server_module.delete_voice("built-in", raw_request)) + + _assert_openai_error_response( + response, + status_code=403, + message="Cannot delete built-in voice 'built-in'", + err_type="ForbiddenError", + ) + + +def test_api_server_delete_default_voice_returns_403(mocker: MockerFixture): + handler = mocker.MagicMock() + handler.uploaded_speakers = set() + handler.delete_voice = mocker.AsyncMock( + side_effect=InvalidPresetVoiceReferenceError("Cannot delete built-in voice 'default'") + ) + raw_request = _make_api_server_request(handler, method="DELETE", path="/v1/audio/voices/default") + + response = asyncio.run(api_server_module.delete_voice("default", raw_request)) + + _assert_openai_error_response( + response, + status_code=403, + message="Cannot delete built-in voice 'default'", + err_type="ForbiddenError", + ) + + def test_api_server_delete_voice_exception_returns_500(mocker: MockerFixture): handler = mocker.MagicMock() handler.delete_voice = mocker.AsyncMock(side_effect=RuntimeError("disk failed")) diff --git a/vllm_omni/entrypoints/openai/api_server.py b/vllm_omni/entrypoints/openai/api_server.py index a1698e379..59c3e939b 100644 --- a/vllm_omni/entrypoints/openai/api_server.py +++ b/vllm_omni/entrypoints/openai/api_server.py @@ -119,6 +119,8 @@ from vllm_omni.entrypoints.openai.diffusion import ( apply_stage_default_sampling_params, ) from vllm_omni.entrypoints.openai.errors import ( + InvalidPresetVoiceReferenceError, + InvalidVoiceReferenceError, _create_speech_error_json_response, _error_response_to_json_response, ) @@ -1672,16 +1674,20 @@ async def delete_voice(name: str, raw_request: Request): try: # Delete the voice - success = await handler.delete_voice(name) - if not success: - return _create_speech_error_json_response( - f"Voice '{name}' not found", - err_type="NotFoundError", - status_code=HTTPStatus.NOT_FOUND, - ) - + await handler.delete_voice(name) return JSONResponse(content={"success": True, "message": f"Voice '{name}' deleted successfully"}) - + except InvalidPresetVoiceReferenceError as e: + return _create_speech_error_json_response( + str(e), + err_type="ForbiddenError", + status_code=HTTPStatus.FORBIDDEN, + ) + except InvalidVoiceReferenceError as e: + return _create_speech_error_json_response( + str(e), + err_type="NotFoundError", + status_code=HTTPStatus.NOT_FOUND, + ) except ValueError as e: return _create_speech_error_json_response(str(e)) except Exception as e: diff --git a/vllm_omni/entrypoints/openai/errors.py b/vllm_omni/entrypoints/openai/errors.py index 612f620a4..c8716daa2 100644 --- a/vllm_omni/entrypoints/openai/errors.py +++ b/vllm_omni/entrypoints/openai/errors.py @@ -19,6 +19,16 @@ class InvalidInputReferenceError(ValueError): super().__init__(message) +class InvalidVoiceReferenceError(ValueError): + def __init__(self, message: str = "Reference to non existent voice.") -> None: + super().__init__(message) + + +class InvalidPresetVoiceReferenceError(InvalidVoiceReferenceError): + def __init__(self, message: str = "Invalid reference to built-in voice.") -> None: + super().__init__(message) + + def _error_response_to_json_response( err: ErrorResponse, *, diff --git a/vllm_omni/entrypoints/openai/serving_speech.py b/vllm_omni/entrypoints/openai/serving_speech.py index 40ae2f911..bb27e6fd6 100644 --- a/vllm_omni/entrypoints/openai/serving_speech.py +++ b/vllm_omni/entrypoints/openai/serving_speech.py @@ -38,6 +38,7 @@ from vllm.v1.engine.exceptions import EngineDeadError, EngineGenerateError from vllm_omni.config.stage_config import StagePipelineConfig from vllm_omni.entrypoints.openai.audio_utils_mixin import AudioMixin, StreamingAudioResampler +from vllm_omni.entrypoints.openai.errors import InvalidPresetVoiceReferenceError, InvalidVoiceReferenceError from vllm_omni.entrypoints.openai.protocol.audio import ( AudioResponse, BatchSpeechRequest, @@ -1095,22 +1096,24 @@ class OmniOpenAIServingSpeech(OpenAIServing, AudioMixin): "embedding_dim": emb_dim, } - async def delete_voice(self, name: str) -> bool: + async def delete_voice(self, name: str): """ Delete an uploaded voice. Args: name: Voice name to delete - - Returns: - bool: True if successful, False if voice doesn't exist """ async with self._upload_lock: voice_name_lower = name.lower() + built_in_speakers = self._get_available_voices() - set(self.uploaded_speakers) + + if voice_name_lower in built_in_speakers: + err = f"Cannot delete built-in voice '{name}'" + raise InvalidPresetVoiceReferenceError(err) if voice_name_lower not in self.uploaded_speakers: - logger.warning("Voice '%s' not found", name) - return False + err = f"Voice '{name}' not found" + raise InvalidVoiceReferenceError(err) speaker_info = self.uploaded_speakers.pop(voice_name_lower) self._ref_audio_data_url_cache.pop(voice_name_lower, None) @@ -1125,7 +1128,6 @@ class OmniOpenAIServingSpeech(OpenAIServing, AudioMixin): self._speaker_cache.clear(voice_name_lower) logger.info("Deleted voice '%s'", name) - return True def _is_tts_model(self) -> bool: """Check if the current model is a supported TTS model."""