Skip to content

Commit

Permalink
修正: プリセットの一律 500 エラーを詳細化し 422 化して修正 (#1162)
Browse files Browse the repository at this point in the history 
* fix: プリセット関連エラーを詳細化し500バグを修正

* fix: lint

* fix: lint

* Update voicevox_engine/preset/PresetError.py

Co-authored-by: Hiroshiba <[email protected]>

* fix: `PresetInternalError` docstring をドメイン内の事象に関する記述へ修正

* fix: `PresetInternalError` docstring を明確化

---------

Co-authored-by: Hiroshiba <[email protected]>
  • Loading branch information
@tarepan @Hiroshiba
tarepan and Hiroshiba authored May 5, 2024
1 parent 7ca6756 commit 5fcd68b
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 31 deletions.
32 changes: 19 additions & 13 deletions test/preset/test_preset.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from unittest import TestCase

from voicevox_engine.preset.Preset import Preset
from voicevox_engine.preset.PresetError import PresetError
from voicevox_engine.preset.PresetError import PresetInputError, PresetInternalError
from voicevox_engine.preset.PresetManager import PresetManager

presets_test_1_yaml_path = Path("test/preset/presets-test-1.yaml")
Expand Down Expand Up @@ -37,26 +37,28 @@ def test_validation_same(self) -> None:
def test_validation_2(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.load_presets()

def test_preset_id(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_3_yaml_path)
with self.assertRaises(PresetError, msg="プリセットのidに重複があります"):
with self.assertRaises(
PresetInternalError, msg="プリセットのidに重複があります"
):
preset_manager.load_presets()

def test_empty_file(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_4_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルが空の内容です"
PresetInternalError, msg="プリセットの設定ファイルが空の内容です"
):
preset_manager.load_presets()

def test_not_exist_file(self) -> None:
preset_manager = PresetManager(preset_path=Path("test/presets-dummy.yaml"))
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルが見つかりません"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.load_presets()

Expand Down Expand Up @@ -89,7 +91,7 @@ def test_add_preset(self) -> None:
def test_add_preset_load_failure(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.add_preset(
Preset(
Expand Down Expand Up @@ -182,7 +184,7 @@ def test_add_preset_write_failure(self) -> None:
preset_manager._refresh_cache = lambda: None # type:ignore[method-assign]
preset_manager.preset_path = "" # type: ignore[assignment]
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルに書き込み失敗しました"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.add_preset(preset)
self.assertEqual(len(preset_manager.presets), 2)
Expand Down Expand Up @@ -217,7 +219,7 @@ def test_update_preset(self) -> None:
def test_update_preset_load_failure(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.update_preset(
Preset(
Expand Down Expand Up @@ -254,7 +256,9 @@ def test_update_preset_not_found(self) -> None:
"postPhonemeLength": 0.1,
}
)
with self.assertRaises(PresetError, msg="更新先のプリセットが存在しません"):
with self.assertRaises(
PresetInputError, msg="更新先のプリセットが存在しません"
):
preset_manager.update_preset(preset)
self.assertEqual(len(preset_manager.presets), 2)
remove(temp_path)
Expand All @@ -281,7 +285,7 @@ def test_update_preset_write_failure(self) -> None:
preset_manager._refresh_cache = lambda: None # type:ignore[method-assign]
preset_manager.preset_path = "" # type: ignore[assignment]
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルに書き込み失敗しました"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.update_preset(preset)
self.assertEqual(len(preset_manager.presets), 2)
Expand All @@ -300,15 +304,17 @@ def test_delete_preset(self) -> None:
def test_delete_preset_load_failure(self) -> None:
preset_manager = PresetManager(preset_path=presets_test_2_yaml_path)
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルにミスがあります"
PresetInternalError, msg="プリセットの設定ファイルにミスがあります"
):
preset_manager.delete_preset(10)

def test_delete_preset_not_found(self) -> None:
temp_path = self.tmp_dir_path / "presets-test-temp.yaml"
copyfile(presets_test_1_yaml_path, temp_path)
preset_manager = PresetManager(preset_path=temp_path)
with self.assertRaises(PresetError, msg="削除対象のプリセットが存在しません"):
with self.assertRaises(
PresetInputError, msg="削除対象のプリセットが存在しません"
):
preset_manager.delete_preset(10)
self.assertEqual(len(preset_manager.presets), 2)
remove(temp_path)
Expand All @@ -321,7 +327,7 @@ def test_delete_preset_write_failure(self) -> None:
preset_manager._refresh_cache = lambda: None # type:ignore[method-assign]
preset_manager.preset_path = "" # type: ignore[assignment]
with self.assertRaises(
PresetError, msg="プリセットの設定ファイルに書き込み失敗しました"
PresetInternalError, msg="プリセットの設定ファイルが見つかりません"
):
preset_manager.delete_preset(1)
self.assertEqual(len(preset_manager.presets), 2)
Expand Down
18 changes: 13 additions & 5 deletions voicevox_engine/app/routers/preset.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from fastapi import APIRouter, Body, Depends, HTTPException, Query, Response

from voicevox_engine.preset.Preset import Preset
from voicevox_engine.preset.PresetError import PresetError
from voicevox_engine.preset.PresetError import PresetInputError, PresetInternalError
from voicevox_engine.preset.PresetManager import PresetManager

from ..dependencies import check_disabled_mutable_api
Expand All @@ -27,8 +27,10 @@ def get_presets() -> list[Preset]:
"""
try:
presets = preset_manager.load_presets()
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return presets

@router.post(
Expand All @@ -51,8 +53,10 @@ def add_preset(
"""
try:
id = preset_manager.add_preset(preset)
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return id

@router.post(
Expand All @@ -75,8 +79,10 @@ def update_preset(
"""
try:
id = preset_manager.update_preset(preset)
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return id

@router.post(
Expand All @@ -93,8 +99,10 @@ def delete_preset(
"""
try:
preset_manager.delete_preset(id)
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
return Response(status_code=204)

return router
6 changes: 4 additions & 2 deletions voicevox_engine/app/routers/tts_pipeline.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
ParseKanaError,
Score,
)
from voicevox_engine.preset.PresetError import PresetError
from voicevox_engine.preset.PresetError import PresetInputError, PresetInternalError
from voicevox_engine.preset.PresetManager import PresetManager
from voicevox_engine.tts_pipeline.kana_converter import create_kana, parse_kana
from voicevox_engine.tts_pipeline.tts_engine import TTSEngine
Expand Down Expand Up @@ -88,8 +88,10 @@ def audio_query_from_preset(
core = get_core(core_version)
try:
presets = preset_manager.load_presets()
except PresetError as err:
except PresetInputError as err:
raise HTTPException(status_code=422, detail=str(err))
except PresetInternalError as err:
raise HTTPException(status_code=500, detail=str(err))
for preset in presets:
if preset.id == preset_id:
selected_preset = preset
Expand Down
13 changes: 12 additions & 1 deletion voicevox_engine/preset/PresetError.py
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1,13 @@
class PresetError(Exception):
"""プリセットに関するエラー"""


class PresetInputError(Exception):
"""受け入れ不可能な入力値に起因するエラー"""

pass


class PresetInternalError(Exception):
"""プリセットマネージャーに起因するエラー"""

pass
20 changes: 10 additions & 10 deletions voicevox_engine/preset/PresetManager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from pydantic import ValidationError, parse_obj_as

from .Preset import Preset
from .PresetError import PresetError
from .PresetError import PresetInputError, PresetInternalError


class PresetManager:
Expand All @@ -32,23 +32,23 @@ def _refresh_cache(self) -> None:
# 更新無し
return
except OSError:
raise PresetError("プリセットの設定ファイルが見つかりません")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")

# データベースの読み込み
with open(self.preset_path, mode="r", encoding="utf-8") as f:
obj = yaml.safe_load(f)
if obj is None:
raise PresetError("プリセットの設定ファイルが空の内容です")
raise PresetInternalError("プリセットの設定ファイルが空の内容です")
try:
_presets = parse_obj_as(List[Preset], obj)
except ValidationError:
raise PresetError("プリセットの設定ファイルにミスがあります")
raise PresetInternalError("プリセットの設定ファイルにミスがあります")

# 全idの一意性をバリデーション
if len([preset.id for preset in _presets]) != len(
{preset.id for preset in _presets}
):
raise PresetError("プリセットのidに重複があります")
raise PresetInternalError("プリセットのidに重複があります")

# キャッシュを更新する
self.presets = _presets
Expand All @@ -72,7 +72,7 @@ def add_preset(self, preset: Preset) -> int:
except Exception as err:
self.presets.pop()
if isinstance(err, FileNotFoundError):
raise PresetError("プリセットの設定ファイルに書き込み失敗しました")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")
else:
raise err

Expand Down Expand Up @@ -100,15 +100,15 @@ def update_preset(self, preset: Preset) -> int:
self.presets[i] = preset
break
else:
raise PresetError("更新先のプリセットが存在しません")
raise PresetInputError("更新先のプリセットが存在しません")

# 変更の反映。失敗時はリバート。
try:
self._write_on_file()
except Exception as err:
self.presets[prev_preset[0]] = prev_preset[1]
if isinstance(err, FileNotFoundError):
raise PresetError("プリセットの設定ファイルに書き込み失敗しました")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")
else:
raise err

Expand All @@ -129,14 +129,14 @@ def delete_preset(self, id: int) -> int:
buf_index = i
break
else:
raise PresetError("削除対象のプリセットが存在しません")
raise PresetInputError("削除対象のプリセットが存在しません")

# 変更の反映。失敗時はリバート。
try:
self._write_on_file()
except FileNotFoundError:
self.presets.insert(buf_index, buf)
raise PresetError("プリセットの設定ファイルに書き込み失敗しました")
raise PresetInternalError("プリセットの設定ファイルが見つかりません")

return id

Expand Down

0 comments on commit 5fcd68b

Please sign in to comment.