@@ -15,7 +15,7 @@ module HoptoadNotifier # any custom processing that is defined with Rails 2's exception helpers. def rescue_action_in_public_with_hoptoad(exception) unless ignore_user_agent? - HoptoadNotifier.notify_or_ignore(exception, :request => request) + HoptoadNotifier.notify_or_ignore(exception, request_data_for_hoptoad) end rescue_action_in_public_without_hoptoad(exception) end @@ -24,7 +24,7 @@ module HoptoadNotifier # inside the controller. Otherwise it works like HoptoadNotifier.notify. def notify_hoptoad(hash_or_exception) unless consider_all_requests_local || local_request? - HoptoadNotifier.notify(hash_or_exception, :request => request) + HoptoadNotifier.notify(hash_or_exception, request_data_for_hoptoad) end end @@ -34,5 +34,10 @@ module HoptoadNotifier HoptoadNotifier.configuration.ignore_user_agent.flatten.any? { |ua| ua === user_agent } end + def request_data_for_hoptoad + { :request => request, + :session => session.to_hash } + end + end end lib/hoptoad_notifier/catcher.rb @@ -53,6 +53,7 @@ module HoptoadNotifier def initialize(args) self.args = args self.exception = args[:exception] + # TODO: should this be in Catcher? self.request = args[:request] self.api_key = args[:api_key] self.project_root = args[:project_root] @@ -228,9 +229,7 @@ module HoptoadNotifier end def find_session_data - self.session_data = args[:session_data] || args[:session] || request.try(:session) || {} - self.session_data = session_data.instance_variable_get('@data') if session_data.instance_variables.include?('@data') - self.session_data = session_data.to_hash if session_data.respond_to?(:to_hash) + self.session_data = args[:session_data] || args[:session] || {} self.session_data = session_data[:data] if session_data[:data] end lib/hoptoad_notifier/notice.rb @@ -22,6 +22,30 @@ class CatcherTest < Test::Unit::TestCase end end + def assert_sent_session_data(data) + assert_not_nil notice = last_sent_notice_data, "should send a notice" + assert_not_nil notice['session'], "should send a session, (got #{notice.inspect})" + assert_equal data, notice['session']['data'] + end + + def assert_sent_parameter_data(data) + assert_not_nil notice = last_sent_notice_data, "should send a notice" + assert_not_nil notice['params'], "should send parameters, (got #{notice.inspect})" + assert_equal data, notice['params'] + end + + def sender + HoptoadNotifier.sender + end + + def last_sent_notice_yaml + sender.collected.last + end + + def last_sent_notice_data + YAML.load(last_sent_notice_yaml)['notice'] + end + def process_action(opts = {}, &action) opts[:request] ||= ActionController::TestRequest.new opts[:response] ||= ActionController::TestResponse.new @@ -43,22 +67,36 @@ class CatcherTest < Test::Unit::TestCase klass.local = opts[:local] controller = klass.new controller.stubs(:rescue_action_in_public_without_hoptoad) + opts[:request].session.clear + opts[:request].session.merge!(opts[:session] || {}) controller.process(opts[:request], opts[:response]) controller end + def process_action_with_manual_notification(args = {}) + process_action(args) do + notify_hoptoad(:error_message => 'fail') + # Rails will raise a template error if we don't render something + render :nothing => true + end + end + + def process_action_with_automatic_notification(args = {}) + process_action(args) { raise "Hello" } + end + should "deliver notices from exceptions raised in public requests" do - process_action { raise "Hello" } + process_action_with_automatic_notification assert_caught_and_sent end should "not deliver notices from exceptions in local requests" do - process_action(:local => true) { raise "Hello" } + process_action_with_automatic_notification(:local => true) assert_caught_and_not_sent end should "not deliver notices from exceptions when all requests are local" do - process_action(:all_local => true) { raise "Hello" } + process_action_with_automatic_notification(:all_local => true) assert_caught_and_not_sent end @@ -70,44 +108,33 @@ class CatcherTest < Test::Unit::TestCase should "not deliver ignored exceptions raised by actions" do ignore(RuntimeError) - process_action { raise "Hello" } + process_action_with_automatic_notification assert_caught_and_not_sent end should "deliver ignored exception raised manually" do ignore(RuntimeError) - process_action { notify_hoptoad(:message => 'uh oh') } + process_action_with_manual_notification assert_caught_and_sent end should "deliver manually sent notices in public requests" do - process_action do - notify_hoptoad(:error_message => 'Yeah') - render :text => 'Hello' - end + process_action_with_manual_notification assert_caught_and_sent end should "not deliver manually sent notices in local requests" do - process_action(:local => true) do - notify_hoptoad(:error_message => 'Yeah') - render :text => 'Hello' - end + process_action_with_manual_notification(:local => true) assert_caught_and_not_sent end should "not deliver manually sent notices when all requests are local" do - process_action(:all_local => true) do - notify_hoptoad(:error_message => 'Yeah') - render :text => 'Hello' - end + process_action_with_manual_notification(:all_local => true) assert_caught_and_not_sent end should "continue with default behavior after delivering an exception" do - controller = process_action(:public => true) do - raise 'Oh no' - end + controller = process_action_with_automatic_notification(:public => true) # TODO: can we test this without stubbing? assert_received(controller, :rescue_action_in_public_without_hoptoad) end @@ -119,9 +146,7 @@ class CatcherTest < Test::Unit::TestCase should "pass the request to the notifier" do stub_notice! - controller = process_action(:public => true) do - raise 'Oh no' - end + controller = process_action_with_automatic_notification(:public => true) assert_received(HoptoadNotifier::Notice, :new) do |expect| expect.with(has_entries(:request => controller.request)) end @@ -129,19 +154,44 @@ class CatcherTest < Test::Unit::TestCase should "ignore exceptions when user agent is being ignored by regular expression" do HoptoadNotifier.configuration.ignore_user_agent_only = [/Ignored/] - process_action(:user_agent => 'ShouldBeIgnored') { raise 'Oh no' } + process_action_with_automatic_notification(:user_agent => 'ShouldBeIgnored') assert_caught_and_not_sent end should "ignore exceptions when user agent is being ignored by string" do HoptoadNotifier.configuration.ignore_user_agent_only = ['IgnoredUserAgent'] - process_action(:user_agent => 'IgnoredUserAgent') { raise 'Oh no' } + process_action_with_automatic_notification(:user_agent => 'IgnoredUserAgent') assert_caught_and_not_sent end should "not ignore exceptions when user agent is not being ignored" do HoptoadNotifier.configuration.ignore_user_agent_only = ['IgnoredUserAgent'] - process_action(:user_agent => 'NonIgnoredAgent') { raise 'Oh no' } + process_action_with_automatic_notification(:user_agent => 'NonIgnoredAgent') assert_caught_and_sent end + + should "send session data for manual notifications" do + data = { 'one' => 'two' } + process_action_with_manual_notification(:session => data) + assert_sent_session_data data + end + + should "send session data for automatic notification" do + data = { 'one' => 'two' } + process_action_with_automatic_notification(:session => data) + assert_sent_session_data data + end + + should "send request data for manual notification" do + params = { 'controller' => "users", 'action' => "create" } + process_action_with_manual_notification(:params => params) + assert_sent_parameter_data params + end + + should "send request data for automatic notification" do + params = { 'controller' => "users", 'action' => "create" } + process_action_with_automatic_notification(:params => params) + assert_sent_parameter_data params + end + end test/catcher_test.rb @@ -83,29 +83,6 @@ class NoticeTest < Test::Unit::TestCase assert_equal data, notice.session_data end - should "accept session data from a session object that converts to a hash" do - data = { 'one' => 'two' } - session = stub('session', :to_hash => data) - notice = build_notice(:session => session) - assert_equal data, notice.session_data - end - - should "accept session data from a session object with a @data variable" do - data = { 'one' => 'two' } - session = "session" - session.instance_variable_set('@data', data) - notice = build_notice(:session => session) - assert_equal data, notice.session_data - end - - should "accept session data from a request object with a session method" do - data = { 'one' => 'two' } - session = stub('session', :to_hash => data) - request = stub_request(:session => session) - notice = build_notice(:request => request) - assert_equal data, notice.session_data - end - should "set the environment from a hash or ENV" do custom_env = { 'string' => 'value' } custom_notice = build_notice(:environment => custom_env) test/notice_test.rb fabd3529f7b78c0f25f6d6f572fabfa7b9fdb8de Joe Ferris jferris@metaphor.local http://github.com/thoughtbot/hoptoad_notifier/commit/d530aa90a8c2f1b5e1f206db05a179e4fb32909c d530aa90a8c2f1b5e1f206db05a179e4fb32909c 2009-11-05T08:12:09-08:00 2009-08-17T12:32:03-07:00 Moved session object parsing into the Catcher 71316ff0ee249f6b97d27448b2c7fd0c69e455c2 Joe Ferris jferris@metaphor.local