Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

特定のエンジンを選択した際にエンジンリストが消失する #1168

Closed
1 of 3 tasks
nmori opened this issue Jan 31, 2023 · 18 comments
Closed
1 of 3 tasks

Comments

@nmori
Copy link
Contributor

nmori commented Jan 31, 2023

不具合の内容

・特定のエンジンにおいて、エンジン詳細を参照したタイミングで、全エンジンの表示が消失する

現象・ログ

[01:45:22.037] [error] TypeError: Cannot convert undefined or null to object
    at Function.entries (<anonymous>)
    at fn (app://./js/webpack:/src/components/EngineManageDialog.vue:266:53)
    at e (app://./js/webpack:/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js:852:19)
    at hSlot (app://./js/webpack:/node_modules/quasar/dist/quasar.esm.prod.js:6:26585)
    at Proxy.call (app://./js/webpack:/node_modules/quasar/dist/quasar.esm.prod.js:6:304545)
    at renderComponentRoot (app://./js/webpack:/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js:898:44)
    at w.fn (app://./js/webpack:/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js:5702:34)
    at w.run (app://./js/webpack:/node_modules/@vue/reactivity/dist/reactivity.esm-bundler.js:187:25)
    at fn (app://./js/webpack:/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js:5745:56)
    at lt (app://./js/webpack:/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js:157:36)

再現手順

・フォルダ指定でSHAREVOX 0.1.7を追加
・再起動する
・エンジンの管理からSHAREVOXを選択

期待動作

・リストは消失せず、エンジンの詳細が表示される
・エンジン側の問題であれば、警告がでるか例外処理が行われる

VOICEVOXのバージョン

0.14.1

OSの種類/ディストリ/バージョン

v0.14.1

  • Windows
  • macOS
  • Linux

Windows 10 Pro 64bit 22H2 (19045.2486)

その他

image

@nmori nmori added the バグ label Jan 31, 2023
@sabonerune
Copy link
Contributor

確かSHAREVOX 0.1.7が提供する/engine_manifestエンドポイントの仕様がVOICEVOX 0.14.0の仕様と異なるため発生するものだった記憶があります。

@y-chan
Copy link
Member

y-chan commented Feb 1, 2023

@sabonerune さんのおっしゃる通りの問題だと思います。
マルチエンジン機能に対応したバージョンのエンジンを指定しなければならないです。
SHAREVOX 0.1.7はマルチエンジン機能に未対応ですので、このような問題が発生します。(もし、SHAREVOXにおいてマルチエンジン機能を利用したい場合は0.2.0-preview.6のvvppをご利用ください)

ただ、エンジンを追加する前に、engine_manifest.jsonのバリデーションなどで、そもそもこのような未対応エンジンを追加できてしまう問題に対応できる可能性があるのでIssueはクローズせず留めておこうかなと思います。

@Hiroshiba
Copy link
Member

Hiroshiba commented Feb 1, 2023

かなり古いエンジンを試した場合でもちゃんと稼働するのがまあベストで、であればengine_manifest.jsonは存在しないかもしれませんね・・・。

エラーが起きても落ちない形にできれば一番良いのかなと思いました!!

@nmori さん、もしよければ挑戦してみませんか・・・・・・?
というのも、naoさん自身がアプリケーションを開発・リリースされていて、UX周りに明るそうな印象を持っています。
VOICEVOXはユーザー目線を大事にしているので、そこもできてプログラミングもできる方と一緒にプロダクトを開発して切磋琢磨したいなと思ったのでお声掛けしてみた次第です!

たた結構入り組んでいる先(Vuex storeという概念の中)なので、大変な箇所かもしれません。
もしわからない点あればdiscordで気軽に聞いて頂ければという感じです。
もしよければ・・・!!

@nmori
Copy link
Contributor Author

nmori commented Feb 3, 2023

@Hiroshiba さん、お声がけ頂きありがとうございます。
フロントエンドの構築は苦手な技術領域ではあるんですが、
習得していきたい領域でもあります。

流儀や作法をこれから身に着ける中での作業になるので
どこまでフィードバックできるかわかりませんが、
せっかく想いを寄せて頂いたので チャレンジしたいとおもいます。
(まずは開発できる環境と周辺環境の読みほどきから始めます)

@Hiroshiba
Copy link
Member

Hiroshiba commented Feb 3, 2023

ありがとうございます!!
なにかあればこちらでもDiscordでも気軽に聞いて頂ければと思います!!

@nmori
Copy link
Contributor Author

nmori commented Feb 3, 2023

なんとなくの流れを理解しました。

エンジンの素性を /engine_manifest に問い合わせているが
未対応エンジンでは json に supported_features の項目がないから undefined or null になって、
表示を作る際に例外が発生してテンプレート処理が中断する、という流れですね。

単純にエラーが出ないようにするなら、
EngineManageDialog.vueのSupportedFeaturesFromJSONTypedで
未定義の場合の処理(規定値)をきめれば対応できそうですが、
「そもそも定義がない時点でおかしい」とするなら、登録を拒否する方法もありますね。

設計方針としては、どちらがよいでしょう?
(現時点で登録されてしまっているケースがあるから、実は両方必要…!?)

それから、今後 SupportedFeatures の定義が増減が想定される場合、
設定の一部が見当たらない場合の挙動も 考えておく必要はありそうですね

@Hiroshiba
Copy link
Member

調査ありがとうございます!!

未対応エンジンでは json に supported_features の項目がないから undefined or null になって、

あーーーなるほどです!!

設計方針としては、どちらがよいでしょう?

難しい判断ですね・・・。
「登録を拒否する」はやりたい、「既定値」もやっておきたい・・・という感じになるかなと思いました!
既定値は全部falseにしちゃって「未対応エンジンです」みたいな案内を出すと良さそう・・・・・・・・・?

SupportedFeaturesが変わった場合(特に増えた場合)は、互換性を考えて値をoptionalにして「値がない=今までと変わらない」となるように設定パラメータの意味を調整したいと思います!

@nmori
Copy link
Contributor Author

nmori commented Feb 4, 2023

もう少しシンプルに考えました。

まず、supportedFeatures がないときは
<span v-else>(取得に失敗しました)</span> に飛ぶようにして
処理をしてしまう方が、不正確な情報を返すよりよさそうです。
(これで、Issueで報告した表面的な問題は解決します)

試した感じでは、 <ul v-if="engineManifests[selectedId].supportedFeatures"> と
書き換えることで実現できそうです(言語に自信はないけど、動作としてはこれでいけそう)

そのうえで、現在フォルダを指定して追加する場合に
ADD_ENGINE_DIR で、未検証なままリストに追加しているようなので、
追加可能かを判断する工程を1つ挟むのがよさそうです。

(このとき、エンジンはまだ起動していないので、
 直接フォルダ内のjsonをパースして判断する感じなのかな?とイメージしています)

この場合、指定されたフォルダのどの情報を参照すると
対応しているエンジンと判断できるか、ヒントを頂けると嬉しいです

@Hiroshiba
Copy link
Member

完璧だと思います!!

今確認したところ、ちょうどengine_manifest.jsonをvalidateしている場所がありました。

if (
["name", "uuid", "port", "command", "icon"].some(
(key) => !(key in manifestContent)
)
) {
return "invalidManifest";
}

ここでjsonにsupported_featuresもあるかをチェックするのがまさにそうなのかなと!!

@sevenc-nanashi
Copy link
Member

ここ、minimumEngineManifestのzodで変えるべきだと思ってたので一緒に変えちゃってもいいかも

nmori added a commit to nmori/voicevox that referenced this issue Feb 5, 2023
・追加されたエンジンが未対応である場合には追加を阻止
・追加されてしまっている場合には、エラーで処理中断しないように
@nmori
Copy link
Contributor Author

nmori commented Feb 5, 2023

ん… CIチェックでひっかかりましたか
(言語仕様をもう少し理解したほうがいいのかもしれない、すみませぬ)
もうちょい見直してみます。

(ちなみに、プルリク提出前にCIをやる手段はあるでしょうか?)

@y-chan
Copy link
Member

y-chan commented Feb 5, 2023

CI自体の実行はできませんが、CI時にテストしているものが以下なので、それぞれを実行して問題なく通れば(エラーを解消できれば)CIも通るはずです!

npm run typecheck
npm run lint
# lint でおかしかったものはfmtで修正できるので、lintがエラーを吐いたら実行する
npm run fmt
# ビルドテスト
npm run build:build_pnever
# typos(typos-cli)の導入が必要
typos

@sevenc-nanashi
Copy link
Member

sevenc-nanashi commented Feb 5, 2023

npm run lintでLinterが、npm run typecheckで型チェック走ります。自分はエディタにESLintのプラグインを入れて即時に出るようにしてます。

@sevenc-nanashi
Copy link
Member

image
image

型チェック、Linterに怒られている例。(Neovim)

@nmori
Copy link
Contributor Author

nmori commented Feb 5, 2023

色々教えて頂き、ありがとうございます。

まずは、lint の実行および ESlint の導入で、
エラーを事前に認知できるようになりました。

@Hiroshiba
Copy link
Member

VSCodeでしたら、拡張機能にpritter・ESLint・Volarを入れつつ、VSCodeの機能で「保存時にフォーマットする」をONにすれば大体うまくいくと思います・・・!

この辺りwebフロント初学者向け情報としてREADMEにあると良いかもと思いました。
もし他に詰まったところとかあれば参考にお伺いできると嬉しいかもです・・・!!

@nmori
Copy link
Contributor Author

nmori commented Feb 5, 2023

ページにある「開発の始め方」をみてVOICEVOXのテスト環境を
動かすところまでは出来たので、始める点は問題ないかと思います。

今回ひっかかった点は、「事前のコードチェック」ぐらいかとおもいます。

初学者向けに情報を頂けるのだとしたら、
・コード提出前の推奨手順
・Issue→PRまでの手順
・言語や技術に関する部分のヒント(どのLvの初学者と仮定するか次第ではありますが)
あたりの情報があると助かるなぁと思いました。

#もしかしたら、ちょっと時間が取れたときに
#貢献者向けドキュメントを書いてPRしてみる手もありですか?

@Hiroshiba
Copy link
Member

OSSでたまに見かけるcontributing guide(貢献者ガイドライン)ですね!!
作って頂けるととても助かります!!! docsディレクトリに良い感じに作ってみて頂けると!!!

@nmori nmori closed this as completed Feb 18, 2023
Hiroshiba added a commit that referenced this issue Feb 13, 2024
* 未対応エンジン追加時にリストが消える件(#1168)
・追加されたエンジンが未対応である場合には追加を阻止
・追加されてしまっている場合には、エラーで処理中断しないように

* lintチェックエラー部分の修正

* コードレビューの反映 (ref #1179)
・MinimumEngineManifestの更新

* コードレビュー分の反映② ref #1179
・engineManifests[selectedId]自体が undefined であるケースに対応

* サードパーティがエンジンへのアクセス情報を得るための設定書き出し機能(ref #1738)

* ファイルは runtime-info.json に書き出し
* エンジン全起動もしくは個別起動/終了のタイミングで更新

* * 関数名の変更 : writeEngineInfoFor3rdParty
* 排他ロックの追加
* 処理の非同期化

* * コンストラクタ引数でファイルパスを渡すように
* 関数をシンプルに
* ログメッセージ修正
* コメント位置修正

* * エクスポートファイパスを渡す所を引数にした
* 変数、関数名修正
* いくつかの構造をクラス化

* 議論 #1738 に基づき、最小項目の書き出しに変更

* * ファイル書き出しクラスに機能を集約
* 変数名、コメントの修正

* RuntimeInfoManager.tsをブラッシュアップ

* EngineManagerとRuntimeInfoManagerを疎結合に

* データ構造調整、テスト追加

* Apply suggestions from code review

---------

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Hiroshiba added a commit that referenced this issue Mar 6, 2024
* 未対応エンジン追加時にリストが消える件(#1168)
・追加されたエンジンが未対応である場合には追加を阻止
・追加されてしまっている場合には、エラーで処理中断しないように

* lintチェックエラー部分の修正

* コードレビューの反映 (ref #1179)
・MinimumEngineManifestの更新

* コードレビュー分の反映② ref #1179
・engineManifests[selectedId]自体が undefined であるケースに対応

* ref #1738 の会話にあった「情報ファイルに関するドキュメント」(新規執筆)

* markdown lint でエラーが出てた件の修正

* Update docs/サードパーティ開発者の方へ.md

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

* フォーマット表記提案サジェストの適用

Co-authored-by: Nanashi. <sevenc7c@sevenc7c.com>

* * 表の表記をJSONP内コメント追記に
* VOICEVOXの仕組みを追記

* 環境変数の修正

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

* コメントいただいた部分を中心に追記

* Update docs/サードパーティ開発者の方へ.md

---------

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Co-authored-by: Nanashi. <sevenc7c@sevenc7c.com>
Hiroshiba added a commit that referenced this issue Jun 16, 2024
* 未対応エンジン追加時にリストが消える件(#1168)
・追加されたエンジンが未対応である場合には追加を阻止
・追加されてしまっている場合には、エラーで処理中断しないように

* lintチェックエラー部分の修正

* コードレビューの反映 (ref #1179)
・MinimumEngineManifestの更新

* コードレビュー分の反映② ref #1179
・engineManifests[selectedId]自体が undefined であるケースに対応

* 貢献者ガイドラインを明文化 (ref #1190)

* レビュー結果の反映①

* 着手周りの手順追記

* CONTRIBUTING.md として配置変更

* markdownlint のエラーを修正

* * ローカル実行時の markdownlint 検索範囲を修正

* Issueを閉じるタイミングを追記

* * ドラフトプルリクエストについての追記
* フォーマットの修正

* * プルリクエストの表記を英語に。
* WIPに付いてのトーンを弱めに。
* リンク切れの修正
* 「その他」の 追記

* * レビュー内容の反映

* * e2e部分の追記
* インデント修正

* 提案いただいた分のコミットと追記

* ・査読分の反映
・README.mdに誘導リンクを追加

* Apply suggestions from code review

* フォーマットを整える

* 崩れてしまった部分を戻す

* こう?

* なぜか * に戻っていた

* pythonはコメントアウトが // ではなかった

---------

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants