Commit d41debaa49968bfdfb27bc309f628b6ec8ad492d
1 parent
95bfe3f7
Exists in
master
and in
1 other branch
Rescue if there are failure in notification system
The rescue is catch and report on the errbit notification system Fix #551
Showing
2 changed files
with
36 additions
and
0 deletions
Show diff stats
app/models/notice.rb
| ... | ... | @@ -170,6 +170,8 @@ class Notice |
| 170 | 170 | def email_notification |
| 171 | 171 | return true unless should_email? |
| 172 | 172 | Mailer.err_notification(self).deliver |
| 173 | + rescue => e | |
| 174 | + HoptoadNotifier.notify(e) | |
| 173 | 175 | end |
| 174 | 176 | |
| 175 | 177 | ## |
| ... | ... | @@ -177,6 +179,8 @@ class Notice |
| 177 | 179 | def services_notification |
| 178 | 180 | return true unless app.notification_service_configured? and should_notify? |
| 179 | 181 | app.notification_service.create_notification(problem) |
| 182 | + rescue => e | |
| 183 | + HoptoadNotifier.notify(e) | |
| 180 | 184 | end |
| 181 | 185 | |
| 182 | 186 | end | ... | ... |
spec/models/notice_observer_spec.rb
| ... | ... | @@ -24,6 +24,7 @@ describe "Callback on Notice" do |
| 24 | 24 | end |
| 25 | 25 | end |
| 26 | 26 | |
| 27 | + | |
| 27 | 28 | describe "email notifications for a resolved issue" do |
| 28 | 29 | before do |
| 29 | 30 | Errbit::Config.per_app_email_at_notices = true |
| ... | ... | @@ -41,6 +42,13 @@ describe "Callback on Notice" do |
| 41 | 42 | and_return(double('email', :deliver => true)) |
| 42 | 43 | Fabricate(:notice, :err => @err) |
| 43 | 44 | end |
| 45 | + it 'self notify if mailer failed' do | |
| 46 | + @err.problem.resolve! | |
| 47 | + Mailer.should_receive(:err_notification). | |
| 48 | + and_raise(ArgumentError) | |
| 49 | + HoptoadNotifier.should_receive(:notify) | |
| 50 | + Fabricate(:notice, :err => @err) | |
| 51 | + end | |
| 44 | 52 | end |
| 45 | 53 | |
| 46 | 54 | describe "should send a notification if a notification service is configured with defaults" do |
| ... | ... | @@ -64,6 +72,30 @@ describe "Callback on Notice" do |
| 64 | 72 | end |
| 65 | 73 | end |
| 66 | 74 | |
| 75 | + describe "send a notification if a notification service is configured with defaults but failed" do | |
| 76 | + let(:app) { Fabricate(:app_with_watcher, | |
| 77 | + :notify_on_errs => true, | |
| 78 | + :email_at_notices => [1, 100], :notification_service => Fabricate(:campfire_notification_service))} | |
| 79 | + let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 99)) } | |
| 80 | + let(:backtrace) { Fabricate(:backtrace) } | |
| 81 | + | |
| 82 | + before do | |
| 83 | + Errbit::Config.per_app_email_at_notices = true | |
| 84 | + end | |
| 85 | + | |
| 86 | + after do | |
| 87 | + Errbit::Config.per_app_email_at_notices = false | |
| 88 | + end | |
| 89 | + | |
| 90 | + it "send email" do | |
| 91 | + app.notification_service.should_receive(:create_notification).and_raise(ArgumentError) | |
| 92 | + Mailer.should_receive(:err_notification).and_return(double(:deliver => true)) | |
| 93 | + | |
| 94 | + Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | |
| 95 | + :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | |
| 96 | + end | |
| 97 | + end | |
| 98 | + | |
| 67 | 99 | describe "should not send a notification if a notification service is not configured" do |
| 68 | 100 | let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))} |
| 69 | 101 | let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } | ... | ... |