Bug(バグ) #2360
#1373 で実施したメール投稿の例外を受ける処理が不適切である
100%
説明
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 の修正を取り込み
関連するチケット
関係しているリビジョン
Squashed commit of the following:
commit 93905c08d7c91ca71152fa0b7e251f5d791cb734
Author: touri <tourimgr@gmail.com>
Date: Wed Aug 17 19:04:42 2011 +0900
fixed to mail error template. (fixes #2360, BP from #1372)
commit 3fc1cdb0aeb224ddf0eaa256ea0a08cc8302037e
Author: touri <tourimgr@gmail.com>
Date: Wed Aug 17 17:21:02 2011 +0900
changed order of process in opMemberAction::executeConfig().
fixed to mail error template. (fixes #2360, BP from #1372)
Conflicts:
lib/action/opMemberAction.class.php
commit 1f56cbf0ca444d19269a9ec9ba5bffa4613b497c
Author: ShinichiU <urabe@nuts-choco.com>
Date: Tue Aug 9 14:19:00 2011 +0900
(refs #1373, BP from #1372) cache Zend_Mail_Protocol_Exception in opExecutionFilter and forwad mailError action.
Conflicts:
lib/filter/opExecutionFilter.class.php
commit c1f6ee4093b994cb483ad17292cc58ced5c243e3
Author: ShinichiU <urabe@tejimaya.com>
Date: Thu Oct 6 20:44:23 2011 +0900
Revert "(fixes #1373) This modification is that modified to catch the exception when sending mail"
This reverts commit c0dd5cb0f8ec1d5c56efb164960a07c228dfeaf5.
履歴
#1 Shingo Yamada が12年以上前に更新
- 優先度 を Normal(通常) から High(高め) に変更
#2 Shingo Yamada が12年以上前に更新
- 360対象 を RC1 にセット
#3 Shingo Yamada が12年以上前に更新
- 360対象 を削除 (
RC1)
#4 Shinichi Urabe が12年以上前に更新
- 期日 を 2011-10-06 にセット
- 担当者 を Shinichi Urabe にセット
#5 Shinichi Urabe が12年以上前に更新
- ステータス を New(新規) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
更新履歴 3e1322737cb934269ba94fe3d0c3d958d8f5a027 で適用されました。
#6 Shinichi Urabe が12年以上前に更新
更新履歴 3e1322737cb934269ba94fe3d0c3d958d8f5a027 で適用されました。
#7 Shinichi Urabe が12年以上前に更新
- 説明 を更新 (diff)
#8 Minoru Takai が12年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更
本チケットでの修正 3e132273 が http://redmine.openpne.jp/issues/1372#note-21 以降のコミットをマージしたもので、結果的に #1372 のレビュー時点(note-30)と同等の状況になっていることを確認しました。レビューOKです。
#9 Mutsumi Imamura が12年以上前に更新
- ステータス を Pending Testing(テスト待ち) から Fixed(完了) に変更
- 進捗率 を 70 から 100 に変更
動作確認OKです。