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

正常系以外の response のステータスを確認する #37

Open
kokoichi206 opened this issue Nov 24, 2022 · 4 comments · Fixed by #38
Open

正常系以外の response のステータスを確認する #37

kokoichi206 opened this issue Nov 24, 2022 · 4 comments · Fixed by #38

Comments

@kokoichi206
Copy link
Member

kokoichi206 commented Nov 24, 2022

#34 の一環で、クエリパラメーター key, gn の有無によってステータスコードがどう変わるかを調べた。

その際、gn がない時と key がない時でステータスコードが異なることがわかった。
この辺の設計が正しいか確認・修正する。

gn がない時: 404
key がない時: 403
両方ない時: 404
@kokoichi206
Copy link
Member Author

kokoichi206 commented Nov 24, 2022

正常系

  1. クエリパラメーターに key の値がある
  2. key の値が条件を満たす
    • 今は仮実装として空じゃなければ true を返している
  3. クエリパラメーターに gn の値がある
  4. gn の値が DB の Groups テーブルに存在する
  5. request header から言語情報の指定を取得する
    • 対象が見つからなかった場合、デフォルトの情報を取得
  6. 実際に返却したい値を取得する

上のどこかで転けた時はどうする?

1. クエリパラメーターに key の値がある

key がなかった場合は、こちらの考慮に従い 404 を返す。
ここで処理を中断する。

2. key の値が条件を満たす

『条件を満たさなかった場合 = key が DB に存在しない = 未認証』という扱いのため、401: Unauthorized を返す。
ここで処理を中断する。

条件チェック時に DB 操作からエラーが返ってきた場合
ユーザーからはどうしようもない部分であり、ユーザーのせいでは無いことの方が多いと思われるのでサーバー側の問題であることを示す 500: Internal Server Error を返す。
ここで処理を中断する。

3. クエリパラメーターに gn の値がある

gn がなかった場合は、こちらの考慮に従い 404 を返す。
ここで処理を中断する。

4. gn の値が DB の Groups テーブルに存在する

『条件を満たさなかった場合 = gn が DB に存在しない = ユーザー(利用者の使い方が違う)』という扱いのため、400: Bad Request を返す。
ここで処理を中断する。

条件チェック時に DB 操作からエラーが返ってきた場合
ユーザーからはどうしようもない部分であり、ユーザーのせいでは無いことの方が多いと思われるのでサーバー側の問題であることを示す 500: Internal Server Error を返す。
ここで処理を中断する。

5. request header から言語情報の指定を取得する

指定された言語情報が DB に見つからなかった場合
→ デフォルトの言語設定を割り当てて、処理を続ける

DB アクセス時にエラーが起きてた場合
→ デフォルトの言語設定を割り当てて、処理を続ける
(本当に DB 側に問題が起きてるなら、別のリソース部分で処理が止まるはず。
ここは止めるほどのことではない。)

6. 実際に返却したい値を取得する

条件チェック時に DB 操作からエラーが返ってきた場合
ユーザーからはどうしようもない部分であり、ユーザーのせいでは無いことの方が多いと思われるのでサーバー側の問題であることを示す 500: Internal Server Error を返す。

@kokoichi206
Copy link
Member Author

kokoichi206 commented Nov 24, 2022

なぜ gn がパスパラメーターにないと 404 になっているのか

Router の設定時に Queries を使ってクエリパラメータを指定してたから。

func (server *Server) setupRouter() {
	r := mux.NewRouter()

	rootPath := os.Getenv("SCRIPT_NAME")
	r.Path(rootPath+"/members").
		Queries("gn", "{gn}").
		HandlerFunc(server.getAllMembers).
		Methods("GET")

そもそも必須のクエリパラメータがあることがよくない?
→ openapi とかでも query に required 付けられたし、一般におかしい設計ではなさそう

必須のパラメーターがなかった場合、その URI がエンドポイントとして機能しないのであれば、ハンドラーに渡す前段階で弾いて 404 とかにするのが自然な気がする。

mux の Queries を使ってクエリパラメータを指定した場合

指定したクエリパラメータが存在しなかった場合、404 が返ってくることとなる。

  • メリット
    • ハンドラーに情報を渡す前(より低レベル部分)で弾いてるので、パフォーマンス的に良い
      • PC のリソースにやさしい
  • デメリット
    • デフォルトの 404: page not found のメッセージしかないため、ユーザーがどうしていいかわからない

逆にハンドラーまで処理を渡すのであれば、404 はおかしいと思う。

@kokoichi206
Copy link
Member Author

kokoichi206 commented Nov 26, 2022

must の query parameters がない時、何を返すといいか?

go の mux しかり、Azure API(?) しかり、ライブラリなどを提供する側としては 404 で返すことが多いよう。
ただ、それを 400 に変更したがるユーザーの声も多いみたい。

mux の Queries を使った場合のメリットデメリットの部分と合わせて、今回は以下の理由から mux がデフォルトで返す 404 を返すことにする。

  • 公開 API ではなく、クライアント・サーバー側も完全にこちら管理のアプリからの使用のみを想定している
    • ユーザー(API の利用者)にとってのメッセージは最小限でいい
    • それよりは少しでも無駄なリソースを使わない方がいい
  • 必須のパラメーターがなかった場合、その URI がエンドポイントとして機能しないので、その URI は存在しないも同然
    • NOT FOUND に違和感がない(自然)

Links

@kokoichi206
Copy link
Member Author

status code メモ

MDM を参考にしている

400 Bad Request

Warning
警告: クライアントは変更なしにこのリクエストを繰り返すべきではありません。

構文が無効であるためサーバーがリクエストを理解できないことを示します。

401 Unauthorized

HTTP 標準では "unauthorized" (不許可) と定義されていますが、意味的にはこのレスポンスは "unauthenticated" (未認証) です。

未認証(cf: 認可がない場合は 403)

404

サーバーがリクエストされたリソースを発見できないことを示します。
ブラウザーでは、これは URL が解釈できなかったことを意味します。
API では、これは通信先が有効であるものの、リソース自体が存在しないことを意味することがあります。

これに該当しそう。

Note
『API では、これは通信先が有効であるものの、リソース自体が存在しないことを意味することがあります。』

403 Forbidden

認証されていないなどの理由でクライアントにコンテンツのアクセス権がなく、サーバーが適切なレスポンスの返信を拒否していることを示します。

未認証ではなく、認可なし。

Note
401 Unauthorized とは異なり、クライアントの ID がサーバーに知られています。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant