Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed error_format, will match the request content-type matched format. #285

Merged
merged 1 commit into from
Dec 3, 2012
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [#273](https://github.com/intridea/grape/pull/273): Disabled formatting via `serializable_hash` and added support for `format :serializable_hash` in API settings - [@dblock](https://github.com/dblock).
* [#277](https://github.com/intridea/grape/pull/277): Added a DSL to declare `formatter` in API settings - [@tim-vandecasteele](https://github.com/tim-vandecasteele).
* [#284](https://github.com/intridea/grape/pull/284): Added a DSL to declare `error_formatter` in API settings - [@dblock](https://github.com/dblock).
* [#285](https://github.com/intridea/grape/pull/285): Removed `error_format` from API settings, now matches request format - [@dblock](https://github.com/dblock).
* Your contribution here.

0.2.2
Expand Down
10 changes: 1 addition & 9 deletions README.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -492,14 +492,7 @@ class Twitter::API < Grape::API
end
```

The error format can be specified using `error_format`. Available formats are
`:json`, `:xml` and `:txt` (default).

``` ruby
class Twitter::API < Grape::API
error_format :json
end
```
The error format will match the request format. See "Content-Types" below.

Custom error formatters for existing and additional types can be defined with a proc.

Expand All @@ -519,7 +512,6 @@ module CustomFormatter
end

class Twitter::API < Grape::API
error_format :custom
error_formatter :custom, CustomFormatter
end
```
Expand Down
6 changes: 0 additions & 6 deletions lib/grape/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,6 @@ def error_formatter(format, new_formatter)
settings.imbue(:error_formatters, format.to_sym => new_formatter)
end

# Specify the format for error messages.
# May be `:json` or `:txt` (default).
def error_format(new_format = nil)
new_format ? set(:error_format, new_format.to_sym) : settings[:error_format]
end

# Specify additional content-types, e.g.:
# content_type :xls, 'application/vnd.ms-excel'
def content_type(key, val)
Expand Down
1 change: 0 additions & 1 deletion lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,6 @@ def build_middleware
:default_status => settings[:default_error_status] || 403,
:rescue_all => settings[:rescue_all],
:rescued_errors => aggregate_setting(:rescued_errors),
:format => settings[:error_format] || :txt,
:error_formatters => settings[:error_formatters],
:rescue_options => settings[:rescue_options],
:rescue_handlers => merged_setting(:rescue_handlers)
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/middleware/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def content_types
end

def content_type
content_types[options[:format]] || 'text/html'
content_types[env['api.format'] || options[:format]] || 'text/html'
end

def mime_types
Expand Down
5 changes: 3 additions & 2 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ def rack_response(message, status = options[:default_status], headers = { 'Conte
end

def format_message(message, backtrace, status)
formatter = Grape::ErrorFormatter::Base.formatter_for(options[:format], options)
throw :error, :status => 406, :message => "The requested format #{options[:format]} is not supported." unless formatter
format = env['api.format'] || options[:format]
formatter = Grape::ErrorFormatter::Base.formatter_for(format, options)
throw :error, :status => 406, :message => "The requested format #{format} is not supported." unless formatter
formatter.call(message, backtrace, options)
end

Expand Down
16 changes: 8 additions & 8 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def subject.enable_root_route!
end

it 'should set content type for error' do
subject.error_format :json
subject.format :json
subject.get('/error') { error!('error in json', 500) }
get '/error.json'
last_response.headers['Content-Type'].should eql 'application/json'
Expand Down Expand Up @@ -868,10 +868,10 @@ class CommunicationError < RuntimeError; end
end
end

describe ".error_format" do
describe ".format for error" do
it 'should rescue all errors and return :txt' do
subject.rescue_from :all
subject.error_format :txt
subject.format :txt
subject.get '/exception' do
raise "rain!"
end
Expand All @@ -881,7 +881,7 @@ class CommunicationError < RuntimeError; end

it 'should rescue all errors and return :txt with backtrace' do
subject.rescue_from :all, :backtrace => true
subject.error_format :txt
subject.format :txt
subject.get '/exception' do
raise "rain!"
end
Expand Down Expand Up @@ -910,7 +910,7 @@ def self.call(message, backtrace, options)

it 'should rescue all errors and return :json' do
subject.rescue_from :all
subject.error_format :json
subject.format :json
subject.get '/exception' do
raise "rain!"
end
Expand All @@ -919,7 +919,7 @@ def self.call(message, backtrace, options)
end
it 'should rescue all errors and return :json with backtrace' do
subject.rescue_from :all, :backtrace => true
subject.error_format :json
subject.format :json
subject.get '/exception' do
raise "rain!"
end
Expand All @@ -929,15 +929,15 @@ def self.call(message, backtrace, options)
json["backtrace"].length.should > 0
end
it 'should rescue error! and return txt' do
subject.error_format :txt
subject.format :txt
subject.get '/error' do
error!("Access Denied", 401)
end
get '/error'
last_response.body.should eql "Access Denied"
end
it 'should rescue error! and return json' do
subject.error_format :json
subject.format :json
subject.get '/error' do
error!("Access Denied", 401)
end
Expand Down
40 changes: 20 additions & 20 deletions spec/grape/validations/presence_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class API < Grape::API
post do
{:ret => params[:id]}
end

params do
requires :name, :company
end
Expand Down Expand Up @@ -52,7 +52,7 @@ class API < Grape::API
end
end
end

def app
ValidationsSpec::PresenceValidatorSpec::API
end
Expand All @@ -62,30 +62,30 @@ def app
last_response.status.should == 200
last_response.body.should == "All the bacon"
end

it 'validates id' do
post('/')
last_response.status.should == 400
last_response.body.should == "missing parameter: id"
last_response.body.should == '{"error":"missing parameter: id"}'

post('/', {}, 'rack.input' => StringIO.new('{"id" : "a56b"}'))
last_response.body.should == 'invalid parameter: id'
last_response.body.should == '{"error":"invalid parameter: id"}'
last_response.status.should == 400

post('/', {}, 'rack.input' => StringIO.new('{"id" : 56}'))
last_response.body.should == '{"ret":56}'
last_response.status.should == 201
end

it 'validates name, company' do
get('/')
last_response.status.should == 400
last_response.body.should == "missing parameter: name"
last_response.body.should == '{"error":"missing parameter: name"}'

get('/', :name => "Bob")
last_response.status.should == 400
last_response.body.should == "missing parameter: company"
last_response.body.should == '{"error":"missing parameter: company"}'

get('/', :name => "Bob", :company => "TestCorp")
last_response.status.should == 200
last_response.body.should == "Hello"
Expand All @@ -94,11 +94,11 @@ def app
it 'validates nested parameters' do
get('/nested')
last_response.status.should == 400
last_response.body.should == "missing parameter: first_name"
last_response.body.should == '{"error":"missing parameter: first_name"}'

get('/nested', :user => {:first_name => "Billy"})
last_response.status.should == 400
last_response.body.should == "missing parameter: last_name"
last_response.body.should == '{"error":"missing parameter: last_name"}'

get('/nested', :user => {:first_name => "Billy", :last_name => "Bob"})
last_response.status.should == 200
Expand All @@ -108,27 +108,27 @@ def app
it 'validates triple nested parameters' do
get('/nested_triple')
last_response.status.should == 400
last_response.body.should == "missing parameter: admin_name"
last_response.body.should == '{"error":"missing parameter: admin_name"}'

get('/nested_triple', :user => {:first_name => "Billy"})
last_response.status.should == 400
last_response.body.should == "missing parameter: admin_name"
last_response.body.should == '{"error":"missing parameter: admin_name"}'

get('/nested_triple', :admin => {:super => {:first_name => "Billy"}})
last_response.status.should == 400
last_response.body.should == "missing parameter: admin_name"
last_response.body.should == '{"error":"missing parameter: admin_name"}'

get('/nested_triple', :super => {:user => {:first_name => "Billy", :last_name => "Bob"}})
last_response.status.should == 400
last_response.body.should == "missing parameter: admin_name"
last_response.body.should == '{"error":"missing parameter: admin_name"}'

get('/nested_triple', :admin => {:super => {:user => {:first_name => "Billy"}}})
last_response.status.should == 400
last_response.body.should == "missing parameter: admin_name"
last_response.body.should == '{"error":"missing parameter: admin_name"}'

get('/nested_triple', :admin => { :admin_name => 'admin', :super => {:user => {:first_name => "Billy"}}})
last_response.status.should == 400
last_response.body.should == "missing parameter: last_name"
last_response.body.should == '{"error":"missing parameter: last_name"}'

get('/nested_triple', :admin => { :admin_name => 'admin', :super => {:user => {:first_name => "Billy", :last_name => "Bob"}}})
last_response.status.should == 200
Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))
$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), 'support'))

$stdout = StringIO.new
# $stdout = StringIO.new

require 'grape'

Expand Down