From 0bbe6002abec53a6178a043d463044a49aa46d2f Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Tue, 16 Feb 2016 06:35:24 +0000 Subject: [PATCH] Return original response in debug middleware patch The call to the original render_exception method was moved into an ensure block to prevent errors in this module from interfering with Rails' own error handling behaviour. However an ensure block has no effect on the return value of the method, and we were returning the result of Raven::Rack.capture_exception instead of the response generated by the original Rails middleware. --- .../middleware/debug_exceptions_catcher.rb | 16 ++-- .../debug_exceptions_catcher_spec.rb | 73 +++++++++++++++++++ 2 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb diff --git a/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb b/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb index bd35ff0b5..4335dc5dd 100644 --- a/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb +++ b/lib/raven/integrations/rails/middleware/debug_exceptions_catcher.rb @@ -3,9 +3,11 @@ class Rails module Middleware module DebugExceptionsCatcher def render_exception(env_or_request, exception) - env = env_or_request.respond_to?(:env) ? env_or_request.env : env_or_request - Raven::Rack.capture_exception(exception, env) - ensure + begin + env = env_or_request.respond_to?(:env) ? env_or_request.env : env_or_request + Raven::Rack.capture_exception(exception, env) + rescue # rubocop:disable Lint/HandleExceptions + end super end end @@ -16,9 +18,11 @@ def self.included(base) end def render_exception_with_raven(env_or_request, exception) - env = env_or_request.respond_to?(:env) ? env_or_request.env : env_or_request - Raven::Rack.capture_exception(exception, env) - ensure + begin + env = env_or_request.respond_to?(:env) ? env_or_request.env : env_or_request + Raven::Rack.capture_exception(exception, env) + rescue # rubocop:disable Lint/HandleExceptions + end render_exception_without_raven(env_or_request, exception) end end diff --git a/spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb b/spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb new file mode 100644 index 000000000..7f93ea698 --- /dev/null +++ b/spec/raven/integrations/rails/middleware/debug_exceptions_catcher_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' +require 'raven/integrations/rails/middleware/debug_exceptions_catcher' + +describe Raven::Rails::Middleware::DebugExceptionsCatcher do + let(:middleware) do + Class.new do + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + rescue => e + render_exception(env, e) + end + + def render_exception(_, exception) + [500, exception.message, {}] + end + end + end + + let(:app) do + lambda { |_| raise "app error" } # rubocop:disable Style/Lambda + end + + let(:env) { {} } + + context "using include" do + before do + 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 "captures the exception" do + expect(Raven::Rack).to receive(:capture_exception) + middleware.new(app).call(env) + 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 + end + + context "using prepend" do + before do + skip "prepend not available" unless middleware.respond_to?(:prepend, true) + middleware.send(:prepend, Raven::Rails::Middleware::DebugExceptionsCatcher) + end + + it "shows the exception" do + expect(middleware.new(app).call(env)).to eq([500, "app error", {}]) + end + + it "captures the exception" do + expect(Raven::Rack).to receive(:capture_exception) + middleware.new(app).call(env) + 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 + end +end