From bf5e8813dc8129b77c32835e0b6e36fda6fe9e1f Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Sat, 13 Feb 2016 23:18:54 +0000 Subject: [PATCH] Check catch_debugged_exceptions option at runtime Instead of adding the hook to report debugged exceptions based on the value of the configuration option while Rails is initialising, we can always add the hook, but have it check the option at runtime. This is more in line with how other configuration options are used, and means that the option can be set or changed at any time. This also lets us write tests that exercise this behaviour, since the logic is no longer tied to the Rails initialisation process which can only happen once during the test suite. --- lib/raven/integrations/rails.rb | 24 +++++---- .../middleware/debug_exceptions_catcher.rb | 4 +- .../debug_exceptions_catcher_spec.rb | 49 +++++++++++-------- spec/raven/integrations/rails_spec.rb | 20 ++++++++ 4 files changed, 62 insertions(+), 35 deletions(-) diff --git a/lib/raven/integrations/rails.rb b/lib/raven/integrations/rails.rb index 428d4bae0..f5d3e90b5 100644 --- a/lib/raven/integrations/rails.rb +++ b/lib/raven/integrations/rails.rb @@ -22,19 +22,17 @@ class Rails < ::Rails::Railtie end config.after_initialize do - if Raven.configuration.catch_debugged_exceptions - require 'raven/integrations/rails/middleware/debug_exceptions_catcher' - if defined?(::ActionDispatch::DebugExceptions) - exceptions_class = ::ActionDispatch::DebugExceptions - elsif defined?(::ActionDispatch::ShowExceptions) - exceptions_class = ::ActionDispatch::ShowExceptions - end - unless exceptions_class.nil? - if RUBY_VERSION.to_f < 2.0 - exceptions_class.send(:include, Raven::Rails::Middleware::OldDebugExceptionsCatcher) - else - exceptions_class.send(:prepend, Raven::Rails::Middleware::DebugExceptionsCatcher) - end + require 'raven/integrations/rails/middleware/debug_exceptions_catcher' + if defined?(::ActionDispatch::DebugExceptions) + exceptions_class = ::ActionDispatch::DebugExceptions + elsif defined?(::ActionDispatch::ShowExceptions) + exceptions_class = ::ActionDispatch::ShowExceptions + end + unless exceptions_class.nil? + if RUBY_VERSION.to_f < 2.0 + exceptions_class.send(:include, Raven::Rails::Middleware::OldDebugExceptionsCatcher) + else + exceptions_class.send(:prepend, Raven::Rails::Middleware::DebugExceptionsCatcher) end end end diff --git a/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb b/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb index 4335dc5dd..2c67c0c83 100644 --- a/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb +++ b/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb @@ -5,7 +5,7 @@ module DebugExceptionsCatcher def render_exception(env_or_request, exception) begin env = env_or_request.respond_to?(:env) ? env_or_request.env : env_or_request - Raven::Rack.capture_exception(exception, env) + Raven::Rack.capture_exception(exception, env) if Raven.configuration.catch_debugged_exceptions rescue # rubocop:disable Lint/HandleExceptions end super @@ -20,7 +20,7 @@ def self.included(base) def render_exception_with_raven(env_or_request, exception) begin env = env_or_request.respond_to?(:env) ? env_or_request.env : env_or_request - Raven::Rack.capture_exception(exception, env) + Raven::Rack.capture_exception(exception, env) if Raven.configuration.catch_debugged_exceptions rescue # rubocop:disable Lint/HandleExceptions end render_exception_without_raven(env_or_request, exception) diff --git a/spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb b/spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb index 7f93ea698..48b8d4623 100644 --- a/spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb +++ b/spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb @@ -26,11 +26,7 @@ def render_exception(_, exception) let(:env) { {} } - context "using include" do - before do - middleware.send(:include, Raven::Rails::Middleware::OldDebugExceptionsCatcher) - end - + shared_examples "the debug exceptions middleware" do it "shows the exception" do expect(middleware.new(app).call(env)).to eq([500, "app error", {}]) end @@ -46,28 +42,41 @@ def render_exception(_, exception) expect(middleware.new(app).call(env)).to eq([500, "app error", {}]) end end + + context "when catch_debugged_exceptions is disabled" do + before do + Raven.configure do |config| + config.catch_debugged_exceptions = false + end + end + + after do + Raven.configure do |config| + config.catch_debugged_exceptions = true + end + end + + it "doesn't capture the exception" do + expect(Raven::Rack).not_to receive(:capture_exception) + middleware.new(app).call(env) + end + end end - context "using prepend" do + context "using include" do before do - skip "prepend not available" unless middleware.respond_to?(:prepend, true) - middleware.send(:prepend, Raven::Rails::Middleware::DebugExceptionsCatcher) + middleware.send(:include, Raven::Rails::Middleware::OldDebugExceptionsCatcher) end - it "shows the exception" do - expect(middleware.new(app).call(env)).to eq([500, "app error", {}]) - end + it_behaves_like "the debug exceptions middleware" + end - it "captures the exception" do - expect(Raven::Rack).to receive(:capture_exception) - middleware.new(app).call(env) + context "using prepend" do + before do + skip "prepend not available" unless middleware.respond_to?(:prepend, true) + middleware.send(:prepend, Raven::Rails::Middleware::DebugExceptionsCatcher) end - context "when an error is raised" do - it "shows the original exception" do - allow(Raven::Rack).to receive(:capture_exception).and_raise("raven error") - expect(middleware.new(app).call(env)).to eq([500, "app error", {}]) - end - end + it_behaves_like "the debug exceptions middleware" end end diff --git a/spec/raven/integrations/rails_spec.rb b/spec/raven/integrations/rails_spec.rb index 66ada6c71..40c71aa29 100644 --- a/spec/raven/integrations/rails_spec.rb +++ b/spec/raven/integrations/rails_spec.rb @@ -48,4 +48,24 @@ it "doesn't clobber a manually configured release" do expect(Raven.configuration.release).to eq('beta') end + + context "when catch_debugged_exceptions is disabled" do + before do + Raven.configure do |config| + config.catch_debugged_exceptions = false + end + end + + after do + Raven.configure do |config| + config.catch_debugged_exceptions = true + end + end + + it "doesn't capture exceptions automatically" do + get "/exception" + expect(response.status).to eq(500) + expect(Raven.client.transport.events.size).to eq(0) + end + end end