fix(users/show): ユーザーが見つからなかった場合に404ステータスコードを返す

[choco14t]

What

エンドポイント /users/show において、ユーザーが見つからなかった場合に 404 ステータスコードを返すようにしました。

Why

現状、ApiError の実装上 kind が指定されていない場合に client が代入される実装となっています。
上記により httpStatusCode が設定されていない場合はステータスコード 400 で返却されます。

データが見つからない場合のエラーコードとしては不適切と思ったため PR を作成しました。

Additional info (optional)

Checklist

• Read the contribution guide
• Test working in a local environment
• (If needed) Update CHANGELOG.md
• (If possible) Add tests

[choco14t]

WSL 上で修正したためか、ローカルで e2e test の実行に失敗したため CI で担保しています。
エラー内容を一応貼っておきます。

❯ pnpm jest -- endpoints.ts

> [email protected] jest /path/misskey
> cd packages/backend && pnpm jest "--" "endpoints.ts"


> backend@ jest /path/misskey/packages/backend
> cross-env NODE_ENV=test node --experimental-vm-modules --experimental-import-meta-resolve node_modules/jest/bin/jest.js --forceExit "--" "endpoints.ts"

(node:7790) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:7790) NOTE: The AWS SDK for JavaScript (v2) will be put into maintenance mode in 2023.

Please migrate your code to use AWS SDK for JavaScript (v3).
For more information, check the migration guide at https://a.co/7PzMCcy
 FAIL  test/e2e/endpoints.ts
  ● Test suite failed to run

    ENOENT: no such file or directory, open '/path/misskey/packages/backend/src/../../../built/meta.json'

      112 |
      113 | export function loadConfig() {
    > 114 |     const meta = JSON.parse(fs.readFileSync(`${_dirname}/../../../built/meta.json`, 'utf-8'));
          |                                ^
      115 |     const clientManifestExists = fs.existsSync(_dirname + '/../../../built/_vite_/manifest.json');
      116 |     const clientManifest = clientManifestExists ?
      117 |             JSON.parse(fs.readFileSync(`${_dirname}/../../../built/_vite_/manifest.json`, 'utf-8'))

      at readFileSync (src/config.ts:114:29)
      at loadConfig (src/GlobalModule.ts:12:16)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        4.095 s
Ran all test suites matching /endpoints.ts/i.
Force exiting Jest: Have you considered using `--detectOpenHandles` to detect async operations that kept running after all tests finished?
 ELIFECYCLE  Command failed with exit code 1.
 ELIFECYCLE  Command failed with exit code 1.

[syuilo]

👍

[Nanashia]

過去のコメントで下記のような方針が出されていますが、入れてしまって大丈夫でしょうか？
#3619

ステータスコードにあまり依存したくない気持ちがあります(400で統一したい) 例えばファイルをフォルダに入れるAPIで404を返しても、ファイルが無かったのかフォルダが無かったのか判別できないので結局はエラーコードのようなものを返す必要があります

また、この修正はクライアントの動作を変えてしまう可能性があるため、入れるのであれば Breaking Change になると思います。

[choco14t]

@syuilo
Nanashia さんが書かれている通りなので、不適切であれば revert してもらって問題ないです 👌🏻

現状の展望だとどちらになるかだけ聞かせてもらえないですか？あくまで参考程度なので希望とかでも大丈夫です。

• ステータスコードの方針として現状も変わらず 400 で統一する
• 段階的に適切なステータスコードを返すよう修正していく

[syuilo]

NO_SUCH_*系のエラーくらいは404で返すようにしても良いと思います。
具体的なHTTPステータスコードで処理を変えるクライアントはほとんど無いと思いますし(4xx系か5xx系かなどでの判定はあると思いますが)、Misskeyとしもエラーの判定はエラーコードまたはIDを用いることを想定しているのでBreakingChangeかは微妙です。

[Nanashia]

各位コメントありがとうございます～。個人的に懸念しているのは下記の点です。

• ステータスコードを返すとなると、コードとテストとAPI仕様、ドキュメントのメンテの大変さが(少しとはいえ)増しそう。その割にステータスコードで判定できてうれしい人ってあまりいないのでは？といったあたりはもう少し考える余地があるかもしれない。
• ママの言う通り、確かに本当にステータスコードを見ているアプリがいるかどうかは微妙（正直いないと思う）だけど、本当にいた場合リリースして壊れちゃったのリスクが怖い。

重要な修正ならリスクをとってもいいけど、そうでもないなら一旦revertして仕切りなおしたほうが諸々モメなくて無難かなあ、と思いました。（ただ、絶対revertしてくれみたいな気持ちはないです。）