From c8f4c53d55762010496cf801cf1ec726e35d75df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Thu, 4 Jan 2018 11:27:14 +0000 Subject: Reduce log noise handling ActionController::RoutingErrors Each time a missing route is hit 32 lines of internal rails traces are written to the log. This is overly verbose and doesn't offer any actionable information to the user. With this change we'll still write an error message showing the route error but the trace will be omitted. --- actionpack/CHANGELOG.md | 4 ++++ .../action_dispatch/middleware/debug_exceptions.rb | 4 +--- .../middleware/exception_wrapper.rb | 6 ++++++ actionpack/test/dispatch/debug_exceptions_test.rb | 23 ++++++++++++++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4bcf313ecc..9a631f8f86 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Reduced log noise handling ActionController::RoutingErrors. + + *Alberto Fernández-Capel* + * Keep part when scope option has value When a route was defined within an optional scope, if that route didn't diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 59113e13f4..6435947434 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -138,9 +138,7 @@ module ActionDispatch return unless logger exception = wrapper.exception - - trace = wrapper.application_trace - trace = wrapper.framework_trace if trace.empty? + trace = wrapper.exception_trace ActiveSupport::Deprecation.silence do message = [] diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 0cc56f5013..2bf3ddb540 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -62,6 +62,12 @@ module ActionDispatch self.class.status_code_for_exception(unwrapped_exception.class.name) end + def exception_trace + trace = application_trace + trace = framework_trace if trace.empty? && !exception.is_a?(ActionController::RoutingError) + trace + end + def application_trace clean_backtrace(:silent) end diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 3e57e8f4d9..7adf935849 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -466,6 +466,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end test "logs exception backtrace when all lines silenced" do + @app = DevelopmentApp + output = StringIO.new backtrace_cleaner = ActiveSupport::BacktraceCleaner.new backtrace_cleaner.add_silencer { true } @@ -478,6 +480,27 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_operator((output.rewind && output.read).lines.count, :>, 10) end + test "doesn't log the framework backtrace when error type is a routing error" do + @app = ProductionApp + + output = StringIO.new + backtrace_cleaner = ActiveSupport::BacktraceCleaner.new + backtrace_cleaner.add_silencer { true } + + env = { "action_dispatch.show_exceptions" => true, + "action_dispatch.logger" => Logger.new(output), + "action_dispatch.backtrace_cleaner" => backtrace_cleaner } + + assert_raises ActionController::RoutingError do + get "/pass", headers: env + end + + log = output.rewind && output.read + + assert_includes log, "ActionController::RoutingError (No route matches [GET] \"/pass\")" + assert_equal 3, log.lines.count + end + test "display backtrace when error type is SyntaxError" do @app = DevelopmentApp -- cgit v1.2.3