diff options
author | Joshua Peek <josh@joshpeek.com> | 2010-01-16 21:34:35 -0600 |
---|---|---|
committer | Joshua Peek <josh@joshpeek.com> | 2010-01-16 21:34:35 -0600 |
commit | eb39d0f7b999f09c4e13f035634887a8f5592443 (patch) | |
tree | 4155dd1bb0cb878ff0aec9f36c113e4a094ab498 | |
parent | d2d4acf02793580e0f0c1bc380389527325b6254 (diff) | |
download | rails-eb39d0f7b999f09c4e13f035634887a8f5592443.tar.gz rails-eb39d0f7b999f09c4e13f035634887a8f5592443.tar.bz2 rails-eb39d0f7b999f09c4e13f035634887a8f5592443.zip |
Use backtrace cleaner for dev mode exception page
9 files changed, 68 insertions, 152 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 10f04dcdf6..56fd4f45c0 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -88,7 +88,10 @@ module ActionDispatch def rescue_action_locally(request, exception) template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], :request => request, - :exception => exception + :exception => exception, + :application_trace => application_trace(exception), + :framework_trace => framework_trace(exception), + :full_trace => full_trace(exception) ) file = "rescues/#{@@rescue_templates[exception.class.name]}.erb" body = template.render(:file => file, :layout => 'rescues/layout.erb') @@ -148,9 +151,21 @@ module ActionDispatch end end - def clean_backtrace(exception) + def application_trace(exception) + clean_backtrace(exception, :silent) + end + + def framework_trace(exception) + clean_backtrace(exception, :noise) + end + + def full_trace(exception) + clean_backtrace(exception, :all) + end + + def clean_backtrace(exception, *args) defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) ? - Rails.backtrace_cleaner.clean(exception.backtrace) : + Rails.backtrace_cleaner.clean(exception.backtrace, *args) : exception.backtrace end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb index 07b4919934..d18b162a93 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb @@ -1,8 +1,8 @@ <% traces = [ - ["Application Trace", @exception.application_backtrace], - ["Framework Trace", @exception.framework_backtrace], - ["Full Trace", @exception.clean_backtrace] + ["Application Trace", @application_trace], + ["Framework Trace", @framework_trace], + ["Full Trace", @full_trace] ] names = traces.collect {|name, trace| name} %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb index 693e56270a..bd6ffbab5d 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -4,7 +4,7 @@ in <%=h @request.parameters['controller'].humanize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> <% end %> </h1> -<pre><%=h @exception.clean_message %></pre> +<pre><%=h @exception.message %></pre> <%= render :file => "rescues/_trace.erb" %> <%= render :file => "rescues/_request_and_response.erb" %> diff --git a/activesupport/lib/active_support/backtrace_cleaner.rb b/activesupport/lib/active_support/backtrace_cleaner.rb index 0e262c003e..6fab565646 100644 --- a/activesupport/lib/active_support/backtrace_cleaner.rb +++ b/activesupport/lib/active_support/backtrace_cleaner.rb @@ -9,7 +9,7 @@ module ActiveSupport # Example: # # bc = BacktraceCleaner.new - # bc.add_filter { |line| line.gsub(Rails.root, '') } + # 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 # @@ -18,10 +18,19 @@ module ActiveSupport 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)) + def clean(backtrace, kind = :silent) + filtered = filter(backtrace) + + case kind + when :silent + silence(filtered) + when :noise + noise(filtered) + else + filtered + end end # Adds a filter from the block provided. Each line in the backtrace will be mapped against this filter. @@ -51,21 +60,28 @@ module ActiveSupport @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 + + def noise(backtrace) + @silencers.each do |s| + backtrace = backtrace.select { |line| s.call(line) } + end + backtrace end end diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 6360a4614e..7213b24f2d 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -30,7 +30,7 @@ module ActiveSupport # # ActiveSupport::Cache.lookup_store(:memory_store) # # => returns a new ActiveSupport::Cache::MemoryStore object - # + # # ActiveSupport::Cache.lookup_store(:mem_cache_store) # # => returns a new ActiveSupport::Cache::MemCacheStore object # @@ -97,7 +97,7 @@ module ActiveSupport # Ruby objects, but don't count on every cache store to be able to do that. # # cache = ActiveSupport::Cache::MemoryStore.new - # + # # cache.read("city") # => nil # cache.write("city", "Duckburgh") # cache.read("city") # => "Duckburgh" @@ -139,7 +139,7 @@ module ActiveSupport # # cache.write("today", "Monday") # cache.fetch("today") # => "Monday" - # + # # cache.fetch("city") # => nil # cache.fetch("city") do # "Duckburgh" @@ -198,7 +198,7 @@ module ActiveSupport # You may also specify additional options via the +options+ argument. # The specific cache store implementation will decide what to do with # +options+. - # + # # For example, MemCacheStore supports the +:expires_in+ option, which # tells the memcached server to automatically expire the cache item after # a certain period: diff --git a/activesupport/lib/active_support/core_ext/exception.rb b/activesupport/lib/active_support/core_ext/exception.rb index b594fbae8e..ef801e713d 100644 --- a/activesupport/lib/active_support/core_ext/exception.rb +++ b/activesupport/lib/active_support/core_ext/exception.rb @@ -1,50 +1,3 @@ module ActiveSupport FrozenObjectError = RUBY_VERSION < '1.9' ? TypeError : RuntimeError end - -# TODO: Turn all this into using the BacktraceCleaner. -class Exception # :nodoc: - # Clean the paths contained in the message. - def self.clean_paths(string) - require 'pathname' unless defined? Pathname - string.gsub(%r{[\w. ]+(/[\w. ]+)+(\.rb)?(\b|$)}) do |path| - Pathname.new(path).cleanpath - end - end - - def clean_message - Exception.clean_paths(message) - end - - TraceSubstitutions = [] - FrameworkStart = /action_controller\/dispatcher\.rb/.freeze - FrameworkRegexp = /generated|vendor|dispatch|ruby|script\/\w+/.freeze - - def clean_backtrace - backtrace.collect do |line| - substituted = TraceSubstitutions.inject(line) do |result, (regexp, sub)| - result.gsub regexp, sub - end - Exception.clean_paths(substituted) - end - end - - def application_backtrace - before_framework_frame = nil - before_application_frame = true - - trace = clean_backtrace.reject do |line| - before_framework_frame ||= (line =~ FrameworkStart) - non_app_frame = (line =~ FrameworkRegexp) - before_application_frame = false unless non_app_frame - before_framework_frame || (non_app_frame && !before_application_frame) - end - - # If we didn't find any application frames, return an empty app trace. - before_application_frame ? [] : trace - end - - def framework_backtrace - clean_backtrace.grep FrameworkRegexp - end -end diff --git a/activesupport/test/core_ext/exception_test.rb b/activesupport/test/core_ext/exception_test.rb deleted file mode 100644 index e63842c0bd..0000000000 --- a/activesupport/test/core_ext/exception_test.rb +++ /dev/null @@ -1,69 +0,0 @@ -require 'abstract_unit' -require 'active_support/core_ext/exception' - -class ExceptionExtTests < Test::Unit::TestCase - - def get_exception(cls = RuntimeError, msg = nil, trace = nil) - begin raise cls, msg, (trace || caller) - rescue Exception => e # passed Exception - return e - end - end - - def setup - Exception::TraceSubstitutions.clear - end - - def test_clean_backtrace - Exception::TraceSubstitutions << [/\s*hidden.*/, ''] - e = get_exception RuntimeError, 'RAWR', ['bhal.rb', 'rawh hid den stuff is not here', 'almost all'] - assert_kind_of Exception, e - assert_equal ['bhal.rb', 'rawh hid den stuff is not here', 'almost all'], e.clean_backtrace - end - - def test_app_backtrace - Exception::TraceSubstitutions << [/\s*hidden.*/, ''] - e = get_exception RuntimeError, 'RAWR', ['bhal.rb', ' vendor/file.rb some stuff', 'almost all'] - assert_kind_of Exception, e - assert_equal ['bhal.rb', 'almost all'], e.application_backtrace - end - - def test_app_backtrace_with_before - Exception::TraceSubstitutions << [/\s*hidden.*/, ''] - e = get_exception RuntimeError, 'RAWR', ['vendor/file.rb some stuff', 'bhal.rb', ' vendor/file.rb some stuff', 'almost all'] - assert_kind_of Exception, e - assert_equal ['vendor/file.rb some stuff', 'bhal.rb', 'almost all'], e.application_backtrace - end - - def test_framework_backtrace_with_before - Exception::TraceSubstitutions << [/\s*hidden.*/, ''] - e = get_exception RuntimeError, 'RAWR', ['vendor/file.rb some stuff', 'bhal.rb', ' vendor/file.rb some stuff', 'almost all'] - assert_kind_of Exception, e - assert_equal ['vendor/file.rb some stuff', ' vendor/file.rb some stuff'], e.framework_backtrace - end - - def test_backtrace_should_clean_paths - Exception::TraceSubstitutions << [/\s*hidden.*/, ''] - e = get_exception RuntimeError, 'RAWR', ['a/b/c/../d/../../../bhal.rb', 'rawh hid den stuff is not here', 'almost all'] - assert_kind_of Exception, e - assert_equal ['bhal.rb', 'rawh hid den stuff is not here', 'almost all'], e.clean_backtrace - end - - def test_clean_message_should_clean_paths - Exception::TraceSubstitutions << [/\s*hidden.*/, ''] - e = get_exception RuntimeError, "I dislike a/z/x/../../b/y/../c", ['a/b/c/../d/../../../bhal.rb', 'rawh hid den stuff is not here', 'almost all'] - assert_kind_of Exception, e - assert_equal "I dislike a/b/c", e.clean_message - end - - def test_app_trace_should_be_empty_when_no_app_frames - Exception::TraceSubstitutions << [/\s*hidden.*/, ''] - e = get_exception RuntimeError, 'RAWR', ['vendor/file.rb some stuff', 'generated/bhal.rb', ' vendor/file.rb some stuff', 'generated/almost all'] - assert_kind_of Exception, e - assert_equal [], e.application_backtrace - end - - def test_frozen_error - assert_raise(ActiveSupport::FrozenObjectError) { "foo".freeze.gsub!(/oo/,'aa') } - end -end diff --git a/railties/lib/rails/backtrace_cleaner.rb b/railties/lib/rails/backtrace_cleaner.rb index cd7dd0f80a..40198306f9 100644 --- a/railties/lib/rails/backtrace_cleaner.rb +++ b/railties/lib/rails/backtrace_cleaner.rb @@ -3,17 +3,7 @@ require 'active_support/backtrace_cleaner' module Rails class BacktraceCleaner < ActiveSupport::BacktraceCleaner ERB_METHOD_SIG = /:in `_run_erb_.*/ - - RAILS_GEMS = %w( actionpack activerecord actionmailer activesupport activeresource rails ) - - VENDOR_DIRS = %w( vendor/rails ) - SERVER_DIRS = %w( lib/mongrel bin/mongrel - lib/passenger bin/passenger-spawn-server - lib/rack ) - RAILS_NOISE = %w( script/server ) - RUBY_NOISE = %w( rubygems/custom_require benchmark.rb ) - - ALL_NOISE = VENDOR_DIRS + SERVER_DIRS + RAILS_NOISE + RUBY_NOISE + APP_DIRS = %w( app config lib test ) def initialize super @@ -22,10 +12,9 @@ module Rails add_filter { |line| line.sub('./', '/') } # for tests add_gem_filters + add_bundler_filters - add_silencer { |line| ALL_NOISE.any? { |dir| line.include?(dir) } } - add_silencer { |line| RAILS_GEMS.any? { |gem| line =~ /^#{gem} / } } - add_silencer { |line| line =~ %r(vendor/plugins/[^\/]+/lib) } + add_silencer { |line| !APP_DIRS.any? { |dir| line =~ /^#{dir}/ } } end private @@ -33,9 +22,21 @@ module Rails return unless defined? Gem (Gem.path + [Gem.default_dir]).uniq.each do |path| # http://gist.github.com/30430 - add_filter { |line| line.sub(/(#{path})\/gems\/([a-z]+)-([0-9.]+)\/(.*)/, '\2 (\3) \4')} + add_filter { |line| + line.sub(%r{(#{path})/gems/([^/]+)-([0-9.]+)/(.*)}, '\2 (\3) \4') + } end end + + def add_bundler_filters + return unless defined? Bundler + add_filter { |line| + line.sub(%r{vendor/gems/[^/]+/[^/]+/gems/([^/]+)-([0-9.]+)/(.*)}, '\1 (\2) \3') + } + add_filter { |line| + line.sub(%r{vendor/gems/[^/]+/[^/]+/dirs/([^/]+)/(.*)}, '\1 \2') + } + end end # For installing the BacktraceCleaner in the test/unit diff --git a/railties/test/backtrace_cleaner_test.rb b/railties/test/backtrace_cleaner_test.rb index 6cff591b94..80077378db 100644 --- a/railties/test/backtrace_cleaner_test.rb +++ b/railties/test/backtrace_cleaner_test.rb @@ -37,7 +37,7 @@ if defined? Gem test "should format installed gems correctly" do @backtrace = [ "#{Gem.path[0]}/gems/nosuchgem-1.2.3/lib/foo.rb" ] - @result = @cleaner.clean(@backtrace) + @result = @cleaner.clean(@backtrace, :all) assert_equal "nosuchgem (1.2.3) lib/foo.rb", @result[0] end @@ -46,7 +46,7 @@ if defined? Gem # skip this test if default_dir is the only directory on Gem.path if @target_dir @backtrace = [ "#{@target_dir}/gems/nosuchgem-1.2.3/lib/foo.rb" ] - @result = @cleaner.clean(@backtrace) + @result = @cleaner.clean(@backtrace, :all) assert_equal "nosuchgem (1.2.3) lib/foo.rb", @result[0] end end |