Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

refactor error handling (removes one instance_eval)

  • Loading branch information...
commit 67ae9be4f2142506c009addd97fa5d576cdaba6f 1 parent 93152ec
@rkh rkh authored
Showing with 34 additions and 37 deletions.
  1. +15 −21 lib/sinatra/base.rb
  2. +19 −16 test/settings_test.rb
View
36 lib/sinatra/base.rb
@@ -754,36 +754,29 @@ def handle_exception!(boom)
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)
- keys.each do |key|
- base = settings
- while base.respond_to?(:errors)
- if block = base.errors[key]
- # found a handler, eval and return result
- return instance_eval(&block)
- else
- base = base.superclass
- end
- end
+ def error_block!(key)
+ base = settings
+ while base.respond_to?(:errors)
+ next base = base.superclass unless args = base.errors[key]
+ return process_route(*args)
end
- raise boom if settings.show_exceptions? and keys == Exception
- nil
+ false
end
def dump_errors!(boom)
- msg = ["#{boom.class} - #{boom.message}:",
- *boom.backtrace].join("\n ")
+ msg = ["#{boom.class} - #{boom.message}:", *boom.backtrace].join("\n\t")
@env['rack.errors'].puts(msg)
end
@@ -859,8 +852,9 @@ def disable(*opts)
# 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)
- Array(codes).each { |code| @errors[code] = block }
+ def error(codes = Exception, &block)
+ args = compile! "ERROR", //, block
+ Array(codes).each { |c| @errors[c] = args }
end
# Sugar for `error(404) { ... }`
@@ -945,7 +939,7 @@ def after(path = nil, options = {}, &block)
# 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 @@ def compile!(verb, path, block, options = {})
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
View
35 test/settings_test.rb
@@ -222,27 +222,30 @@ def foo=(value)
end
it 'does not override app-specified error handling when set to :after_handler' do
- klass = Sinatra.new(Sinatra::Application)
- mock_app(klass) {
+ ran = false
+ mock_app do
set :show_exceptions, :after_handler
-
- error RuntimeError do
- 'Big mistake !'
- end
-
- get '/' do
- raise RuntimeError
- end
- }
-
+ error(RuntimeError) { ran = true }
+ get('/') { raise RuntimeError }
+ end
+
get '/'
assert_equal 500, status
+ assert ran
+ end
+
+ 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
- assert ! body.include?("<code>")
- assert body.include? "Big mistake !"
-
+ get '/'
+ assert_equal 500, status
+ assert !ran
end
-
end
describe 'dump_errors' do
Please sign in to comment.
Something went wrong with that request. Please try again.