Permalink
Browse files

Merge branch 'seperate-resque-web-ignored-errors'

* seperate-resque-web-ignored-errors:
  seperate out the resque and web ignored errors
  • Loading branch information...
2 parents b093cd6 + bb34e20 commit ff431d60a1704c1d55bf1cbd00f4839f75724244 @nolman nolman committed Sep 30, 2011
Showing with 116 additions and 41 deletions.
  1. +12 −1 lib/hoptoad_notifier/configuration.rb
  2. +24 −2 lib/hoptoad_notifier/notice.rb
  3. +14 −1 test/configuration_test.rb
  4. +6 −0 test/helper.rb
  5. +60 −37 test/notice_test.rb
@@ -4,7 +4,7 @@ class Configuration
OPTIONS = [:api_key, :backtrace_filters, :development_environments,
:development_lookup, :environment_name, :host,
- :http_open_timeout, :http_read_timeout, :ignore, :ignore_by_filters,
+ :http_open_timeout, :http_read_timeout, :ignore, :worker_ignore, :ignore_by_filters,
:ignore_user_agent, :notifier_name, :notifier_url, :notifier_version,
:params_filters, :project_root, :port, :protocol, :proxy_host,
:proxy_pass, :proxy_port, :proxy_user, :secure, :framework,
@@ -54,6 +54,9 @@ class Configuration
# A list of exception classes to ignore. The array can be appended to.
attr_reader :ignore
+ #A list of exception classes to ignore in the worker processes.
@tylerkovacs

tylerkovacs Sep 30, 2011

Whitespace use should be consistent with other comments in the file:

A list

should be

A list

+ attr_reader :worker_ignore
+
# A list of user agents that are being ignored. The array can be appended to.
attr_reader :ignore_user_agent
@@ -126,6 +129,7 @@ def initialize
@backtrace_filters = DEFAULT_BACKTRACE_FILTERS.dup
@ignore_by_filters = []
@ignore = IGNORE_DEFAULT.dup
+ @worker_ignore = []
@ignore_user_agent = []
@development_environments = %w(development test cucumber)
@development_lookup = true
@@ -172,6 +176,13 @@ def ignore_only=(names)
@ignore = [names].flatten
end
+ # Overrides the list of default worker_ignored errors.
+ #
+ # @param [Array<Exception>] names A list of exceptions to ignore.
+ def worker_ignore_only=(names)
+ @worker_ignore = [names].flatten
+ end
+
# Overrides the list of default ignored user agents
#
# @param [Array<String>] A list of user agents to ignore
@@ -53,6 +53,9 @@ class Notice
# See Configuration#ignore
attr_reader :ignore
+ # See Configuration#worker_ignore
+ attr_reader :worker_ignore
+
# See Configuration#ignore_by_filters
attr_reader :ignore_by_filters
@@ -77,6 +80,7 @@ def initialize(args)
self.notifier_url = args[:notifier_url]
self.ignore = args[:ignore] || []
+ self.worker_ignore = args[:worker_ignore] || []
self.ignore_by_filters = args[:ignore_by_filters] || []
self.backtrace_filters = args[:backtrace_filters] || []
self.params_filters = args[:params_filters] || []
@@ -158,9 +162,17 @@ def to_xml
xml.to_s
end
+ def ignore_for_platform?(error_class)
+ if ::Rails.env.web_server?
+ ignored_class_names.include?(error_class)
+ else
+ worker_ignored_class_names.include?(error_class)
+ end
+ end
+
# Determines if this notice should be ignored
def ignore?
- ignored_class_names.include?(error_class) ||
+ ignore_for_platform?(error_class) ||
ignore_by_filters.any? {|filter| filter.call(self) }
end
@@ -183,7 +195,7 @@ def [](method)
attr_writer :exception, :api_key, :backtrace, :error_class, :error_message,
:backtrace_filters, :parameters, :params_filters,
- :environment_filters, :session_data, :project_root, :url, :ignore,
+ :environment_filters, :session_data, :project_root, :url, :ignore, :worker_ignore,
:ignore_by_filters, :notifier_name, :notifier_url, :notifier_version,
:component, :action, :cgi_data, :environment_name
@@ -298,6 +310,16 @@ def ignored_class_names
end
end
+ def worker_ignored_class_names
+ worker_ignore.collect do |string_or_class|
+ if string_or_class.respond_to?(:name)
+ string_or_class.name
+ else
+ string_or_class
+ end
+ end
+ end
+
def xml_vars_for(builder, hash)
hash.each do |key, value|
if value.respond_to?(:to_hash)
View
@@ -27,6 +27,7 @@ class ConfigurationTest < Test::Unit::TestCase
HoptoadNotifier::Configuration::DEFAULT_BACKTRACE_FILTERS
assert_config_default :ignore,
HoptoadNotifier::Configuration::IGNORE_DEFAULT
+ assert_config_default :worker_ignore, []
assert_config_default :development_lookup, true
assert_config_default :framework, 'Standalone'
end
@@ -81,7 +82,7 @@ class ConfigurationTest < Test::Unit::TestCase
hash = config.to_hash
[:api_key, :backtrace_filters, :development_environments,
:environment_name, :host, :http_open_timeout,
- :http_read_timeout, :ignore, :ignore_by_filters, :ignore_user_agent,
+ :http_read_timeout, :ignore, :worker_ignore, :ignore_by_filters, :ignore_user_agent,
:notifier_name, :notifier_url, :notifier_version, :params_filters,
:project_root, :port, :protocol, :proxy_host, :proxy_pass, :proxy_port,
:proxy_user, :secure, :development_lookup].each do |option|
@@ -143,10 +144,22 @@ class ConfigurationTest < Test::Unit::TestCase
assert_same_elements original_filters + [new_filter], config.ignore
end
+ should "allow worker_ignored exceptions to be appended" do
+ config = HoptoadNotifier::Configuration.new
+ original_filters = config.worker_ignore.dup
+ new_filter = 'hello'
+ config.worker_ignore << new_filter
+ assert_same_elements original_filters + [new_filter], config.worker_ignore
+ end
+
should "allow ignored exceptions to be replaced" do
assert_replaces(:ignore, :ignore_only=)
end
+ should "allow worker_ignored exceptions to be replaced" do
+ assert_replaces(:worker_ignore, :worker_ignore_only=)
+ end
+
should "allow ignored user agents to be replaced" do
assert_replaces(:ignore_user_agent, :ignore_user_agent_only=)
end
View
@@ -246,3 +246,9 @@ def error(*args); end
def fatal(*args); end
end
+#Fake rails env
+module Rails
+ def self.env
+ OpenStruct.new(:web_server? => true)
+ end
+end
View
@@ -277,53 +277,76 @@ def stub_request(attrs = {})
end
end
- should "not ignore an exception not matching ignore filters" do
- notice = build_notice(:error_class => 'ArgumentError',
- :ignore => ['Argument'],
- :ignore_by_filters => [lambda { |notice| false }])
- assert !notice.ignore?
- end
+ context "resque worker errors" do
+ setup do
+ Rails.stubs(:env).returns(OpenStruct.new(:web_server? => false))
+ end
- should "ignore an exception with a matching error class" do
- notice = build_notice(:error_class => 'ArgumentError',
- :ignore => [ArgumentError])
- assert notice.ignore?
+ should "not ignore web server errors" do
+ notice = build_notice(:error_class => 'ArgumentError',
+ :ignore => ['ArgumentError'])
+ assert !notice.ignore?
+ end
+ should "ignore worker errors" do
+ notice = build_notice(:error_class => 'ArgumentError',
+ :worker_ignore => ['ArgumentError'])
+ assert notice.ignore?
+ end
end
- should "ignore an exception with a matching error class name" do
- notice = build_notice(:error_class => 'ArgumentError',
- :ignore => ['ArgumentError'])
- assert notice.ignore?
- end
+ context "web server errors" do
+ setup do
+ Rails.stubs(:env).returns(OpenStruct.new(:web_server? => true))
+ end
- should "ignore an exception with a matching filter" do
- filter = lambda {|notice| notice.error_class == 'ArgumentError' }
- notice = build_notice(:error_class => 'ArgumentError',
- :ignore_by_filters => [filter])
- assert notice.ignore?
- end
+ should "not ignore an exception not matching ignore filters" do
+ notice = build_notice(:error_class => 'ArgumentError',
+ :ignore => ['Argument'],
+ :ignore_by_filters => [lambda { |notice| false }])
+ assert !notice.ignore?
+ end
- should "not raise without an ignore list" do
- notice = build_notice(:ignore => nil, :ignore_by_filters => nil)
- assert_nothing_raised do
- notice.ignore?
+ should "ignore an exception with a matching error class" do
+ notice = build_notice(:error_class => 'ArgumentError',
+ :ignore => [ArgumentError])
+ assert notice.ignore?
end
- end
- ignored_error_classes = %w(
- ActiveRecord::RecordNotFound
- AbstractController::ActionNotFound
- ActionController::RoutingError
- ActionController::InvalidAuthenticityToken
- CGI::Session::CookieStore::TamperedWithCookie
- ActionController::UnknownAction
- )
+ should "ignore an exception with a matching error class name" do
+ notice = build_notice(:error_class => 'ArgumentError',
+ :ignore => ['ArgumentError'])
+ assert notice.ignore?
+ end
- ignored_error_classes.each do |ignored_error_class|
- should "ignore #{ignored_error_class} error by default" do
- notice = build_notice(:error_class => ignored_error_class)
+ should "ignore an exception with a matching filter" do
+ filter = lambda {|notice| notice.error_class == 'ArgumentError' }
+ notice = build_notice(:error_class => 'ArgumentError',
+ :ignore_by_filters => [filter])
assert notice.ignore?
end
+
+ should "not raise without an ignore list" do
+ notice = build_notice(:ignore => nil, :ignore_by_filters => nil)
+ assert_nothing_raised do
+ notice.ignore?
+ end
+ end
+
+ ignored_error_classes = %w(
+ ActiveRecord::RecordNotFound
+ AbstractController::ActionNotFound
+ ActionController::RoutingError
+ ActionController::InvalidAuthenticityToken
+ CGI::Session::CookieStore::TamperedWithCookie
+ ActionController::UnknownAction
+ )
+
+ ignored_error_classes.each do |ignored_error_class|
+ should "ignore #{ignored_error_class} error by default" do
+ notice = build_notice(:error_class => ignored_error_class)
+ assert notice.ignore?
+ end
+ end
end
should "act like a hash" do

2 comments on commit ff431d6

Could we just put the logic in the hoptoad initializer and not change hoptoad_notifier at all?

config.ignore.concat Rails.env.web_server? ? [web server ignores here] : [worker ignores here]

Hmm, good call that is a much less invasive approach.

Please sign in to comment.