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.

Sign up for GitHub

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

fix: config.json の validation 失敗時にわかりやすいログを出すように #1222

Merged
merged 5 commits into from
Feb 23, 2023
Merged

fix: config.json の validation 失敗時にわかりやすいログを出すように #1222

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4b17c2
fix: config.json の validation 失敗時にわかりやすいログを出すように
raa0121 Feb 22, 2023
60e6867
catch 後にprocess.exit(1)するようにし、ログの文章を提案されたものに
raa0121 Feb 22, 2023
8f36e1e
fmt
raa0121 Feb 22, 2023
bdbc440
log.error から ダイアログに変更して、process.exit から 上位へ throw するように
raa0121 Feb 23, 2023
ff5d5ac
app.exit してから throw するように
raa0121 Feb 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 57 additions & 46 deletions src/background.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,55 +125,66 @@ const electronStoreJsonSchema = zodToJsonSchema(electronStoreSchema);
if (!("properties" in electronStoreJsonSchema)) {
throw new Error("electronStoreJsonSchema must be object");
}
const store = new Store<ElectronStoreType>({
Hiroshiba marked this conversation as resolved.
Show resolved Hide resolved
schema: electronStoreJsonSchema.properties as Schema<ElectronStoreType>,
migrations: {
">=0.13": (store) => {
// acceptTems -> acceptTerms
const prevIdentifier = "acceptTems";
const prevValue = store.get(prevIdentifier, undefined) as
| AcceptTermsStatus
| undefined;
if (prevValue) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
store.delete(prevIdentifier as any);
store.set("acceptTerms", prevValue);
}
},
">=0.14": (store) => {
// FIXME: できるならEngineManagerからEnginIDを取得したい
if (process.env.DEFAULT_ENGINE_INFOS == undefined)
throw new Error("DEFAULT_ENGINE_INFOS == undefined");
const engineId = EngineId(
JSON.parse(process.env.DEFAULT_ENGINE_INFOS)[0].uuid
);
if (engineId == undefined)
throw new Error("DEFAULT_ENGINE_INFOS[0].uuid == undefined");
const prevDefaultStyleIds = store.get("defaultStyleIds");
store.set(
"defaultStyleIds",
prevDefaultStyleIds.map((defaultStyle) => ({
engineId,
speakerUuid: defaultStyle.speakerUuid,
defaultStyleId: defaultStyle.defaultStyleId,
}))
);
let store: Store<ElectronStoreType>;
try {
store = new Store<ElectronStoreType>({
schema: electronStoreJsonSchema.properties as Schema<ElectronStoreType>,
migrations: {
">=0.13": (store) => {
// acceptTems -> acceptTerms
const prevIdentifier = "acceptTems";
const prevValue = store.get(prevIdentifier, undefined) as
| AcceptTermsStatus
| undefined;
if (prevValue) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
store.delete(prevIdentifier as any);
store.set("acceptTerms", prevValue);
}
},
">=0.14": (store) => {
// FIXME: できるならEngineManagerからEnginIDを取得したい
if (process.env.DEFAULT_ENGINE_INFOS == undefined)
throw new Error("DEFAULT_ENGINE_INFOS == undefined");
const engineId = EngineId(
JSON.parse(process.env.DEFAULT_ENGINE_INFOS)[0].uuid
);
if (engineId == undefined)
throw new Error("DEFAULT_ENGINE_INFOS[0].uuid == undefined");
const prevDefaultStyleIds = store.get("defaultStyleIds");
store.set(
"defaultStyleIds",
prevDefaultStyleIds.map((defaultStyle) => ({
engineId,
speakerUuid: defaultStyle.speakerUuid,
defaultStyleId: defaultStyle.defaultStyleId,
}))
);

const outputSamplingRate: number =
const outputSamplingRate: number =
// @ts-expect-error 削除されたパラメータ。
store.get("savingSetting").outputSamplingRate;
store.set(`engineSettings.${engineId}`, {
useGpu: store.get("useGpu"),
outputSamplingRate:
outputSamplingRate === 24000 ? "engineDefault" : outputSamplingRate,
});
// @ts-expect-error 削除されたパラメータ。
store.get("savingSetting").outputSamplingRate;
store.set(`engineSettings.${engineId}`, {
useGpu: store.get("useGpu"),
outputSamplingRate:
outputSamplingRate === 24000 ? "engineDefault" : outputSamplingRate,
});
// @ts-expect-error 削除されたパラメータ。
store.delete("savingSetting.outputSamplingRate");
// @ts-expect-error 削除されたパラメータ。
store.delete("useGpu");
store.delete("savingSetting.outputSamplingRate");
// @ts-expect-error 削除されたパラメータ。
store.delete("useGpu");
},
},
},
});
});
} catch (e) {
log.error(
`設定ファイルの読み込みに失敗しました。${app.getPath(
"userData"
)}にあるconfig.json の名前を変えることで解決することがあります（ただし設定がすべてリセットされます）。`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

フォルダを開くボタン（またはリネームして再起動ボタン）を追加してあげると親切なような気がしました。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

electron 自体が起動する前なので、ウィンドウを出すのも厳しい気がします。
検証はしてみますが。

Copy link
Member

@sevenc-nanashi sevenc-nanashi Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.electronjs.org/ja/docs/latest/api/dialog#dialogshowerrorboxtitle-content

app モジュールで ready イベントが発生する前でも、このAPIは安全に呼び出すことができます。これは、起動の初期段階でのエラーを報告するのによく使用されます。

これが使えそう？
選択はできませんが

Copy link
Member

@Hiroshiba Hiroshiba Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、僕もちょっと思いました。ダイアログにしたら良さそうだなと。
たしかにdialog.showErrorBox使えそうですね！！
他にもdialog.showErrorBox使ってる所あるので参考になるかもです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dialog.showErrorBox を出した後に、shell.openPath や shell.showItemInFolder でフォルダを開けないか試してみましたが、流石に ready 前なので動かなそうでした。
ひとまず dialog.showErrorBox にしますね。

Copy link
Member

@Hiroshiba Hiroshiba Feb 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

個人的には依存増やしてまで（メンテコスト増やしてまで）ディレクトリ開く機能を付けるのはしなくても良いかも？と思いました。
まあ、@raa0121 さんのやりたいように…！

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

私も同じく、依存を増やすほどじゃないかなと思いました。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

どうでしょう？ @sevenc-nanashi
まあ良いかと思える感じであればapproveいただけると！！

Copy link
Member

@sevenc-nanashi sevenc-nanashi Feb 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

依存を増やしたくない、なるほど（そういう考えをしたことが無かった）

まぁ大丈夫だと思います。
（ボタンを提案した理由はファイルパスを手打ちしなくてもいいから。エラーダイアログはコピペが出来ないので）

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

手打ちが面倒なのはめちゃわかります。
そもそも開発者向けには、やはりconfig.jsonをクリアする方法を提供するのが一番いいかなと思いました。
（そしてそれは結構実装が大変。electron-storeの仕様が。。。 😇 ）

);
log.error(e);
process.exit(1);
Hiroshiba marked this conversation as resolved.
Show resolved Hide resolved
}

// engine
const vvppEngineDir = path.join(app.getPath("userData"), "vvpp-engines");
Expand Down