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