From f42c77f927eb49b00e84d355e07de48723d03fcb Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 22 Nov 2008 18:06:08 +0100 Subject: Added ActiveSupport::BacktraceCleaner and Rails::BacktraceCleaner for cutting down on backtrace noise (inspired by the Thoughtbot Quiet Backtrace plugin) [DHH] --- actionpack/lib/action_controller/rescue.rb | 14 ++--- actionpack/lib/action_view/template_error.rb | 17 +++-- actionpack/test/controller/rescue_test.rb | 18 ------ activesupport/CHANGELOG | 2 + activesupport/lib/active_support.rb | 1 + .../lib/active_support/backtrace_cleaner.rb | 72 ++++++++++++++++++++++ .../lib/active_support/core_ext/exception.rb | 1 + activesupport/lib/active_support/test_case.rb | 10 ++- activesupport/test/abstract_unit.rb | 1 + activesupport/test/clean_backtrace_test.rb | 47 ++++++++++++++ activesupport/test/core_ext/array_ext_test.rb | 2 +- railties/CHANGELOG | 2 + railties/Rakefile | 6 +- .../configs/initializers/backtrace_silencers.rb | 7 +++ railties/lib/initializer.rb | 8 +++ railties/lib/rails/backtrace_cleaner.rb | 34 ++++++++++ .../generators/applications/app/app_generator.rb | 7 ++- 17 files changed, 204 insertions(+), 45 deletions(-) create mode 100644 activesupport/lib/active_support/backtrace_cleaner.rb create mode 100644 activesupport/test/clean_backtrace_test.rb create mode 100644 railties/configs/initializers/backtrace_silencers.rb create mode 100644 railties/lib/rails/backtrace_cleaner.rb diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index ec8e9b92d5..9c24c3def4 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -68,9 +68,8 @@ module ActionController #:nodoc: logger.fatal(exception.to_s) else logger.fatal( - "\n\n#{exception.class} (#{exception.message}):\n " + - clean_backtrace(exception).join("\n ") + - "\n\n" + "\n#{exception.class} (#{exception.message}):\n " + + clean_backtrace(exception).join("\n ") + "\n\n" ) end end @@ -151,13 +150,8 @@ module ActionController #:nodoc: end def clean_backtrace(exception) - if backtrace = exception.backtrace - if defined?(RAILS_ROOT) - backtrace.map { |line| line.sub RAILS_ROOT, '' } - else - backtrace - end - end + defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) ? + Rails.backtrace_cleaner.clean(exception.backtrace) : exception.backtrace end end end diff --git a/actionpack/lib/action_view/template_error.rb b/actionpack/lib/action_view/template_error.rb index bcce331773..37cb1c7c6c 100644 --- a/actionpack/lib/action_view/template_error.rb +++ b/actionpack/lib/action_view/template_error.rb @@ -20,7 +20,11 @@ module ActionView end def clean_backtrace - original_exception.clean_backtrace + if defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) + Rails.backtrace_cleaner.clean(original_exception.backtrace) + else + original_exception.backtrace + end end def sub_template_message @@ -66,8 +70,8 @@ module ActionView end def to_s - "\n\n#{self.class} (#{message}) #{source_location}:\n" + - "#{source_extract}\n #{clean_backtrace.join("\n ")}\n\n" + "\n#{self.class} (#{message}) #{source_location}:\n" + + "#{source_extract}\n #{clean_backtrace.join("\n ")}\n\n" end # don't do anything nontrivial here. Any raised exception from here becomes fatal @@ -92,9 +96,4 @@ module ActionView end + file_name end end -end - -if defined?(Exception::TraceSubstitutions) - Exception::TraceSubstitutions << [/:in\s+`_run_.*'\s*$/, ''] - Exception::TraceSubstitutions << [%r{^\s*#{Regexp.escape RAILS_ROOT}/}, ''] if defined?(RAILS_ROOT) -end +end \ No newline at end of file diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 32c6c013f1..63f9827f4a 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -291,24 +291,6 @@ class RescueControllerTest < ActionController::TestCase assert_equal 'template_error', templates[ActionView::TemplateError.name] end - def test_clean_backtrace - with_rails_root nil do - # No action if RAILS_ROOT isn't set. - cleaned = @controller.send(:clean_backtrace, @exception) - assert_equal @exception.backtrace, cleaned - end - - with_rails_root Dir.pwd do - # RAILS_ROOT is removed from backtrace. - cleaned = @controller.send(:clean_backtrace, @exception) - expected = @exception.backtrace.map { |line| line.sub(RAILS_ROOT, '') } - assert_equal expected, cleaned - - # No action if backtrace is nil. - assert_nil @controller.send(:clean_backtrace, Exception.new) - end - end - def test_not_implemented with_all_requests_local false do with_rails_public_path(".") do diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index d0895bb709..5e1b1cb4b0 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *2.3.0 [Edge]* +* Added ActiveSupport::BacktraceCleaner to cut down on backtrace noise according to filters and silencers [DHH] + * Added Object#try. ( Taken from http://ozmm.org/posts/try.html ) [Chris Wanstrath] * Added Enumerable#none? to check that none of the elements match the block #1408 [Damian Janowski] diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index d2a7df8bf0..cbfd95f092 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -29,6 +29,7 @@ require 'active_support/callbacks' require 'active_support/core_ext' require 'active_support/buffered_logger' +require 'active_support/backtrace_cleaner' require 'active_support/gzip' require 'active_support/cache' diff --git a/activesupport/lib/active_support/backtrace_cleaner.rb b/activesupport/lib/active_support/backtrace_cleaner.rb new file mode 100644 index 0000000000..0e262c003e --- /dev/null +++ b/activesupport/lib/active_support/backtrace_cleaner.rb @@ -0,0 +1,72 @@ +module ActiveSupport + # Many backtraces include too much information that's not relevant for the context. This makes it hard to find the signal + # in the backtrace and adds debugging time. With a BacktraceCleaner, you can setup filters and silencers for your particular + # context, so only the relevant lines are included. + # + # If you need to reconfigure an existing BacktraceCleaner, like the one in Rails, to show as much as possible, you can always + # call BacktraceCleaner#remove_silencers! + # + # Example: + # + # bc = BacktraceCleaner.new + # bc.add_filter { |line| line.gsub(Rails.root, '') } + # bc.add_silencer { |line| line =~ /mongrel|rubygems/ } + # bc.clean(exception.backtrace) # will strip the Rails.root prefix and skip any lines from mongrel or rubygems + # + # Inspired by the Quiet Backtrace gem by Thoughtbot. + class BacktraceCleaner + def initialize + @filters, @silencers = [], [] + end + + # Returns the backtrace after all filters and silencers has been run against it. Filters run first, then silencers. + def clean(backtrace) + silence(filter(backtrace)) + end + + # Adds a filter from the block provided. Each line in the backtrace will be mapped against this filter. + # + # Example: + # + # # Will turn "/my/rails/root/app/models/person.rb" into "/app/models/person.rb" + # backtrace_cleaner.add_filter { |line| line.gsub(Rails.root, '') } + def add_filter(&block) + @filters << block + end + + # Adds a silencer from the block provided. If the silencer returns true for a given line, it'll be excluded from the + # clean backtrace. + # + # Example: + # + # # Will reject all lines that include the word "mongrel", like "/gems/mongrel/server.rb" or "/app/my_mongrel_server/rb" + # backtrace_cleaner.add_silencer { |line| line =~ /mongrel/ } + def add_silencer(&block) + @silencers << block + end + + # Will remove all silencers, but leave in the filters. This is useful if your context of debugging suddenly expands as + # you suspect a bug in the libraries you use. + def remove_silencers! + @silencers = [] + end + + + private + def filter(backtrace) + @filters.each do |f| + backtrace = backtrace.map { |line| f.call(line) } + end + + backtrace + end + + def silence(backtrace) + @silencers.each do |s| + backtrace = backtrace.reject { |line| s.call(line) } + end + + backtrace + end + end +end diff --git a/activesupport/lib/active_support/core_ext/exception.rb b/activesupport/lib/active_support/core_ext/exception.rb index 57c8568334..73470cbe05 100644 --- a/activesupport/lib/active_support/core_ext/exception.rb +++ b/activesupport/lib/active_support/core_ext/exception.rb @@ -6,6 +6,7 @@ module ActiveSupport end end +# TODO: Turn all this into using the BacktraceCleaner. class Exception # :nodoc: def clean_message Pathname.clean_within message diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index 5b825b212d..d1e657c15f 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -21,15 +21,21 @@ module ActiveSupport Assertion = MiniTest::Assertion end + # TODO: Figure out how to get the Rails::BacktraceFilter into minitest/unit # Test::Unit compatibility. rescue LoadError require 'test/unit/testcase' require 'active_support/testing/default' + if defined?(Rails) + require 'rails/backtrace_cleaner' + Test::Unit::Util::BacktraceFilter.module_eval { include Rails::BacktraceFilterForTestUnit } + end + class TestCase < ::Test::Unit::TestCase Assertion = Test::Unit::AssertionFailedError include ActiveSupport::Testing::Default - end + end end class TestCase @@ -37,4 +43,4 @@ module ActiveSupport include ActiveSupport::Testing::Assertions extend ActiveSupport::Testing::Declarative end -end +end \ No newline at end of file diff --git a/activesupport/test/abstract_unit.rb b/activesupport/test/abstract_unit.rb index deb512eeb6..047f0effad 100644 --- a/activesupport/test/abstract_unit.rb +++ b/activesupport/test/abstract_unit.rb @@ -3,6 +3,7 @@ require 'test/unit' gem 'mocha', '>= 0.9.0' require 'mocha' +$:.unshift "#{File.dirname(__FILE__)}/../lib" require 'active_support' require 'active_support/test_case' diff --git a/activesupport/test/clean_backtrace_test.rb b/activesupport/test/clean_backtrace_test.rb new file mode 100644 index 0000000000..ddbc258df1 --- /dev/null +++ b/activesupport/test/clean_backtrace_test.rb @@ -0,0 +1,47 @@ +require 'abstract_unit' + +class BacktraceCleanerFilterTest < ActiveSupport::TestCase + def setup + @bc = ActiveSupport::BacktraceCleaner.new + @bc.add_filter { |line| line.gsub("/my/prefix", '') } + end + + test "backtrace should not contain prefix when it has been filtered out" do + assert_equal "/my/class.rb", @bc.clean([ "/my/prefix/my/class.rb" ]).first + end + + test "backtrace should contain unaltered lines if they dont match a filter" do + assert_equal "/my/other_prefix/my/class.rb", @bc.clean([ "/my/other_prefix/my/class.rb" ]).first + end + + test "backtrace should filter all lines in a backtrace" do + assert_equal \ + ["/my/class.rb", "/my/module.rb"], + @bc.clean([ "/my/prefix/my/class.rb", "/my/prefix/my/module.rb" ]) + end +end + +class BacktraceCleanerSilencerTest < ActiveSupport::TestCase + def setup + @bc = ActiveSupport::BacktraceCleaner.new + @bc.add_silencer { |line| line =~ /mongrel/ } + end + + test "backtrace should not contain lines that match the silencer" do + assert_equal \ + [ "/other/class.rb" ], + @bc.clean([ "/mongrel/class.rb", "/other/class.rb", "/mongrel/stuff.rb" ]) + end +end + +class BacktraceCleanerFilterAndSilencerTest < ActiveSupport::TestCase + def setup + @bc = ActiveSupport::BacktraceCleaner.new + @bc.add_filter { |line| line.gsub("/mongrel", "") } + @bc.add_silencer { |line| line =~ /mongrel/ } + end + + test "backtrace should not silence lines that has first had their silence hook filtered out" do + assert_equal [ "/class.rb" ], @bc.clean([ "/mongrel/class.rb" ]) + end +end \ No newline at end of file diff --git a/activesupport/test/core_ext/array_ext_test.rb b/activesupport/test/core_ext/array_ext_test.rb index 32cab2724b..d3edbc6826 100644 --- a/activesupport/test/core_ext/array_ext_test.rb +++ b/activesupport/test/core_ext/array_ext_test.rb @@ -21,7 +21,7 @@ class ArrayExtAccessTests < Test::Unit::TestCase assert_equal array[2], array.third assert_equal array[3], array.fourth assert_equal array[4], array.fifth - assert_equal array[41], array.fourty_two + assert_equal array[41], array.forty_two end end diff --git a/railties/CHANGELOG b/railties/CHANGELOG index a93052298b..6a644ca63a 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,7 @@ *2.3.0 [Edge]* +* Added Rails.backtrace_cleaner as an accessor for the Rails::BacktraceCleaner instance used by the framework to cut down on backtrace noise and config/initializers/backtrace_silencers.rb to add your own (or turn them all off) [DHH] + * Switch from Test::Unit::TestCase to ActiveSupport::TestCase. [Jeremy Kemper] * Added config.i18n settings gatherer to config/environment, auto-loading of all locales in config/locales/*.rb,yml, and config/locales/en.yml as a sample locale [DHH] diff --git a/railties/Rakefile b/railties/Rakefile index 8c8bec0d27..cbfa14e887 100644 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -197,8 +197,10 @@ task :copy_configs do cp "configs/routes.rb", "#{PKG_DESTINATION}/config/routes.rb" - cp "configs/initializers/inflections.rb", "#{PKG_DESTINATION}/config/initializers/inflections.rb" - cp "configs/initializers/mime_types.rb", "#{PKG_DESTINATION}/config/initializers/mime_types.rb" + cp "configs/initializers/backtrace_silencers.rb", "#{PKG_DESTINATION}/config/initializers/backtrace_silencers.rb" + cp "configs/initializers/inflections.rb", "#{PKG_DESTINATION}/config/initializers/inflections.rb" + cp "configs/initializers/mime_types.rb", "#{PKG_DESTINATION}/config/initializers/mime_types.rb" + cp "configs/initializers/new_rails_defaults.rb", "#{PKG_DESTINATION}/config/initializers/new_rails_defaults.rb" cp "configs/locales/en.yml", "#{PKG_DESTINATION}/config/locales/en.yml" diff --git a/railties/configs/initializers/backtrace_silencers.rb b/railties/configs/initializers/backtrace_silencers.rb new file mode 100644 index 0000000000..c2169ed01c --- /dev/null +++ b/railties/configs/initializers/backtrace_silencers.rb @@ -0,0 +1,7 @@ +# Be sure to restart your server when you modify this file. + +# You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces. +# Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } + +# You can also remove all the silencers if you're trying do debug a problem that might steem from framework code. +# Rails.backtrace_cleaner.remove_silencers! \ No newline at end of file diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 16abe62d1b..fcf6ea3feb 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -39,6 +39,14 @@ module Rails nil end end + + def backtrace_cleaner + @@backtrace_cleaner ||= begin + # Relies on ActiveSupport, so we have to lazy load to postpone definition until AS has been loaded + require 'rails/backtrace_cleaner' + Rails::BacktraceCleaner.new + end + end def root if defined?(RAILS_ROOT) diff --git a/railties/lib/rails/backtrace_cleaner.rb b/railties/lib/rails/backtrace_cleaner.rb new file mode 100644 index 0000000000..ffc5ef42aa --- /dev/null +++ b/railties/lib/rails/backtrace_cleaner.rb @@ -0,0 +1,34 @@ +module Rails + class BacktraceCleaner < ActiveSupport::BacktraceCleaner + ERB_METHOD_SIG = /:in `_run_erb_.*/ + + VENDOR_DIRS = %w( vendor/plugins vendor/gems vendor/rails ) + MONGREL_DIRS = %w( lib/mongrel bin/mongrel ) + RAILS_NOISE = %w( script/server ) + RUBY_NOISE = %w( rubygems/custom_require benchmark.rb ) + + ALL_NOISE = VENDOR_DIRS + MONGREL_DIRS + RAILS_NOISE + RUBY_NOISE + + + def initialize + super + add_filter { |line| line.sub(RAILS_ROOT, '') } + add_filter { |line| line.sub(ERB_METHOD_SIG, '') } + add_silencer { |line| ALL_NOISE.any? { |dir| line.include?(dir) } } + end + end + + + # For installing the BacktraceCleaner in the test/unit + module BacktraceFilterForTestUnit #:nodoc: + def self.included(klass) + klass.send :alias_method_chain, :filter_backtrace, :cleaning + end + + def filter_backtrace_with_cleaning(backtrace) + backtrace = filter_backtrace_without_cleaning(backtrace) + backtrace = backtrace.first.split("\n") if backtrace.size == 1 + Rails.backtrace_cleaner.clean(backtrace) + end + end +end \ No newline at end of file diff --git a/railties/lib/rails_generator/generators/applications/app/app_generator.rb b/railties/lib/rails_generator/generators/applications/app/app_generator.rb index 347ce6352c..e52dcadd4d 100644 --- a/railties/lib/rails_generator/generators/applications/app/app_generator.rb +++ b/railties/lib/rails_generator/generators/applications/app/app_generator.rb @@ -62,9 +62,10 @@ class AppGenerator < Rails::Generator::Base m.template "configs/routes.rb", "config/routes.rb" # Initializers - m.template "configs/initializers/inflections.rb", "config/initializers/inflections.rb" - m.template "configs/initializers/mime_types.rb", "config/initializers/mime_types.rb" - m.template "configs/initializers/new_rails_defaults.rb", "config/initializers/new_rails_defaults.rb" + m.template "configs/initializers/backtrace_silencers.rb", "config/initializers/backtrace_silencers.rb" + m.template "configs/initializers/inflections.rb", "config/initializers/inflections.rb" + m.template "configs/initializers/mime_types.rb", "config/initializers/mime_types.rb" + m.template "configs/initializers/new_rails_defaults.rb", "config/initializers/new_rails_defaults.rb" # Locale m.template "configs/locales/en.yml", "config/locales/en.yml" -- cgit v1.2.3