Skip to content

Commit 927b4c4

Browse files
author
Michael Bleigh
committed
Refactor rescues into rescue_from for a better API.
1 parent 146934e commit 927b4c4

File tree

7 files changed

+126
-91
lines changed

7 files changed

+126
-91
lines changed

Gemfile.lock

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ GEM
2525
rack (>= 1.0.0)
2626
rack-test (0.5.4)
2727
rack (>= 1.0)
28+
rake (0.9.0)
2829
rspec (2.6.0)
2930
rspec-core (~> 2.6.0)
3031
rspec-expectations (~> 2.6.0)
@@ -46,5 +47,6 @@ DEPENDENCIES
4647
json_pure
4748
maruku
4849
rack-test
50+
rake
4951
rspec (~> 2.6.0)
5052
yard

Rakefile

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ require 'rubygems'
22
require 'bundler'
33
Bundler.setup :default, :test, :development
44

5-
require 'mg'
6-
MG.new('grape.gemspec')
5+
Bundler::GemHelper.install_tasks
76

87
require 'rspec/core/rake_task'
98
RSpec::Core::RakeTask.new(:spec) do |spec|

grape.gemspec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Gem::Specification.new do |s|
1919
s.add_runtime_dependency 'multi_json'
2020
s.add_runtime_dependency 'multi_xml'
2121

22+
s.add_development_dependency 'rake'
2223
s.add_development_dependency 'maruku'
2324
s.add_development_dependency 'yard'
2425
s.add_development_dependency 'rack-test'

lib/grape/api.rb

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,26 @@ def default_error_status(new_status = nil)
9090
new_status ? set(:default_error_status, new_status) : settings[:default_error_status]
9191
end
9292

93-
# Specify whether to rescue all errors.
94-
def rescue_all_errors(new_value = true)
95-
set(:rescue_all_errors, new_value)
96-
end
97-
98-
# Specify whether to include error backtrace with errors.
99-
def rescue_with_backtrace(new_value = true)
100-
set(:rescue_with_backtrace, new_value)
93+
# Allows you to rescue certain exceptions that occur to return
94+
# a grape error rather than raising all the way to the
95+
# server level.
96+
#
97+
# @example Rescue from custom exceptions
98+
# class ExampleAPI < Grape::API
99+
# class CustomError < StandardError; end
100+
#
101+
# rescue_from CustomError
102+
# end
103+
#
104+
# @overload rescue_from(*exception_classes, options = {})
105+
# @param [Array] exception_classes A list of classes that you want to rescue, or
106+
# the symbol :all to rescue from all exceptions.
107+
# @param [Hash] options Options for the rescue usage.
108+
# @option options [Boolean] :backtrace Include a backtrace in the rescue response.
109+
def rescue_from(*args)
110+
set(:rescue_options, args.pop) if args.last.is_a?(Hash)
111+
set(:rescue_all, true) and return if args.include?(:all)
112+
set(:rescued_errors, args)
101113
end
102114

103115
# Add helper methods that will be accessible from any
@@ -238,7 +250,12 @@ def nest(*blocks, &block)
238250

239251
def build_endpoint(&block)
240252
b = Rack::Builder.new
241-
b.use Grape::Middleware::Error, :default_status => settings[:default_error_status] || 403, :rescue => settings[:rescue_all_errors], :format => settings[:error_format] || :txt, :backtrace => settings[:rescue_with_backtrace]
253+
b.use Grape::Middleware::Error,
254+
:default_status => settings[:default_error_status] || 403,
255+
:rescue_all => settings[:rescue_all],
256+
:rescued_errors => settings[:rescued_errors],
257+
:format => settings[:error_format] || :txt,
258+
:rescue_options => settings[:rescue_options]
242259
b.use Rack::Auth::Basic, settings[:auth][:realm], &settings[:auth][:proc] if settings[:auth] && settings[:auth][:type] == :http_basic
243260
b.use Grape::Middleware::Prefixer, :prefix => prefix if prefix
244261
b.use Grape::Middleware::Versioner, :versions => (version if version.is_a?(Array)) if version

lib/grape/middleware/error.rb

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
require 'grape/middleware/base'
2+
require 'multi_json'
23

34
module Grape
45
module Middleware
@@ -7,11 +8,13 @@ class Error < Base
78
def default_options
89
{
910
:default_status => 403, # default status returned on error
10-
:rescue => true, # true to rescue all exceptions
1111
:default_message => "",
1212
:format => :txt,
1313
:formatters => {},
14-
:backtrace => false, # true to display backtrace
14+
15+
:rescue_all => false, # true to rescue all exceptions
16+
:rescue_options => {:backtrace => false}, # true to display backtrace
17+
:rescued_errors => []
1518
}
1619
end
1720

@@ -38,15 +41,15 @@ def formatter_for(api_format)
3841

3942
def format_json(message, backtrace)
4043
result = message.is_a?(Hash) ? message : { :error => message }
41-
if (options[:backtrace] && backtrace && ! backtrace.empty?)
44+
if (options[:rescue_options] || {})[:backtrace] && backtrace && ! backtrace.empty?
4245
result = result.merge({ :backtrace => backtrace })
4346
end
44-
result.to_json
47+
MultiJson.encode(result)
4548
end
4649

4750
def format_txt(message, backtrace)
48-
result = message.is_a?(Hash) ? message.to_json : message
49-
if options[:backtrace] && backtrace && ! backtrace.empty?
51+
result = message.is_a?(Hash) ? MultiJson.encode(message) : message
52+
if (options[:rescue_options] || {})[:backtrace] && backtrace && ! backtrace.empty?
5053
result += "\r\n "
5154
result += backtrace.join("\r\n ")
5255
end
@@ -61,7 +64,7 @@ def call!(env)
6164
return @app.call(@env)
6265
})
6366
rescue Exception => e
64-
raise unless options[:rescue]
67+
raise unless options[:rescue_all] || (options[:rescued_errors] || []).include?(e.class)
6568
error_response({ :message => e.message, :backtrace => e.backtrace })
6669
end
6770

spec/grape/api_spec.rb

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -472,46 +472,58 @@ def two
472472
end
473473
end
474474

475-
describe ".rescue_all_errors" do
476-
it 'should not rescue all errors when rescue_all_errors is false' do
477-
subject.rescue_all_errors false
475+
describe ".rescue_from" do
476+
it 'should not rescue errors when rescue_from is not set' do
478477
subject.get '/exception' do
479478
raise "rain!"
480479
end
481480
lambda { get '/exception' }.should raise_error
482481
end
483-
it 'should rescue all errors' do
484-
subject.rescue_all_errors true
482+
483+
it 'should rescue all errors if rescue_from :all is called' do
484+
subject.rescue_from :all
485485
subject.get '/exception' do
486486
raise "rain!"
487487
end
488488
get '/exception'
489489
last_response.status.should eql 403
490490
end
491+
492+
it 'should rescue only certain errors if rescue_from is called with specific errors' do
493+
subject.rescue_from ArgumentError
494+
subject.get('/rescued'){ raise ArgumentError }
495+
subject.get('/unrescued'){ raise "beefcake" }
496+
497+
get '/rescued'
498+
last_response.status.should eql 403
499+
500+
lambda{ get '/unrescued' }.should raise_error
501+
end
491502
end
492503

493504
describe ".error_format" do
494505
it 'should rescue all errors and return :txt' do
495-
subject.rescue_all_errors true
506+
subject.rescue_from :all
496507
subject.error_format :txt
497508
subject.get '/exception' do
498509
raise "rain!"
499510
end
500511
get '/exception'
501512
last_response.body.should eql "rain!"
502513
end
514+
503515
it 'should rescue all errros and return :txt with backtrace' do
504-
subject.rescue_all_errors true
516+
subject.rescue_from :all, :backtrace => true
505517
subject.error_format :txt
506-
subject.rescue_with_backtrace true
507518
subject.get '/exception' do
508519
raise "rain!"
509520
end
510521
get '/exception'
511522
last_response.body.start_with?("rain!\r\n").should be_true
512523
end
524+
513525
it 'should rescue all errors and return :json' do
514-
subject.rescue_all_errors true
526+
subject.rescue_from :all
515527
subject.error_format :json
516528
subject.get '/exception' do
517529
raise "rain!"
@@ -520,9 +532,8 @@ def two
520532
last_response.body.should eql '{"error":"rain!"}'
521533
end
522534
it 'should rescue all errors and return :json with backtrace' do
523-
subject.rescue_all_errors true
535+
subject.rescue_from :all, :backtrace => true
524536
subject.error_format :json
525-
subject.rescue_with_backtrace true
526537
subject.get '/exception' do
527538
raise "rain!"
528539
end
@@ -551,7 +562,7 @@ def two
551562

552563
describe ".default_error_status" do
553564
it 'should allow setting default_error_status' do
554-
subject.rescue_all_errors true
565+
subject.rescue_from :all
555566
subject.default_error_status 200
556567
subject.get '/exception' do
557568
raise "rain!"
@@ -560,7 +571,7 @@ def two
560571
last_response.status.should eql 200
561572
end
562573
it 'should have a default error status' do
563-
subject.rescue_all_errors true
574+
subject.rescue_from :all
564575
subject.get '/exception' do
565576
raise "rain!"
566577
end

spec/grape/middleware/exception_spec.rb

Lines changed: 62 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -38,80 +38,82 @@ def call(env)
3838
def app
3939
@app
4040
end
41-
42-
it 'should set the message appropriately' do
41+
42+
it 'should not trap errors by default' do
4343
@app ||= Rack::Builder.app do
4444
use Grape::Middleware::Error
4545
run ExceptionApp
4646
end
47-
get '/'
48-
last_response.body.should == "rain!"
47+
lambda { get '/' }.should raise_error
4948
end
5049

51-
it 'should default to a 403 status' do
52-
@app ||= Rack::Builder.app do
53-
use Grape::Middleware::Error
54-
run ExceptionApp
50+
context 'with rescue_all set to true' do
51+
it 'should set the message appropriately' do
52+
@app ||= Rack::Builder.app do
53+
use Grape::Middleware::Error, :rescue_all => true
54+
run ExceptionApp
55+
end
56+
get '/'
57+
last_response.body.should == "rain!"
5558
end
56-
get '/'
57-
last_response.status.should == 403
58-
end
5959

60-
it 'should be possible to specify a different default status code' do
61-
@app ||= Rack::Builder.app do
62-
use Grape::Middleware::Error, :default_status => 500
63-
run ExceptionApp
64-
end
65-
get '/'
66-
last_response.status.should == 500
67-
end
68-
69-
it 'should be possible to disable exception trapping' do
70-
@app ||= Rack::Builder.app do
71-
use Grape::Middleware::Error, :rescue => false
72-
run ExceptionApp
60+
it 'should default to a 403 status' do
61+
@app ||= Rack::Builder.app do
62+
use Grape::Middleware::Error, :rescue_all => true
63+
run ExceptionApp
64+
end
65+
get '/'
66+
last_response.status.should == 403
7367
end
74-
lambda { get '/' }.should raise_error
75-
end
7668

77-
it 'should be possible to return errors in json format' do
78-
@app ||= Rack::Builder.app do
79-
use Grape::Middleware::Error, :format => :json
80-
run ExceptionApp
69+
it 'should be possible to specify a different default status code' do
70+
@app ||= Rack::Builder.app do
71+
use Grape::Middleware::Error, :rescue_all => true, :default_status => 500
72+
run ExceptionApp
73+
end
74+
get '/'
75+
last_response.status.should == 500
8176
end
82-
get '/'
83-
last_response.body.should == '{"error":"rain!"}'
84-
end
85-
86-
it 'should be possible to return hash errors in json format' do
87-
@app ||= Rack::Builder.app do
88-
use Grape::Middleware::Error, :format => :json
89-
run ErrorHashApp
77+
78+
it 'should be possible to return errors in json format' do
79+
@app ||= Rack::Builder.app do
80+
use Grape::Middleware::Error, :rescue_all => true, :format => :json
81+
run ExceptionApp
82+
end
83+
get '/'
84+
last_response.body.should == '{"error":"rain!"}'
9085
end
91-
get '/'
92-
last_response.body.should == '{"error":"rain!","detail":"missing widget"}'
93-
end
9486

95-
it 'should be possible to specify a custom formatter' do
96-
@app ||= Rack::Builder.app do
97-
use Grape::Middleware::Error,
98-
:format => :custom,
99-
:formatters => {
100-
:custom => lambda { |message, backtrace| { :custom_formatter => message } }
101-
}
102-
run ExceptionApp
87+
it 'should be possible to return hash errors in json format' do
88+
@app ||= Rack::Builder.app do
89+
use Grape::Middleware::Error, :rescue_all => true, :format => :json
90+
run ErrorHashApp
91+
end
92+
get '/'
93+
last_response.body.should == '{"error":"rain!","detail":"missing widget"}'
10394
end
104-
get '/'
105-
last_response.body.should == '{:custom_formatter=>"rain!"}'
106-
end
107-
108-
it 'should not trap regular error! codes' do
109-
@app ||= Rack::Builder.app do
110-
use Grape::Middleware::Error
111-
run AccessDeniedApp
95+
96+
it 'should be possible to specify a custom formatter' do
97+
@app ||= Rack::Builder.app do
98+
use Grape::Middleware::Error,
99+
:rescue_all => true,
100+
:format => :custom,
101+
:formatters => {
102+
:custom => lambda { |message, backtrace| { :custom_formatter => message } }
103+
}
104+
run ExceptionApp
105+
end
106+
get '/'
107+
last_response.body.should == '{:custom_formatter=>"rain!"}'
112108
end
113-
get '/'
114-
last_response.status.should == 401
109+
110+
it 'should not trap regular error! codes' do
111+
@app ||= Rack::Builder.app do
112+
use Grape::Middleware::Error
113+
run AccessDeniedApp
114+
end
115+
get '/'
116+
last_response.status.should == 401
117+
end
115118
end
116-
117119
end

0 commit comments

Comments
 (0)