Project

General

Profile

Bug(バグ) #1372

メールサーバーに接続できない場合の例外処理がキャッチされていないため500エラーになる

Added by Shinichi Urabe over 12 years ago. Updated over 7 years ago.

Status:
Fixed(完了)
Priority:
High(高め)
Target version:
Start date:
2010-07-18
Due date:
% Done:

100%

3.6 で発生するか:
Yes
3.8 で発生するか:
Unknown (未調査)

Description

現象

  • config/OpenPNE.yml の mail_smtp_host に接続できないメールサーバーのホストを記述し、招待メールを送信すると500エラーになる
  • メールサーバーの障害の対処がされていない (エラー表示やログ出力などの対処が必要)

    バージョン:3.6beta1

原因

Zend_Mail_Protocol_Exception の例外がキャッチされていない

修正内容


Related issues

Related to OpenPNE 3 - Backport(バックポート) #1373: メールサーバーに接続できない場合の例外処理がキャッチされていないため500エラーになる Fixed(完了) 2010-07-18
Related to OpenPNE 3 - Backport(バックポート) #1374: メールサーバーに接続できない場合の例外処理がキャッチされていないため500エラーになる Invalid(無効) 2010-07-18
Related to OpenPNE 3 - Backport(バックポート) #1407: メールサーバーに接続できない場合の例外処理がキャッチされていないため500エラーになる Fixed(完了) 2010-07-18
Related to OpenPNE 3 - Bug(バグ) #2328: DB接続障害発生時などにメール投稿をした際に「現在、サーバが混み合っているか〜」のHTML形式の文言がリターンメールとして送信される New(新規) 2011-07-31
Related to OpenPNE 3 - Bug(バグ) #2360: #1373 で実施したメール投稿の例外を受ける処理が不適切である Fixed(完了) 2011-08-19 2011-10-06
Related to OpenPNE 3 - Backport(バックポート) #2521: メールサーバーに接続できない場合の例外処理がキャッチされていないため500エラーになる Invalid(無効) 2010-07-18

Associated revisions

Revision 2c0e95dc (diff)
Added by Itsuro Tajima over 12 years ago

(fixes #1372) fixed to catch mail send error exception on sending invitation

Revision 461564a2 (diff)
Added by Itsuro Tajima over 12 years ago

(fixes #1372) fixed to output error message on mail not sent

Revision 17e886ec (diff)
Added by Itsuro Tajima over 12 years ago

(fixes #1372) fixed no need error output

Revision 59aaaa6d (diff)
Added by Itsuro Tajima over 12 years ago

Revert "(fixes #1372) fixed to catch mail send error exception on sending invitation"

This reverts commit 2c0e95dc7743ef515e4fccc92e4492588c0344d9.

Revision 3586cc94 (diff)
Added by Shingo Yamada over 11 years ago

delete the incorrect description contained in the messages.ja.xml. (fixes #1372)

Revision 22e2f57f (diff)
Added by Shinichi Urabe over 11 years ago

Revert "delete the incorrect description contained in the messages.ja.xml. (fixes #1372)"

This reverts commit 3586cc9458e4701d8d4bf3a6945a74a85a55bf8b.

Revision 4a81cebb (diff)
Added by Shinichi Urabe over 11 years ago

Revert "Revert "(fixes #1372) fixed to catch mail send error exception on sending invitation""

This reverts commit 59aaaa6dfbe97dd893347af76d44a1db00a4a0dd.

Revision 016e30d6 (diff)
Added by Shinichi Urabe over 11 years ago

Revert "(fixes #1372) fixed no need error output"

This reverts commit 17e886ec5d4ab06bcbe8942fae86ff4cab474318.

Revision a568d894 (diff)
Added by Shinichi Urabe over 11 years ago

Revert "(fixes #1372) fixed to output error message on mail not sent"

This reverts commit 461564a2fcb13ad7fd6b426caadc8453f890fdd2.

Conflicts:

apps/mobile_frontend/i18n/messages.ja.xml
apps/pc_backend/i18n/messages.ja.xml

Revision eb8b102c (diff)
Added by Shinichi Urabe over 11 years ago

Revert "(fixes #1372) fixed to catch mail send error exception on sending invitation"

This reverts commit 2c0e95dc7743ef515e4fccc92e4492588c0344d9.

Revision 69791cd8 (diff)
Added by Shinichi Urabe over 11 years ago

(refs #1372) cache Zend_Mail_Protocol_Exception in opExecutionFilter and forwad mailError action.

Revision d8966456 (diff)
Added by Hiroshi Kato over 11 years ago

changed order of process in opMemberAction::executeConfig().
fixed to mail error template. (fixes #1372)

Revision e099cc79 (diff)
Added by Hiroshi Kato over 11 years ago

fixed to mail error template. (fixes #1372)

History

#1 Updated by Rimpei Ogawa over 12 years ago

  • 3.6 で発生するか set to Yes

#2 Updated by Itsuro Tajima over 12 years ago

  • Status changed from New(新規) to Accepted(着手)
  • Assignee set to Itsuro Tajima

#3 Updated by Itsuro Tajima over 12 years ago

いくつか修正案を考えています。

  1. 単純にキャッチして何もせず、成功時と同じ画面遷移にする
  2. opMemberActionのエラーページに条件分岐でメールエラーを出す
  3. メール専用にエラーページを作り、そこにリダイレクトする

とりあえず、現在の方針としては1を考えています。

#4 Updated by Itsuro Tajima over 12 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

更新履歴 2c0e95dc7743ef515e4fccc92e4492588c0344d9 で適用されました。

#5 Updated by Shogo Kawahara over 12 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

差し戻しです。

  • メール送信処理は lib/action/opMemberAction.class.php

だけでやっているわけではないので、ここで例外をキャッチするのは良くないです。
やるとすれば opMailSend::execute() でちょうどメールを送信している箇所が適切と言えるでしょう。

  • もしエラーで例外をキャッチするならば、ログは残したほうが良いでしょう。

#6 Updated by Itsuro Tajima over 12 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

opMailSendでのメール送信はaction, task両方で使われているため、opMailSend.phpに
・ログを残す
・actionとtaskの判別
・taskの場合「Mail Send Error」と表示
・actionの場合、エラーページにリダイレクト
以上の例外処理を追加します。

#7 Updated by Itsuro Tajima over 12 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

更新履歴 461564a2fcb13ad7fd6b426caadc8453f890fdd2 で適用されました。

#8 Updated by Itsuro Tajima over 12 years ago

taskかactionかの判別には、synfony 1.2では
http://blog.asial.co.jp/615
の方法がありますが、1.4では正常に動作しませんでした。
このため、アクションの名前を参照することで判別しています。

(未実装項目)
ログを残す部分

また、最初のopMemberAction固有のコミットはrevertしました。

#9 Updated by Itsuro Tajima over 12 years ago

更新履歴 59aaaa6dfbe97dd893347af76d44a1db00a4a0dd で適用されました。

#10 Updated by Itsuro Tajima over 12 years ago

更新履歴 17e886ec5d4ab06bcbe8942fae86ff4cab474318 で適用されました。

#11 Updated by Masato Nagasawa almost 12 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

3.4系(#1373) と修正内容が異なっているので、問題がないか確認をお願いします。

#12 Updated by Shingo Yamada over 11 years ago

  • Priority changed from Normal(通常) to High(高め)

#13 Updated by Minoru Takai over 11 years ago

このチケットの状況を確認したのでコメントしておきます。

3.4.x 向けチケットでは http://redmine.openpne.jp/issues/1373#note-6 の通り、「メールサーバに接続できない場合には500エラーとならないようにする」という修正となっています。そして master 向けチケットでは http://redmine.openpne.jp/issues/1372#note-6 の通り、「メールサーバに接続できない場合には、500エラーとならないようにすることに加え、メール送信が失敗したことを示すためのエラーページに遷移させる」という修正となっています。

修正方針は良いと思いますので、メールサーバに接続できない状態でSNSからメールを送信した場合に、動作が不自然でないか、エラーページの表示は不自然でないかを確認できれば、この修正で問題を解消したと判断して良いと思います。

ちなみに簡単にコードを確認してみましたが、 apps/mobile_frontend/i18n/messages.ja.xml の修正 で「下さい>。」と句点の直前に不適切な文字が含まれていることを確認しました。

#14 Updated by Shingo Yamada over 11 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

対応します。

#15 Updated by Shingo Yamada over 11 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

Minoru Takai は書きました:

ちなみに簡単にコードを確認してみましたが、 apps/mobile_frontend/i18n/messages.ja.xml の修正 で「下さい>。」と句点の直前に不適切な文字が含まれていることを確認しました。

上記指摘点のみ 3586cc9458e4701d8d4bf3a6945a74a85a55bf8b で修正しました。

#16 Updated by Shingo Yamada over 11 years ago

更新履歴 3586cc9458e4701d8d4bf3a6945a74a85a55bf8b で適用されました。

#17 Updated by Rimpei Ogawa over 11 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

opMailSend クラス内で例外を止めてリダイレクト処理までしてしまう変更は不適切だと思います。このような変更をしてしまうと、opMailSend クラスを利用するコード側でメール送信失敗時の例外処理を記述することができなくなります。

その意味では #1373 で行われた 3.4 向けの変更も不適切だと思います。(もちろん意図しない 500 エラーが発生しなくなったことはよいことですが、これまで利用可能であった例外が利用不可になっており、(バンドルにはありませんでしたが)プラグインやカスタマイズされたコードの動作に影響を与えている可能性があります)

メール送信専用の共通エラー画面を用意するのであれば、CSRFトークンエラー用のエラー画面と同じように opExecutionFilter のような opMailSend クラスを利用することが想定されるクラス(例えば、アクションクラス)よりも上位のクラスで例外をキャッチし、フォワード(もしくはリダイレクト)の処理を行うべきではないでしょうか。

タスク内からの呼び出し時に関してはエラー画面とは別の考慮が必要ですが、ログにエラーが記録されるのみでタスク内からメール送信処理の失敗が検知できないような変更よりは、仮にうまく共通化ができなかったとしても個別タスク内できちんと例外処理を行うように修正するほうがよいと思います。

#18 Updated by Shinichi Urabe over 11 years ago

  • Assignee changed from Itsuro Tajima to Shinichi Urabe

引き継ぎますー

#19 Updated by Shingo Yamada over 11 years ago

  • 360対象 set to beta13

#20 Updated by Shinichi Urabe over 11 years ago

呼び出されている箇所

urabe@mac OpenPNE3$ ack opMailSend                                                                             11-08-09
apps/pc_backend/modules/member/lib/ReissuePasswordForm.class.php
36:    opMailSend::sendTemplateMail('reissuedPassword', $to, opConfig::get('admin_mail_address'), $params);

→アクション上で呼び出されているので、opExecutionFilterで対処

apps/pc_frontend/modules/member/lib/registerMobileForm.class.php
41:    $mail = new opMailSend();

→アクション上で呼び出されているので、opExecutionFilterで対処

lib/action/opAuthAction.class.php
45:      opMailSend::sendTemplateMailToMember('registerEnd', $member, $params);

→アクション上で呼び出されているので、opExecutionFilterで対処

lib/action/opCommunityAction.class.php
454:      opMailSend::sendTemplateMailToMember('joinCommunity', $community->getAdminMember(), $params, $options);

→アクション上で呼び出されているので、opExecutionFilterで対処

lib/action/opMemberAction.class.php
413:    $mail = new opMailSend();
420:    opMailSend::sendTemplateMailToMember('leave', $member, $param);

→アクション上で呼び出されているので、opExecutionFilterで対処

lib/form/doctrine/InviteForm.class.php
137:    opMailSend::sendTemplateMail('requestRegisterURL', $to, opConfig::get('admin_mail_address'), $params);

→アクション上で呼び出されているので、opExecutionFilterで対処

lib/form/MemberConfigForm/MemberConfigMobileAddressForm.class.php
58:    opMailSend::sendTemplateMail('changeMailAddress', $to, opConfig::get('admin_mail_address'), $params);

→継承している registerMobileFormはアクション上で呼び出されているので、opExecutionFilterで対処

lib/form/MemberConfigForm/MemberConfigPcAddressForm.class.php
58:    opMailSend::sendTemplateMail('changeMailAddress', $to, opConfig::get('admin_mail_address'), $params);

→継承している InviteFormはアクション上で呼び出されているので、opExecutionFilterで対処

lib/model/doctrine/MemberRelationshipTable.class.php
200:      opMailSend::sendTemplateMailToMember('friendLinkComplete', $toMember, $params);

→ MemberRelationshipTable::processFriendConfirm() はイベントで呼び出されるが、アクションないの処理となるので、opExecutionFilterで対処

lib/task/opBaseSendMailTask.class.php
33:    // opMailSend requires Zend

コメント

lib/task/openpneExecutemailactionTask.class.php
58:      opMailSend::execute($subject, $to, $from, $retval);

→ タスクでリターンメールを送信する際に呼び出される、例外をキャッチして別処理を追加する必要はないと考える

lib/task/openpneSendBirthDayMailTask.class.php
58:        opMailSend::sendTemplateMailToMember('birthday', $member, $params, array(), $context);

→ タスクで誕生日通知メールを送信する際に呼び出される、例外をキャッチして別処理を追加する必要はないと考える

lib/task/openpneSendDailyNewsTask.class.php
80:      opMailSend::sendTemplateMail('dailyNews', $address, opConfig::get('admin_mail_address'), $params, $context);

→ タスクでデイリーニュースを送信する際に呼び出される、例外をキャッチして別処理を追加する必要はないと考える
urabe@mac OpenPNE3$ ack sfOpenPNEMailSend                                                                      11-08-09
lib/util/sfOpenPNEMailSend.class.php
23:class sfOpenPNEMailSend extends opMailSend

plugins/opAuthMailAddressPlugin/lib/form/opAuthMailAddressPasswordRecoveryForm.class.php
102:    sfOpenPNEMailSend::sendTemplateMail('passwordRecovery', $this->member->getEMailAddress(), opConfig::get('admin_mail_address'), $params);

→アクション上で呼び出されているので、opExecutionFilterで対処

#21 Updated by Shinichi Urabe over 11 years ago

  • Status changed from Rejected(差し戻し) to Pending Review(レビュー待ち)

更新履歴 eb8b102cae887b84e8bfeca9c43e05fa7f2cdc73 で適用されました。

#22 Updated by Shinichi Urabe over 11 years ago

更新履歴 a568d894ecb91580b2d99d28f11963107a22ead1 で適用されました。

#23 Updated by Shinichi Urabe over 11 years ago

更新履歴 016e30d6bff01b3a4028f614b7a19d82c787755c で適用されました。

#24 Updated by Shinichi Urabe over 11 years ago

更新履歴 4a81cebbafc516e535c11588828e679fe2c02546 で適用されました。

#25 Updated by Shinichi Urabe over 11 years ago

更新履歴 22e2f57f5ebbcc1300ec5f53585cb22343802ba5 で適用されました。

#26 Updated by Shinichi Urabe over 11 years ago

いったん修正を戻し、クラス外から例外がわからないようにする修正は問題なので opMailSend::execute() では例外をキャッチしないように対処

  • アクションは opExecutionFilter で例外をキャッチし、メール送信失敗のページにforwardされるように変更
  • タスク(デイリーニュース、誕生日通知、メール投稿のリターンメール)については実行時にそのまま例外が表示できることが必要と判断し、例外をキャッチしていません

#27 Updated by Maki Takahashi over 11 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)
  • note-16 までの修正が取り消されていることを確認しました。
  • 69791cd8 について
    • pc_frontendおよび、pc_backendでのエラー画面(mailErrorSuccess.php)についてloginError.phpと表示が異なるようです。
      • ただし、csrfErrorSuccess.phpとも画面が違うので「統一されてない」というわけではないです。が、loginError.phpにあわせた方が見栄えが良いように思います。
    • 携帯メールアドレス設定(member/config?category=mobileAddress)でメール送信に失敗してmailErrorSuccess.phpが表示されてもFlashでセットされたデータ「メールを送信しました。・・・・」と出てしまいます(PCメールアドレス設定も同様かと思われます)。

#28 Updated by Hiroshi Kato over 11 years ago

  • Status changed from Rejected(差し戻し) to Pending Review(レビュー待ち)

更新履歴 d89664564b08404f751f658b30dce2822bf5f525 で適用されました。

#29 Updated by Hiroshi Kato over 11 years ago

更新履歴 e099cc79c365f5d6ef96cf2e39e2002eab4d4961 で適用されました。

#30 Updated by Maki Takahashi over 11 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

d8966456 および e099cc79 修正確認いたしました。
問題ないと思います。

#31 Updated by Mutsumi Imamura over 11 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

#32 Updated by kaoru n over 7 years ago

  • 3.8 で発生するか set to Unknown (未調査)

Also available in: Atom PDF