refactor error handling (removes one instance_eval)

lib/sinatra/base.rb

@@ -754,36 +754,29 @@ module Sinatra
 
       if @response.status == 404
         @response.headers['X-Cascade'] = 'pass'
-        @response.body                 = ['<h1>Not Found</h1>']
+        @response.body = ['<h1>Not Found</h1>']
       end
 
       if res = error_block!(boom.class)
         res
       elsif @response.status >= 500
-        settings.raise_errors? ? raise(boom) : error_block!(Exception)
+        raise boom if settings.raise_errors? or settings.show_exceptions?
+        error_block! Exception
       end
     end
 
     # Find an custom error block for the key(s) specified.
-    def error_block!(*keys)
+    def error_block!(key)
-      keys.each do |key|
+      base = settings
-        base = settings
+      while base.respond_to?(:errors)
-        while base.respond_to?(:errors)
+        next base = base.superclass unless args = base.errors[key]
-          if block = base.errors[key]
+        return process_route(*args)
-            # found a handler, eval and return result
-            return instance_eval(&block)
-          else
-            base = base.superclass
-          end
-        end
       end
-      raise boom if settings.show_exceptions? and keys == Exception
+      false
-      nil
     end
 
     def dump_errors!(boom)
-      msg = ["#{boom.class} - #{boom.message}:",
+      msg = ["#{boom.class} - #{boom.message}:", *boom.backtrace].join("\n\t")
-        *boom.backtrace].join("\n ")
       @env['rack.errors'].puts(msg)
     end
 
@@ -859,8 +852,9 @@ module Sinatra
       # Define a custom error handler. Optionally takes either an Exception
       # class, or an HTTP status code to specify which errors should be
       # handled.
-      def error(codes=Exception, &block)
+      def error(codes = Exception, &block)
-        Array(codes).each { |code| @errors[code] = block }
+        args = compile! "ERROR", //, block
+        Array(codes).each { |c| @errors[c] = args }
       end
 
       # Sugar for `error(404) { ... }`
@@ -945,7 +939,7 @@ module Sinatra
       # add a filter
       def add_filter(type, path = nil, options = {}, &block)
         path, options = //, path if path.respond_to?(:each_pair)
-        filters[type] << compile!(type, path, block, options)
+        filters[type] << compile!(type, path || //, block, options)
       end
 
       # Add a route condition. The route is considered non-matching when the
@@ -1025,7 +1019,7 @@ module Sinatra
 
         define_method(method_name, &block)
         unbound_method          = instance_method method_name
-        pattern, keys           = compile(path || //)
+        pattern, keys           = compile path
         conditions, @conditions = @conditions, []
         remove_method method_name
 

test/settings_test.rb

@@ -222,27 +222,30 @@ class SettingsTest < Test::Unit::TestCase
     end
 
     it 'does not override app-specified error handling when set to :after_handler' do
-      klass = Sinatra.new(Sinatra::Application)
+      ran = false
-      mock_app(klass) {
+      mock_app do
         set :show_exceptions, :after_handler
-        
+        error(RuntimeError) { ran = true }
-        error RuntimeError do
+        get('/') { raise RuntimeError }
-          'Big mistake !'
+      end
-        end
+
-        
-        get '/' do
-          raise RuntimeError
-        end  
-      }
-      
       get '/'
       assert_equal 500, status
-
+      assert ran
-      assert ! body.include?("<code>")
+    end
-      assert body.include? "Big mistake !"
+
-      
+    it 'does catch any other exceptions when set to :after_handler' do
+      ran = false
+      mock_app do
+        set :show_exceptions, :after_handler
+        error(RuntimeError) { ran = true }
+        get('/') { raise ArgumentError }
+      end
+
+      get '/'
+      assert_equal 500, status
+      assert !ran
     end
-    
   end
 
   describe 'dump_errors' do