Exception notifier #416
Exception notifier #416
Conversation
…nto exception-notifier
app/mailers/error_mailer.rb
Outdated
| @@ -10,7 +10,7 @@ def error_email(email, path, user, serializable_error) | |||
|
|
|||
| @host = Rails.env.production? ? 'hub.parking.umass.edu' : 'localhost:3000' | |||
|
|
|||
| subject = "Probable-Engine Error Occured #{Time.zone.now}" | |||
| subject = "Golf-Cart-Rentals Error Occured #{Time.zone.now}" | |||
There was a problem hiding this comment.
I believe we don't have to write our own error mailer anymore with exception notifier.
There was a problem hiding this comment.
I went through and removed our custom error mailer everywhere, replaced it in one function (related to API errors) with a manual call to exception notifier. Ended up removing some tests, but I wasn't sure if I should replace them or we assume exception notifier works as advertised (and should already be tested internally). Also wasn't sure how to go about testing exception notifier sending email. Let me know if I missed anything or need to add anything.
There was a problem hiding this comment.
We use it in all our apps so I think at least right now it's safe to assume it works as advertised, and you're right--the gem maintainers should be testing it.
You don't have to test that it sends emails IMO, I at least never have... just make sure the smtp settings are correct.
There was a problem hiding this comment.
Okay, question about this. Tom setup exception_notifier with this config a looong time ago:
config.middleware.use ExceptionNotification::Rack,
:email => {
:email_prefix => "[ERROR] ",
:sender_address => %{"notifier" <[email protected]>},
:exception_recipients => %w{[email protected]}
},
:slack => {
:webhook_url => "https://hooks.slack.com/services/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",
:channel => "#probably-exceptions",
:additional_parameters => {
:icon_url => "http://image.jpg",
:mrkdwn => true
}
}
Compared to this which is duplicated from umaps with the names switched out:
GolfCartRentals::Application.config.middleware.use ExceptionNotification::Rack,
email: {
email_prefix: "Golf-Cart-Rentals Exception: ",
sender_address: %{"Golf-Cart-Rentals" <[email protected]>},
exception_recipients: %w{[email protected]}
}
smpte settings are the same, but we have these settings enabled on action mailer in golf cart app:
config.action_mailer.delivery_method = :smtp
config.action_mailer.perform_deliveries = true
config.action_mailer.raise_delivery_errors = true
The first two are defaults, but I'm not sure about raise_delivery_errors as I don't know anything about the mail server configuration.
Do we care about the slack hooks? Does sender address matter for anything other than display?
There was a problem hiding this comment.
I think sender address should be parkserv and I can't back that up.. not like we can reply to it.
Do we care about slack hooks? I don't know if that even works, but if it does, I would feel pretty neutral about it. I do NOT care for getting exception notifications from Slack. Especially since I check my email regularly and that's how all the other exception notifications come in. I just don't see the need. But whatever.
|
Why those travis errors?? |
Not really sure, I'm gonna add in the rails 5.2 update branch once you merge into master and I'll look into it if that doesn't fix the problem. |
| # Inventory api url | ||
| config.inventory_api_uri = 'https://rentalapi.parking.umass.edu/v1/' # not sure what this is yet | ||
| end | ||
|
|
||
| GolfCartRentals::Application.config.middleware.user ExceptionNotification::Rack, | ||
| GolfCartRentals::Application.config.middleware.use ExceptionNotification::Rack, |
There was a problem hiding this comment.
I think you can just use the config declared above, as in all our other apps. No need to re-declare the GolfCartRentals::Application here.
Adds exception notifier gem and removes old 500 page rescue handling.
fixes #412