Develop に追従するとログインできない(dc3629e)

kanarikanaru commented 1 week ago

💡 Summary

feat(backend): report `Retry-After` if client hit rate limit (#13949) · misskey-dev/misskey@dc3629e · GitHub これを適用すると、新規にログインできない等の問題が発生する。現状Developブランチにのみにあるコミットです。

🥰 Expected Behavior

新規にログインできる
デフォルトロールでノートやリアクションが消せる

🤬 Actual Behavior

ログインするためにPOSTしても結果が返ってこない
nginxのリバースプロキシを経由させなくても発生

📝 Steps to Reproduce

Developに追従する
新しいブラウザでログインを試みる
ぐるぐる

💻 Frontend Environment

* Model and OS of the device(s): MacBook Pro (14inch, 2021), macOS Sonoma 14.5

* Browser: Chrome 125
* Server URL:misskey.backspace.fm
* Misskey:2024.05(Developに追従、該当コミットをrevertして通常通り動作することを確認)

🛰 Backend Environment (for server admin)

* Installation Method or Hosting Service:systemd
* Misskey: 2024.05(Developに追従、該当コミットをrevertして通常通り動作することを確認)
* Node: 20.14
* PostgreSQL: 15
* Redis: 7
* OS and Architecture: Ubuntu 22

Do you want to address this bug yourself?

[ ] Yes, I will patch the bug myself and send a pull request

tai-cha commented 1 week ago

追加：新規アカウント作成も通らないかも → とても長いローディングのあとデータのみ作成された（アカウントのチュートリアルに進まないもののユーザーは作成された）

tai-cha commented 1 week ago

ローカルdev環境では再現できず、misskey-tgaでは再現できる

KisaragiEffective commented 1 week ago

~~外出中のため帰ったら確認します~~

→確認済み

u1-liquid commented 1 week ago

ローカルdev環境では再現できず

👀 https://github.com/misskey-dev/misskey/blob/c51347d78bc6dd30b6b4db2af64f0ea4bc83091e/packages/backend/src/server/api/RateLimiterService.ts#L28-L30

zyoshoka commented 1 week ago

https://github.com/misskey-dev/misskey/blob/c51347d78bc6dd30b6b4db2af64f0ea4bc83091e/packages/backend/src/server/api/RateLimiterService.ts#L60

記法的に良いのか分かりませんがここをとりあえず

return max.then(ok).catch(e => reject(e));

とすると直りました

anatawa12 commented 1 week ago

その方針で行くのであればmax.then(ok, reject)のほうが良さそう。PRはやします

tai-cha commented 1 week ago

自分も return max.then(ok).catch(reject); で動くのは確認してPR出そうと思ってました（anatawa12さんのでよさそう）

anatawa12 commented 1 week ago

ok(Promiseのresolveコールバック)がエラー出すこと多分ないけどthen(ok).catch(reject)だと両方呼ばれるリスクがあるので一応thenに２つのほうが良さそうとしました

KisaragiEffective commented 1 week ago

off-topic(?): resolveを呼んでいないのはESLintのno-floating-promiseで検知されているかと思ったが、CI実行時に--quietがついているためおそらく抑制されている。これも帰ったら見る。

KisaragiEffective commented 1 week ago

off-topic(?): resolveを呼んでいないのはESLintのno-floating-promiseで検知されているかと思ったが、CI実行時に--quietがついているためおそらく抑制されている。これも帰ったら見る。

→ c51347d78bc6dd30b6b4db2af64f0ea4bc83091e において /misskey/packages/backend $ eslint "src/*/**.ts" を実行してもno-floating-promiseが発火されていない。

anatawa12 commented 1 week ago

voidにreturnしてるところで発火してもいい気はするけど、new Promiseのresolveに関してはlimiter.getのコールバックがいつどのように呼ばれるか不定だからlinterがどうにかできる範疇を超えてるきはする

misskey-dev / misskey