操作
Bug(バグ) #2360
完了#1373 で実施したメール投稿の例外を受ける処理が不適切である
開始日:
2011-08-19
期日:
2011-10-06
進捗率:
100%
予定工数:
3.6 で発生するか:
Unknown (未調査)
3.8 で発生するか:
Unknown (未調査)
説明
Overview (概要)¶
http://redmine.openpne.jp/issues/1372#note-17 の指摘点をコピー
opMailSend クラス内で例外を止めてリダイレクト処理までしてしまう変更は不適切だと思います。このような変更をしてしまうと、opMailSend クラスを利用するコード側でメール送信失敗時の例外処理を記述することができなくなります。 その意味では #1373 で行われた 3.4 向けの変更も不適切だと思います。(もちろん意図しない 500 エラーが発生しなくなったことはよいことですが、これまで利用可能であった例外が利用不可になっており、(バンドルにはありませんでしたが)プラグインやカスタマイズされたコードの動作に影響を与えている可能性があります) メール送信専用の共通エラー画面を用意するのであれば、CSRFトークンエラー用のエラー画面と同じように opExecutionFilter のような opMailSend クラスを利用することが想定されるクラス(例えば、アクションクラス)よりも上位のクラスで例外をキャッチし、フォワード(もしくはリダイレクト)の処理を行うべきではないでしょうか。 タスク内からの呼び出し時に関してはエラー画面とは別の考慮が必要ですが、ログにエラーが記録されるのみでタスク内からメール送信処理の失敗が検知できないような変更よりは、仮にうまく共通化ができなかったとしても個別タスク内できちんと例外処理を行うように修正するほうがよいと思います。
Causes¶
#1373 の修正が原因となり、メールの送信に失敗した場合、メール送信のメソッド内で例外を捕捉してしまっているため、上位のクラスで例外を捕捉できない問題が発生している
修正内容¶
#1373 でのコミットを取消、 #1372 の修正と同等の修正内容を取り込む
- sfOpenPNEMailSend::execute() で例外をキャッチしている処理は取消し、適切な場所で例外がキャッチできなくなる問題を改善する。
- このままではアクションの処理の段階で例外が投げられた場合、ユーザ画面に500のレスポンスを返すため、 sfOpenPNEExecutionFilter クラスで例外をキャッチし、適切な画面を表示するように対応した #1372 の修正を取り込み
Shinichi Urabe さんが約13年前に更新
- ステータス を New(新規) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
更新履歴 3e1322737cb934269ba94fe3d0c3d958d8f5a027 で適用されました。
Minoru Takai さんが約13年前に更新
- ステータス を Pending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更
本チケットでの修正 3e132273 が http://redmine.openpne.jp/issues/1372#note-21 以降のコミットをマージしたもので、結果的に #1372 のレビュー時点(note-30)と同等の状況になっていることを確認しました。レビューOKです。
Mutsumi Imamura さんが約13年前に更新
- ステータス を Pending Testing(テスト待ち) から Fixed(完了) に変更
- 進捗率 を 70 から 100 に変更
動作確認OKです。
操作