APIのリクエスト内で、DBに繋がらない, DBクエリが誤っている などの理由でエラーが発生した場合
サーバー側では例外が発生しているにもかかわらず、クライアントには応答が返ってこなくなる。
DBクエリに限らず、API endpointsで使用されている
export default (params: any, user: ILocalUser) => new Promise(async (res, rej) => { 内で
awaitを使用しているコードが全て該当してそう。
ただのasyncにすれば思ったとおりに動きそう
https://github.com/mei23/misskey/commit/69d7d7af08a4d284d54986453ba1fdda2c14b75c
DBエラー時に、APIリクエストが完了しエラーが返る。
DBエラーが発生しても、APIがリクエスト中のままになる。
curl => Misskey 9.7.1 などで検証
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
ありがとうございます!
なぜ async ではなく Promise を使っているのかというと、前者では、値を返すには return する必要があり、当然ですがそうするとそれ以降に処理を書いたとしても実行されることはありません。
これで問題になるのは、何かクライアントにレスポンスを返した後で、何かしら後処理をしたい場合です。async にしてしまうと、クライアントにレスポンスを返すには return するしかなくなり、レスポンスを返した後の後処理ができなくなります。
それに対して現在のアプローチである Promise を生成して返す方法では、クライアントにレスポンスを返すのは return ではなくリゾルブコールバックを呼んだときなので、クライアントに何かレスポンスを返した後も処理を続行することができます。
@syuilo Promiseが存在する理由は理解したわ。
new Promise(async (res, rej) => { } 内のawaitは厳密にはcatchで守る必要がある。
特に後続処理が必要ないならばasyncでよい
(既存の後続処理がないAPIで、「asyncが少数」「Promiseが大多数」になっていたので・・・)
という認識でOKでしょうか?
そうですね。
ただ async 方式と Promise 方式が混在しているのもあれなので、全部(後処理が無いやつも)Promiseに統一しようかなと思ったりしてます。
なるほど(しんどそうだわ)
何かいいアイデアがあれば...
大量にあるDB系のawaitを全部守るのがしんどい気がしたけど
丸ごとtryで囲むとか
Promise直下にあまり処理を書かないようにすればいいだけな気もしたわ。
export default (params: any, user: ILocalUser) => new Promise((res, rej) => {
const [ps, psErr] = getParams(meta, params);
if (psErr) return rej(psErr);
mainProcess(ps, user).then(r => res(r), e => rej(e));
postProcess();
});
const mainProcess = async (ps: any, user: ILocalUser) => {
なるほど~!
修正されたと思います