bcdice / bcdice-api

BCDiceを提供するWeb APIサーバー
MIT License
35 stars 37 forks source link

不正なコマンドを受け取った際のエラーハンドリング #6

Closed daicyan closed 6 years ago

daicyan commented 6 years ago

どどんとふ公式鯖で受け取っているDiceBotのかなりの数が、Command=部分に普通のチャットメッセージ等が挿入されていて、その際に「NoMethodError - undefined method `gsub' for nil:NilClass:」や「CommandError - CommandError:」のExceptionのログが大量に吐かれてしまいます。 API的には400を返して終わりなので利用者や開発者側から気がつくことなくサーバー側にかなりの量のログが溜まってしまうので、この部分のエラーをTRAPするようにする等はできないでしょうか?

ysakasin commented 6 years ago

指摘ありがとうございます。

エンドユーザーに対する周知は400を受け取ったクライアントがすべきことなので、APIとしてできるのはログの扱いを改善することかなと思います。CommandErrorUnsupportedDicebotの時には単純にログを隠蔽するのが手っ取り早いと思うのですが、どうでしょうか?

加えて、NoMethodErrorが起こっていることは想定外なので、CommandErrorUnsupportedDicebotになるように仕向けるつもりです。

ysakasin commented 6 years ago

あと、どのようなテキストでNoMethodErrorが起こっているのか教えていただけると嬉しいです。

daicyan commented 6 years ago

はい、あくまでクライアントに返すべきものは400で問題ないと思っていますので、logの量と可読性とExceptionというイメージだけの問題だと思っています。 NoMethodErrorは、ログにあった分は全てgsub部分でそのうちのいくつかをURLデコードしたところ、「/」記号混じり、「ブランク」、「URLを含んだ文字列」などだったので、エスケープ処理がうまく機能していないのか機能した結果Exeptionになったのかな??

とりあえず、IPアドレスをマスキングした実際のログファイルをアップロードいたしました。 https://oc.taruki.com:60443/owncloud/index.php/s/r6ynvcSEa8ZqZ4o

ysakasin commented 6 years ago

ログファイルを確認しました。

確かにこれだけの量を出力されるのは問題ですね。 SinatraでLoggingのハンドリングができるのかわからないのですが、いろいろやってみます。

ysakasin commented 6 years ago

📝 ログを見た感じ500はcommandが空白の時だけ

ysakasin commented 6 years ago

先ほど上記修正を適用したBCDice-API 0.5.3 をリリースしました。rackup時にproductionを指定するとバックトレースのログ出力が無くなります。