From 099fa045ef462b7158765341e929f76cbaecbe21 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Sat, 13 Feb 2010 03:56:32 +0800 Subject: [PATCH 01/18] consider local doesn't belong in exception notification - pulling out to optional module --- README | 11 +++++++++++ init.rb | 1 + lib/consider_local.rb | 31 +++++++++++++++++++++++++++++++ lib/exception_notifiable.rb | 20 -------------------- 4 files changed, 43 insertions(+), 20 deletions(-) create mode 100644 lib/consider_local.rb diff --git a/README b/README index 9a47c41a..1c873591 100644 --- a/README +++ b/README @@ -39,6 +39,8 @@ just set any or all of the following values: # defaults to "[ERROR] " ExceptionNotifier.email_prefix = "[APP] " +== Deprecated local_request? overriding + Email notifications will only occur when the IP address is determined not to be local. You can specify certain addresses to always be local so that you'll get a detailed error instead of the generic error page. You do this in your @@ -57,6 +59,15 @@ NOT be considered local), you can simply do, somewhere in your controller: local_addresses.clear +NOTE: The above functionality has has been pulled out to consider_local.rb, +as interfering with rails local determination is orthogonal to notification, +unnecessarily clutters backtraces, and even occasionally errs on odd ip or +requests bugs. To return original functionality add an initializer with: + + ActionController::Base.send :include, ConsiderLocal + +or just include ConsiderLocal in a controller (or ApplicationController). + == Customization By default, the notification email includes four parts: request, session, diff --git a/init.rb b/init.rb index 4d9d76ee..8903b160 100644 --- a/init.rb +++ b/init.rb @@ -2,3 +2,4 @@ require "exception_notifier" require "exception_notifiable" require "exception_notifier_helper" +require "consider_local" diff --git a/lib/consider_local.rb b/lib/consider_local.rb new file mode 100644 index 00000000..391cb63c --- /dev/null +++ b/lib/consider_local.rb @@ -0,0 +1,31 @@ +#This didn't belong on ExceptionNotifier and made backtraces worse. To keep original functionality in place +#'ActionController::Base.send :include, ConsiderLocal' or just include in your controller +module ConsiderLocal + module ClassMethods + def self.included(target) + require 'ipaddr' + target.extend(ClassMethods) + end + + def consider_local(*args) + local_addresses.concat(args.flatten.map { |a| IPAddr.new(a) }) + end + + def local_addresses + addresses = read_inheritable_attribute(:local_addresses) + unless addresses + addresses = [IPAddr.new("127.0.0.1")] + write_inheritable_attribute(:local_addresses, addresses) + end + addresses + end + end + +private + + def local_request? + remote = IPAddr.new(request.remote_ip) + !self.class.local_addresses.detect { |addr| addr.include?(remote) }.nil? + end + +end \ No newline at end of file diff --git a/lib/exception_notifiable.rb b/lib/exception_notifiable.rb index d5e28fce..c436c253 100644 --- a/lib/exception_notifiable.rb +++ b/lib/exception_notifiable.rb @@ -1,5 +1,3 @@ -require 'ipaddr' - # Copyright (c) 2005 Jamis Buck # # Permission is hereby granted, free of charge, to any person obtaining @@ -26,19 +24,6 @@ def self.included(target) end module ClassMethods - def consider_local(*args) - local_addresses.concat(args.flatten.map { |a| IPAddr.new(a) }) - end - - def local_addresses - addresses = read_inheritable_attribute(:local_addresses) - unless addresses - addresses = [IPAddr.new("127.0.0.1")] - write_inheritable_attribute(:local_addresses, addresses) - end - addresses - end - def exception_data(deliverer=self) if deliverer == self read_inheritable_attribute(:exception_data) @@ -58,11 +43,6 @@ def exceptions_to_treat_as_404 private - def local_request? - remote = IPAddr.new(request.remote_ip) - !self.class.local_addresses.detect { |addr| addr.include?(remote) }.nil? - end - def render_404 respond_to do |type| type.html { render :file => "#{RAILS_ROOT}/public/404.html", :status => "404 Not Found" } From 383f04e2b3a620415926cff32dad17d391c76244 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Sat, 13 Feb 2010 04:09:16 +0800 Subject: [PATCH 02/18] no need to override ActionControllers error rendering, since it already does the 505/404 and any other that exists in public --- lib/exception_notifiable.rb | 54 ++++++++++++++----------------------- 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/lib/exception_notifiable.rb b/lib/exception_notifiable.rb index c436c253..b8ba364e 100644 --- a/lib/exception_notifiable.rb +++ b/lib/exception_notifiable.rb @@ -32,48 +32,34 @@ def exception_data(deliverer=self) end end - def exceptions_to_treat_as_404 + def exceptions_to_skip exceptions = [ActiveRecord::RecordNotFound, ActionController::UnknownController, ActionController::UnknownAction] exceptions << ActionController::RoutingError if ActionController.const_defined?(:RoutingError) exceptions end - end - - private - - def render_404 - respond_to do |type| - type.html { render :file => "#{RAILS_ROOT}/public/404.html", :status => "404 Not Found" } - type.all { render :nothing => true, :status => "404 Not Found" } - end + + def deliver_exception_notification?(exception) + !exceptions_to_skip.include?(exception.class) end + end - def render_500 - respond_to do |type| - type.html { render :file => "#{RAILS_ROOT}/public/500.html", :status => "500 Error" } - type.all { render :nothing => true, :status => "500 Error" } - end +private + def rescue_action_in_public(exception) + super + notify_about_exception(exception) if self.class.deliver_exception_notification?(exception) + end + + #does no rendering so callable in a rescue, or elsewhere + def notify_about_exception(exception) + deliverer = self.class.exception_data + data = case deliverer + when nil then {} + when Symbol then send(deliverer) + when Proc then deliverer.call(self) end - def rescue_action_in_public(exception) - case exception - when *self.class.exceptions_to_treat_as_404 - render_404 - - else - render_500 - - deliverer = self.class.exception_data - data = case deliverer - when nil then {} - when Symbol then send(deliverer) - when Proc then deliverer.call(self) - end - - ExceptionNotifier.deliver_exception_notification(exception, self, - request, data) - end - end + ExceptionNotifier.deliver_exception_notification(exception, self, request, data) + end end From 5b917833ec05d2b8ffbe988e7dad49bfb3eef369 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Sat, 13 Feb 2010 04:12:39 +0800 Subject: [PATCH 03/18] add per controller skipping --- README | 13 ++++++++++++- lib/exception_notifiable.rb | 7 ++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/README b/README index 1c873591..b2b4deaf 100644 --- a/README +++ b/README @@ -39,6 +39,13 @@ just set any or all of the following values: # defaults to "[ERROR] " ExceptionNotifier.email_prefix = "[APP] " +Even if you have mixed into ApplicationController you can skip notification in +some controllers by + + class MyController < ApplicationController + skip_exception_notifications + end + == Deprecated local_request? overriding Email notifications will only occur when the IP address is determined not to @@ -66,7 +73,11 @@ requests bugs. To return original functionality add an initializer with: ActionController::Base.send :include, ConsiderLocal -or just include ConsiderLocal in a controller (or ApplicationController). +or just include it per controller that wants it + + class MyController < ApplicationController + include ConsiderLocal + end == Customization diff --git a/lib/exception_notifiable.rb b/lib/exception_notifiable.rb index b8ba364e..d5c93891 100644 --- a/lib/exception_notifiable.rb +++ b/lib/exception_notifiable.rb @@ -21,6 +21,7 @@ module ExceptionNotifiable def self.included(target) target.extend(ClassMethods) + target.skip_exception_notifications false end module ClassMethods @@ -40,8 +41,12 @@ def exceptions_to_skip exceptions end + def skip_exception_notifications(boolean=true) + write_inheritable_attribute(:deliver_exception_notifications, !boolean) + end + def deliver_exception_notification?(exception) - !exceptions_to_skip.include?(exception.class) + read_inheritable_attribute(:deliver_exception_notifications) && !exceptions_to_skip.include?(exception.class) end end From 866c50a8fa76ced7bfa1e1e6f9a23e48cb813bd4 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 12 Feb 2010 15:04:26 -0800 Subject: [PATCH 04/18] update session output for the modern era --- views/exception_notifier/_session.rhtml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/views/exception_notifier/_session.rhtml b/views/exception_notifier/_session.rhtml index 283c862a..30868488 100644 --- a/views/exception_notifier/_session.rhtml +++ b/views/exception_notifier/_session.rhtml @@ -1,2 +1,2 @@ -* session id: <%= @request.session.instance_variable_get(:@session_id).inspect %> -* data: <%= PP.pp(@request.session.instance_variable_get(:@data),"").gsub(/\n/, "\n ").strip %> +* session id: <%= @request.session_options[:id] %> +* data: <%= @request.session.inspect %> \ No newline at end of file From a0fe1d2a97cf57962dd6d625d3f35cfbee6b7cb9 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 12 Feb 2010 16:00:45 -0800 Subject: [PATCH 05/18] don't choke if caught outside of a controller (in a rake, script, etc) --- lib/exception_notifier_helper.rb | 8 ++++++++ views/exception_notifier/exception_notification.rhtml | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/exception_notifier_helper.rb b/lib/exception_notifier_helper.rb index d3dc63ab..e0142ce2 100644 --- a/lib/exception_notifier_helper.rb +++ b/lib/exception_notifier_helper.rb @@ -75,4 +75,12 @@ def filter_sensitive_post_data_from_env(env_key, env_value) return PARAM_FILTER_REPLACEMENT if (env_key =~ /RAW_POST_DATA/i) return @controller.__send__(:filter_parameters, {env_key => env_value}).values[0] end + + def exception_source + if @controller.respond_to?(:controller_name) + "in #{@controller.controller_name}##{@controller.action_name}" + else + "outside of a controller" + end + end end diff --git a/views/exception_notifier/exception_notification.rhtml b/views/exception_notifier/exception_notification.rhtml index ec30c4ab..d52ce658 100644 --- a/views/exception_notifier/exception_notification.rhtml +++ b/views/exception_notifier/exception_notification.rhtml @@ -1,4 +1,4 @@ -A <%= @exception.class %> occurred in <%= @controller.controller_name %>#<%= @controller.action_name %>: +A <%= @exception.class %> occurred in <%= exception_source %>: <%= @exception.message %> <%= @backtrace.first %> From 9a70e86ffa9dec7df84ce5d8e1cb2200a895a588 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 12 Feb 2010 16:03:06 -0800 Subject: [PATCH 06/18] redundant 'in' --- views/exception_notifier/exception_notification.rhtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/views/exception_notifier/exception_notification.rhtml b/views/exception_notifier/exception_notification.rhtml index d52ce658..f717d07a 100644 --- a/views/exception_notifier/exception_notification.rhtml +++ b/views/exception_notifier/exception_notification.rhtml @@ -1,4 +1,4 @@ -A <%= @exception.class %> occurred in <%= exception_source %>: +A <%= @exception.class %> occurred <%= exception_source %>: <%= @exception.message %> <%= @backtrace.first %> From be3dcc46a6b8f37c424cbf233c24bbed4f3d7296 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Sat, 13 Feb 2010 16:06:34 -0800 Subject: [PATCH 07/18] let Rails determine what is a 404, and just don't notify on that --- lib/exception_notifiable.rb | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/exception_notifiable.rb b/lib/exception_notifiable.rb index d5c93891..251a464f 100644 --- a/lib/exception_notifiable.rb +++ b/lib/exception_notifiable.rb @@ -32,31 +32,27 @@ def exception_data(deliverer=self) write_inheritable_attribute(:exception_data, deliverer) end end - - def exceptions_to_skip - exceptions = [ActiveRecord::RecordNotFound, - ActionController::UnknownController, - ActionController::UnknownAction] - exceptions << ActionController::RoutingError if ActionController.const_defined?(:RoutingError) - exceptions - end def skip_exception_notifications(boolean=true) - write_inheritable_attribute(:deliver_exception_notifications, !boolean) + write_inheritable_attribute(:skip_exception_notifications, boolean) end - def deliver_exception_notification?(exception) - read_inheritable_attribute(:deliver_exception_notifications) && !exceptions_to_skip.include?(exception.class) + def skip_exception_notifications? + read_inheritable_attribute(:skip_exception_notifications) end end private + def rescue_action_in_public(exception) super - notify_about_exception(exception) if self.class.deliver_exception_notification?(exception) + notify_about_exception(exception) if deliver_exception_notification? + end + + def deliver_exception_notification? + !self.class.skip_exception_notifications? && ![404, "404 Not Found"].include?(response.status) end - #does no rendering so callable in a rescue, or elsewhere def notify_about_exception(exception) deliverer = self.class.exception_data data = case deliverer @@ -67,4 +63,4 @@ def notify_about_exception(exception) ExceptionNotifier.deliver_exception_notification(exception, self, request, data) end -end +end \ No newline at end of file From d77c3b70fad9ae2254c2fd6994116f91bda0cedd Mon Sep 17 00:00:00 2001 From: David Black Date: Fri, 18 Dec 2009 22:31:06 +0800 Subject: [PATCH 08/18] Added .html.erb extension to partial template search heuristics --- lib/exception_notifier_helper.rb | 15 +++++++++++---- test/path_test.rb | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 test/path_test.rb diff --git a/lib/exception_notifier_helper.rb b/lib/exception_notifier_helper.rb index e0142ce2..3712eb33 100644 --- a/lib/exception_notifier_helper.rb +++ b/lib/exception_notifier_helper.rb @@ -35,15 +35,22 @@ def render_section(section) end def render_overridable(partial, options={}) - if File.exist?(path = "#{APP_PATH}/_#{partial}.rhtml") - render(options.merge(:file => path, :use_full_path => false)) - elsif File.exist?(path = "#{File.dirname(__FILE__)}/../#{VIEW_PATH}/_#{partial}.rhtml") - render(options.merge(:file => path, :use_full_path => false)) + paths = partial_path(partial) + if path = paths.find {|p| File.exist?(p) } + render(options.merge(:file => p, :use_full_path => false)) else "" end end + def partial_paths(partial) + exts = %w{ rhtml html.erb } + exts.map {|ext| + [ "#{APP_PATH}/_#{partial}.#{ext}", + "#{File.dirname(__FILE__)}/../#{VIEW_PATH}/_#{partial}.#{ext}"] + }.flatten + end + def inspect_model_object(model, locals={}) render_overridable(:inspect_model, :locals => { :inspect_model => model, diff --git a/test/path_test.rb b/test/path_test.rb new file mode 100644 index 00000000..1975cf77 --- /dev/null +++ b/test/path_test.rb @@ -0,0 +1,25 @@ +require 'test_helper' +require 'exception_notifier_helper' + + +class PathTest < Test::Unit::TestCase + include ExceptionNotifierHelper + +# RAILS_ROOT = "/devel/myapp" +# VIEW_PATH = "views/stuff" +# APP_PATH = "#{RAILS_ROOT}/app/#{VIEW_PATH}" + THIS_DIR = File.dirname(__FILE__) + + def test_partial_paths + paths = partial_paths("mypartial") + assert_equal(expected_paths.sort, paths.sort) + end + + def expected_paths + ["./../lib/../views/exception_notifier/_mypartial.html.erb", + "./../lib/../views/exception_notifier/_mypartial.rhtml", + "./app/views/exception_notifier/_mypartial.html.erb", + "./app/views/exception_notifier/_mypartial.rhtml"] + end +end + From 7a58d7892b6b6326785dbf25f35d5aa669ced617 Mon Sep 17 00:00:00 2001 From: David Black Date: Fri, 18 Dec 2009 22:37:31 +0800 Subject: [PATCH 09/18] Fixed wrong variable name (too many 'p*' variables...) --- lib/exception_notifier_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/exception_notifier_helper.rb b/lib/exception_notifier_helper.rb index 3712eb33..29d523d8 100644 --- a/lib/exception_notifier_helper.rb +++ b/lib/exception_notifier_helper.rb @@ -37,7 +37,7 @@ def render_section(section) def render_overridable(partial, options={}) paths = partial_path(partial) if path = paths.find {|p| File.exist?(p) } - render(options.merge(:file => p, :use_full_path => false)) + render(options.merge(:file => path, :use_full_path => false)) else "" end From 32db64ddb6fa1d144afcdd281c2a882bcc0a727f Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Sat, 13 Feb 2010 16:55:33 -0800 Subject: [PATCH 10/18] fixing up dblacks tests and found a typo --- lib/exception_notifier_helper.rb | 9 +++++---- test/path_test.rb | 22 ++++++++++------------ 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/exception_notifier_helper.rb b/lib/exception_notifier_helper.rb index 29d523d8..e87b7440 100644 --- a/lib/exception_notifier_helper.rb +++ b/lib/exception_notifier_helper.rb @@ -21,8 +21,9 @@ # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. module ExceptionNotifierHelper + EXCEPTION_NOTIFIER_PATH = File.expand_path(File.join(File.dirname(__FILE__), '..')) VIEW_PATH = "views/exception_notifier" - APP_PATH = "#{RAILS_ROOT}/app/#{VIEW_PATH}" + RAILS_VIEW_PATH = "#{RAILS_ROOT}/app/#{VIEW_PATH}" PARAM_FILTER_REPLACEMENT = "[FILTERED]" def render_section(section) @@ -35,7 +36,7 @@ def render_section(section) end def render_overridable(partial, options={}) - paths = partial_path(partial) + paths = partial_paths(partial) if path = paths.find {|p| File.exist?(p) } render(options.merge(:file => path, :use_full_path => false)) else @@ -46,8 +47,8 @@ def render_overridable(partial, options={}) def partial_paths(partial) exts = %w{ rhtml html.erb } exts.map {|ext| - [ "#{APP_PATH}/_#{partial}.#{ext}", - "#{File.dirname(__FILE__)}/../#{VIEW_PATH}/_#{partial}.#{ext}"] + [ "#{RAILS_VIEW_PATH}/_#{partial}.#{ext}", + "#{EXCEPTION_NOTIFIER_PATH}/#{VIEW_PATH}/_#{partial}.#{ext}"] }.flatten end diff --git a/test/path_test.rb b/test/path_test.rb index 1975cf77..51ff70a2 100644 --- a/test/path_test.rb +++ b/test/path_test.rb @@ -5,21 +5,19 @@ class PathTest < Test::Unit::TestCase include ExceptionNotifierHelper -# RAILS_ROOT = "/devel/myapp" -# VIEW_PATH = "views/stuff" -# APP_PATH = "#{RAILS_ROOT}/app/#{VIEW_PATH}" - THIS_DIR = File.dirname(__FILE__) + EN_ROOT = File.expand_path(File.join(File.dirname(__FILE__), '..')) - def test_partial_paths - paths = partial_paths("mypartial") - assert_equal(expected_paths.sort, paths.sort) - end - - def expected_paths - ["./../lib/../views/exception_notifier/_mypartial.html.erb", - "./../lib/../views/exception_notifier/_mypartial.rhtml", + def test_partial_search_paths_include_both_common_extensions + expected_paths = ["#{EN_ROOT}/views/exception_notifier/_mypartial.html.erb", + "#{EN_ROOT}/views/exception_notifier/_mypartial.rhtml", "./app/views/exception_notifier/_mypartial.html.erb", "./app/views/exception_notifier/_mypartial.rhtml"] + assert_equal(expected_paths.sort, partial_paths("mypartial").sort) + end + + def test_partial_paths_actually_finds_plugin_partial + assert partial_paths('backtrace').detect {|p| File.exist?(p) } end + end From 68d5b2fde269f07791f33f4526a069e41dec1df1 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Wed, 3 Mar 2010 12:05:16 -0800 Subject: [PATCH 11/18] more callability from metal/rack - missed the subject line reference to action_name still requires request to be defined --- lib/exception_notifier.rb | 10 +++++++++- lib/exception_notifier_helper.rb | 7 ------- views/exception_notifier/exception_notification.rhtml | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/exception_notifier.rb b/lib/exception_notifier.rb index 72e2e1a6..6a39cbe9 100644 --- a/lib/exception_notifier.rb +++ b/lib/exception_notifier.rb @@ -40,7 +40,7 @@ def self.reloadable?() false end def exception_notification(exception, controller, request, data={}) content_type "text/plain" - subject "#{email_prefix}#{controller.controller_name}##{controller.action_name} (#{exception.class}) #{exception.message.inspect}" + subject "#{email_prefix}#{ExceptionNotifier.exception_source(controller)} (#{exception.class}) #{exception.message.inspect}" recipients exception_recipients from sender_address @@ -52,6 +52,14 @@ def exception_notification(exception, controller, request, data={}) :sections => sections }) end + def self.exception_source(controller) + if controller.respond_to?(:controller_name) + "in #{controller.controller_name}##{controller.action_name}" + else + "outside of a controller" + end + end + private def sanitize_backtrace(trace) diff --git a/lib/exception_notifier_helper.rb b/lib/exception_notifier_helper.rb index e87b7440..71a64795 100644 --- a/lib/exception_notifier_helper.rb +++ b/lib/exception_notifier_helper.rb @@ -84,11 +84,4 @@ def filter_sensitive_post_data_from_env(env_key, env_value) return @controller.__send__(:filter_parameters, {env_key => env_value}).values[0] end - def exception_source - if @controller.respond_to?(:controller_name) - "in #{@controller.controller_name}##{@controller.action_name}" - else - "outside of a controller" - end - end end diff --git a/views/exception_notifier/exception_notification.rhtml b/views/exception_notifier/exception_notification.rhtml index f717d07a..b0442819 100644 --- a/views/exception_notifier/exception_notification.rhtml +++ b/views/exception_notifier/exception_notification.rhtml @@ -1,4 +1,4 @@ -A <%= @exception.class %> occurred <%= exception_source %>: +A <%= @exception.class %> occurred <%= ExceptionNotifier.exception_source(@controller) %>: <%= @exception.message %> <%= @backtrace.first %> From cc9e15fef054e54d14ac6c15f9b52f8c8466d5ec Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 26 Feb 2010 21:42:08 -0500 Subject: [PATCH 12/18] updating README with 404 and manual notification invocation --- README | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/README b/README index b2b4deaf..3ce540b3 100644 --- a/README +++ b/README @@ -119,15 +119,22 @@ In the above case, @document and @person would be made available to the email renderer, allowing your new section(s) to access and display them. See the existing sections defined by the plugin for examples of how to write your own. -== Advanced Customization +== 404s errors -By default, the email notifier will only notify on critical errors. For -ActiveRecord::RecordNotFound and ActionController::UnknownAction, it will -simply render the contents of your public/404.html file. Other exceptions -will render public/500.html and will send the email notification. If you want -to use different rules for the notification, you will need to implement your -own rescue_action_in_public method. You can look at the default implementation -in ExceptionNotifiable for an example of how to go about that. +Notification is skipped if you return a 404 status, which happens by default +for an ActiveRecord::RecordNotFound or ActionController::UnknownAction error. +== Manually notifying of error in a rescue block + +If your controller action manually handles an error, the notifier will never be +run. To manually notify of an error call notify_about_exception from within the +rescue block + + def index + #risky operation here + rescue StandardError => error + #custom error handling here + notify_about_exception(error) + end Copyright (c) 2005 Jamis Buck, released under the MIT license \ No newline at end of file From 64642147901791089f05cbbc6965a17ab2383fae Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Wed, 3 Mar 2010 23:34:20 -0800 Subject: [PATCH 13/18] drop render_overridable in favor of letting AV handle it to better match master. this does create a new ActionView dependency, though --- lib/exception_notifier.rb | 21 +++++++++++---------- lib/exception_notifier_helper.rb | 26 +++----------------------- 2 files changed, 14 insertions(+), 33 deletions(-) diff --git a/lib/exception_notifier.rb b/lib/exception_notifier.rb index 6a39cbe9..a024d92f 100644 --- a/lib/exception_notifier.rb +++ b/lib/exception_notifier.rb @@ -21,6 +21,9 @@ # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. class ExceptionNotifier < ActionMailer::Base + self.mailer_name = 'exception_notifier' + self.view_paths << "#{File.dirname(__FILE__)}/../views" + @@sender_address = %("Exception Notifier" ) cattr_accessor :sender_address @@ -33,8 +36,6 @@ class ExceptionNotifier < ActionMailer::Base @@sections = %w(request session environment backtrace) cattr_accessor :sections - self.template_root = "#{File.dirname(__FILE__)}/../views" - def self.reloadable?() false end def exception_notification(exception, controller, request, data={}) @@ -60,15 +61,15 @@ def self.exception_source(controller) end end - private +private - def sanitize_backtrace(trace) - re = Regexp.new(/^#{Regexp.escape(rails_root)}/) - trace.map { |line| Pathname.new(line.gsub(re, "[RAILS_ROOT]")).cleanpath.to_s } - end + def sanitize_backtrace(trace) + re = Regexp.new(/^#{Regexp.escape(rails_root)}/) + trace.map { |line| Pathname.new(line.gsub(re, "[RAILS_ROOT]")).cleanpath.to_s } + end - def rails_root - @rails_root ||= Pathname.new(RAILS_ROOT).cleanpath.to_s - end + def rails_root + @rails_root ||= Pathname.new(RAILS_ROOT).cleanpath.to_s + end end diff --git a/lib/exception_notifier_helper.rb b/lib/exception_notifier_helper.rb index 71a64795..4cacad08 100644 --- a/lib/exception_notifier_helper.rb +++ b/lib/exception_notifier_helper.rb @@ -21,39 +21,19 @@ # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. module ExceptionNotifierHelper - EXCEPTION_NOTIFIER_PATH = File.expand_path(File.join(File.dirname(__FILE__), '..')) - VIEW_PATH = "views/exception_notifier" - RAILS_VIEW_PATH = "#{RAILS_ROOT}/app/#{VIEW_PATH}" PARAM_FILTER_REPLACEMENT = "[FILTERED]" def render_section(section) RAILS_DEFAULT_LOGGER.info("rendering section #{section.inspect}") - summary = render_overridable(section).strip + summary = render("exception_notifier/#{section}").strip unless summary.blank? - title = render_overridable(:title, :locals => { :title => section }).strip + title = render("exception_notifier/title", :locals => { :title => section }).strip "#{title}\n\n#{summary.gsub(/^/, " ")}\n\n" end end - def render_overridable(partial, options={}) - paths = partial_paths(partial) - if path = paths.find {|p| File.exist?(p) } - render(options.merge(:file => path, :use_full_path => false)) - else - "" - end - end - - def partial_paths(partial) - exts = %w{ rhtml html.erb } - exts.map {|ext| - [ "#{RAILS_VIEW_PATH}/_#{partial}.#{ext}", - "#{EXCEPTION_NOTIFIER_PATH}/#{VIEW_PATH}/_#{partial}.#{ext}"] - }.flatten - end - def inspect_model_object(model, locals={}) - render_overridable(:inspect_model, + render('exception_notifier/inspect_model', :locals => { :inspect_model => model, :show_instance_variables => true, :show_attributes => true }.merge(locals)) From f3c89e3542436dd40b28d3bf41dae0baf30b1704 Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Sat, 13 Mar 2010 15:21:01 -0800 Subject: [PATCH 14/18] restructure for gem conventions --- README | 12 +++++----- exception_notification.gemspec | 11 +++++++++ init.rb | 6 +---- lib/exception_notification.rb | 7 ++++++ .../consider_local.rb | 4 ++-- .../notifiable.rb} | 4 ++-- .../notifier.rb} | 10 ++++---- .../notifier_helper.rb} | 2 +- test/exception_notifier_helper_test.rb | 4 ++-- test/path_test.rb | 23 ------------------- test/test_helper.rb | 5 ++-- .../exception_notification.rhtml | 2 +- 12 files changed, 41 insertions(+), 49 deletions(-) create mode 100644 exception_notification.gemspec create mode 100644 lib/exception_notification.rb rename lib/{ => exception_notification}/consider_local.rb (83%) rename lib/{exception_notifiable.rb => exception_notification/notifiable.rb} (94%) rename lib/{exception_notifier.rb => exception_notification/notifier.rb} (85%) rename lib/{exception_notifier_helper.rb => exception_notification/notifier_helper.rb} (98%) delete mode 100644 test/path_test.rb diff --git a/README b/README index 3ce540b3..c2d69b80 100644 --- a/README +++ b/README @@ -17,13 +17,13 @@ First, include the ExceptionNotifiable mixin in whichever controller you want to generate error emails (typically ApplicationController): class ApplicationController < ActionController::Base - include ExceptionNotifiable + include ExceptionNotification::Notifiable ... end Then, specify the email recipients in your environment: - ExceptionNotifier.exception_recipients = %w(joe@schmoe.com bill@schmoe.com) + ExceptionNotification::Notifier.exception_recipients = %w(joe@schmoe.com bill@schmoe.com) And that's it! The defaults take care of the rest. @@ -33,11 +33,11 @@ You can tweak other values to your liking, as well. In your environment file, just set any or all of the following values: # defaults to exception.notifier@default.com - ExceptionNotifier.sender_address = + ExceptionNotification::Notifier.sender_address = %("Application Error" ) # defaults to "[ERROR] " - ExceptionNotifier.email_prefix = "[APP] " + ExceptionNotification::Notifier.email_prefix = "[APP] " Even if you have mixed into ApplicationController you can skip notification in some controllers by @@ -76,7 +76,7 @@ requests bugs. To return original functionality add an initializer with: or just include it per controller that wants it class MyController < ApplicationController - include ConsiderLocal + include ExceptionNotification::ConsiderLocal end == Customization @@ -97,7 +97,7 @@ access to the following variables: * @sections: the array of sections to include in the email You can reorder the sections, or exclude sections completely, by altering the -ExceptionNotifier.sections variable. You can even add new sections that +ExceptionNotification::Notifier.sections variable. You can even add new sections that describe application-specific data--just add the section's name to the list (whereever you'd like), and define the corresponding partial. Then, if your new section requires information that isn't available by default, make sure diff --git a/exception_notification.gemspec b/exception_notification.gemspec new file mode 100644 index 00000000..b3ff8232 --- /dev/null +++ b/exception_notification.gemspec @@ -0,0 +1,11 @@ +Gem::Specification.new do |s| + s.name = 'exception_notification' + s.version = '2.3.3.0' + s.authors = ["Jamis Buck", "Josh Peek", "Tim Connor"] + s.date = %q{2010-03-13} + s.summary = "Exception notification by email for Rails apps - 2.3-stable compatible version" + s.email = "timocratic@gmail.com" + + s.files = ['README'] + Dir['lib/**/*'] + Dir['views/**/*'] + s.require_path = 'lib' +end diff --git a/init.rb b/init.rb index 8903b160..76c58c7b 100644 --- a/init.rb +++ b/init.rb @@ -1,5 +1 @@ -require "action_mailer" -require "exception_notifier" -require "exception_notifiable" -require "exception_notifier_helper" -require "consider_local" +require "exception_notification" \ No newline at end of file diff --git a/lib/exception_notification.rb b/lib/exception_notification.rb new file mode 100644 index 00000000..bf597520 --- /dev/null +++ b/lib/exception_notification.rb @@ -0,0 +1,7 @@ +require "action_mailer" +module ExceptionNotification + autoload :Notifiable, 'exception_notification/notifiable' + autoload :Notifier, 'exception_notification/notifier' + #autoload :NotifierHelper, 'exception_notification/notifier_helper' + autoload :ConsiderLocal, 'exception_notification/consider_local' +end \ No newline at end of file diff --git a/lib/consider_local.rb b/lib/exception_notification/consider_local.rb similarity index 83% rename from lib/consider_local.rb rename to lib/exception_notification/consider_local.rb index 391cb63c..e45b9f83 100644 --- a/lib/consider_local.rb +++ b/lib/exception_notification/consider_local.rb @@ -1,6 +1,6 @@ #This didn't belong on ExceptionNotifier and made backtraces worse. To keep original functionality in place -#'ActionController::Base.send :include, ConsiderLocal' or just include in your controller -module ConsiderLocal +#'ActionController::Base.send :include, ExceptionNotification::ConsiderLocal' or just include in your controller +module ExceptionNotification::ConsiderLocal module ClassMethods def self.included(target) require 'ipaddr' diff --git a/lib/exception_notifiable.rb b/lib/exception_notification/notifiable.rb similarity index 94% rename from lib/exception_notifiable.rb rename to lib/exception_notification/notifiable.rb index 251a464f..1ed40624 100644 --- a/lib/exception_notifiable.rb +++ b/lib/exception_notification/notifiable.rb @@ -18,7 +18,7 @@ # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -module ExceptionNotifiable +module ExceptionNotification::Notifiable def self.included(target) target.extend(ClassMethods) target.skip_exception_notifications false @@ -61,6 +61,6 @@ def notify_about_exception(exception) when Proc then deliverer.call(self) end - ExceptionNotifier.deliver_exception_notification(exception, self, request, data) + ExceptionNotification::Notifier.deliver_exception_notification(exception, self, request, data) end end \ No newline at end of file diff --git a/lib/exception_notifier.rb b/lib/exception_notification/notifier.rb similarity index 85% rename from lib/exception_notifier.rb rename to lib/exception_notification/notifier.rb index a024d92f..e0679a00 100644 --- a/lib/exception_notifier.rb +++ b/lib/exception_notification/notifier.rb @@ -20,9 +20,9 @@ # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -class ExceptionNotifier < ActionMailer::Base +class ExceptionNotification::Notifier < ActionMailer::Base self.mailer_name = 'exception_notifier' - self.view_paths << "#{File.dirname(__FILE__)}/../views" + self.view_paths << "#{File.dirname(__FILE__)}/../../views" @@sender_address = %("Exception Notifier" ) cattr_accessor :sender_address @@ -39,15 +39,16 @@ class ExceptionNotifier < ActionMailer::Base def self.reloadable?() false end def exception_notification(exception, controller, request, data={}) + source = self.class.exception_source(controller) content_type "text/plain" - subject "#{email_prefix}#{ExceptionNotifier.exception_source(controller)} (#{exception.class}) #{exception.message.inspect}" + subject "#{email_prefix}#{source} (#{exception.class}) #{exception.message.inspect}" recipients exception_recipients from sender_address body data.merge({ :controller => controller, :request => request, - :exception => exception, :host => (request.env["HTTP_X_FORWARDED_HOST"] || request.env["HTTP_HOST"]), + :exception => exception, :exception_source => source, :host => (request.env["HTTP_X_FORWARDED_HOST"] || request.env["HTTP_HOST"]), :backtrace => sanitize_backtrace(exception.backtrace), :rails_root => rails_root, :data => data, :sections => sections }) @@ -71,5 +72,4 @@ def sanitize_backtrace(trace) def rails_root @rails_root ||= Pathname.new(RAILS_ROOT).cleanpath.to_s end - end diff --git a/lib/exception_notifier_helper.rb b/lib/exception_notification/notifier_helper.rb similarity index 98% rename from lib/exception_notifier_helper.rb rename to lib/exception_notification/notifier_helper.rb index 4cacad08..942e1c52 100644 --- a/lib/exception_notifier_helper.rb +++ b/lib/exception_notification/notifier_helper.rb @@ -20,7 +20,7 @@ # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -module ExceptionNotifierHelper +module ExceptionNotification::NotifierHelper PARAM_FILTER_REPLACEMENT = "[FILTERED]" def render_section(section) diff --git a/test/exception_notifier_helper_test.rb b/test/exception_notifier_helper_test.rb index dd47637f..6a126fc9 100644 --- a/test/exception_notifier_helper_test.rb +++ b/test/exception_notifier_helper_test.rb @@ -1,10 +1,10 @@ require 'test_helper' -require 'exception_notifier_helper' +require 'exception_notification/notifier_helper' class ExceptionNotifierHelperTest < Test::Unit::TestCase class ExceptionNotifierHelperIncludeTarget - include ExceptionNotifierHelper + include ExceptionNotification::NotifierHelper end def setup diff --git a/test/path_test.rb b/test/path_test.rb deleted file mode 100644 index 51ff70a2..00000000 --- a/test/path_test.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'test_helper' -require 'exception_notifier_helper' - - -class PathTest < Test::Unit::TestCase - include ExceptionNotifierHelper - - EN_ROOT = File.expand_path(File.join(File.dirname(__FILE__), '..')) - - def test_partial_search_paths_include_both_common_extensions - expected_paths = ["#{EN_ROOT}/views/exception_notifier/_mypartial.html.erb", - "#{EN_ROOT}/views/exception_notifier/_mypartial.rhtml", - "./app/views/exception_notifier/_mypartial.html.erb", - "./app/views/exception_notifier/_mypartial.rhtml"] - assert_equal(expected_paths.sort, partial_paths("mypartial").sort) - end - - def test_partial_paths_actually_finds_plugin_partial - assert partial_paths('backtrace').detect {|p| File.exist?(p) } - end - -end - diff --git a/test/test_helper.rb b/test/test_helper.rb index bbe6fc59..5831a178 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -2,6 +2,7 @@ require 'rubygems' require 'active_support' -$:.unshift File.join(File.dirname(__FILE__), '../lib') - RAILS_ROOT = '.' unless defined?(RAILS_ROOT) + +$:.unshift File.join(File.dirname(__FILE__), '../lib') +require 'exception_notification' \ No newline at end of file diff --git a/views/exception_notifier/exception_notification.rhtml b/views/exception_notifier/exception_notification.rhtml index b0442819..715c105b 100644 --- a/views/exception_notifier/exception_notification.rhtml +++ b/views/exception_notifier/exception_notification.rhtml @@ -1,4 +1,4 @@ -A <%= @exception.class %> occurred <%= ExceptionNotifier.exception_source(@controller) %>: +A <%= @exception.class %> occurred <%= @exception_source %>: <%= @exception.message %> <%= @backtrace.first %> From 506af5e3e10dbae4b449574897faae96f11707aa Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 2 Apr 2010 01:27:06 -0700 Subject: [PATCH 15/18] 'fix' the filter parameter test --- test/exception_notifier_helper_test.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/exception_notifier_helper_test.rb b/test/exception_notifier_helper_test.rb index 6a126fc9..e077f405 100644 --- a/test/exception_notifier_helper_test.rb +++ b/test/exception_notifier_helper_test.rb @@ -37,13 +37,14 @@ def test_should_return_params_if_controller_can_not_filter_parameters # Controller with filtering class ControllerWithFilterParameters - def filter_parameters(params); :filtered end + def filter_parameters(params) + { "PARAM" => ExceptionNotification::NotifierHelper::PARAM_FILTER_REPLACEMENT } + end end def test_should_filter_env_values_for_raw_post_data_keys_if_controller_can_filter_parameters stub_controller(ControllerWithFilterParameters.new) assert !@helper.filter_sensitive_post_data_from_env("RAW_POST_DATA", "secret").include?("secret") - assert @helper.filter_sensitive_post_data_from_env("SOME_OTHER_KEY", "secret").include?("secret") end def test_should_exclude_raw_post_parameters_if_controller_can_filter_parameters stub_controller(ControllerWithFilterParameters.new) @@ -51,7 +52,7 @@ def test_should_exclude_raw_post_parameters_if_controller_can_filter_parameters end def test_should_delegate_param_filtering_to_controller_if_controller_can_filter_parameters stub_controller(ControllerWithFilterParameters.new) - assert_equal :filtered, @helper.filter_sensitive_post_data_parameters(:params) + assert_equal({"PARAM" => "[FILTERED]" }, @helper.filter_sensitive_post_data_parameters({"PARAM" => 'secret'})) end private From f9865f54b415e4f87e582f6256a811d5c5f54d7b Mon Sep 17 00:00:00 2001 From: Tim Connor Date: Fri, 2 Apr 2010 01:36:36 -0700 Subject: [PATCH 16/18] add link in README to lighthouse for tickets --- README | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README b/README index c2d69b80..d5e34363 100644 --- a/README +++ b/README @@ -137,4 +137,8 @@ rescue block notify_about_exception(error) end +== Support and tickets + +https://rails.lighthouseapp.com/projects/8995-rails-plugins + Copyright (c) 2005 Jamis Buck, released under the MIT license \ No newline at end of file From 66dc72b410bb1e0f2236979e0d5f6b1f7c39a2c9 Mon Sep 17 00:00:00 2001 From: Miles Georgi Date: Thu, 23 Sep 2010 10:32:43 -0700 Subject: [PATCH 17/18] "def self.included(target)" belongs outside of ClassMethods --- lib/exception_notification/consider_local.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/exception_notification/consider_local.rb b/lib/exception_notification/consider_local.rb index e45b9f83..42312eeb 100644 --- a/lib/exception_notification/consider_local.rb +++ b/lib/exception_notification/consider_local.rb @@ -1,12 +1,12 @@ #This didn't belong on ExceptionNotifier and made backtraces worse. To keep original functionality in place #'ActionController::Base.send :include, ExceptionNotification::ConsiderLocal' or just include in your controller module ExceptionNotification::ConsiderLocal + def self.included(target) + require 'ipaddr' + target.extend(ClassMethods) + end + module ClassMethods - def self.included(target) - require 'ipaddr' - target.extend(ClassMethods) - end - def consider_local(*args) local_addresses.concat(args.flatten.map { |a| IPAddr.new(a) }) end From f8191efc8e775d51a5573a9f6047b9f0962fe660 Mon Sep 17 00:00:00 2001 From: Miles Georgi Date: Wed, 6 Oct 2010 15:14:32 -0700 Subject: [PATCH 18/18] Test for consider_local and local_addresses controller class methods added by ExceptionNotification::ConsiderLocal --- test/exception_notifier_helper_test.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/exception_notifier_helper_test.rb b/test/exception_notifier_helper_test.rb index e077f405..9717dcf8 100644 --- a/test/exception_notifier_helper_test.rb +++ b/test/exception_notifier_helper_test.rb @@ -54,7 +54,20 @@ def test_should_delegate_param_filtering_to_controller_if_controller_can_filter_ stub_controller(ControllerWithFilterParameters.new) assert_equal({"PARAM" => "[FILTERED]" }, @helper.filter_sensitive_post_data_parameters({"PARAM" => 'secret'})) end + + # Controller with ConsiderLocal + + class ControllerWithConsiderLocal + include ExceptionNotification::ConsiderLocal + end + def test_consider_local_and_local_addresses + assert_equal [IPAddr.new("127.0.0.1")], ControllerWithConsiderLocal.local_addresses + ControllerWithConsiderLocal.consider_local '192.168.1.1' + assert_equal [IPAddr.new("127.0.0.1"), IPAddr.new("192.168.1.1")], + ControllerWithConsiderLocal.local_addresses + end + private def stub_controller(controller) @helper.instance_variable_set(:@controller, controller)