From 5dbc6b612686519cdd9bd0de367ac88e098c4c5e Mon Sep 17 00:00:00 2001 From: dblock Date: Sun, 2 Dec 2012 11:24:57 -0500 Subject: [PATCH] Removed error_format, will match the request content-type matched format. --- CHANGELOG.markdown | 1 + README.markdown | 10 +------ lib/grape/api.rb | 6 ---- lib/grape/endpoint.rb | 1 - lib/grape/middleware/base.rb | 2 +- lib/grape/middleware/error.rb | 5 ++-- spec/grape/api_spec.rb | 16 +++++----- spec/grape/validations/presence_spec.rb | 40 ++++++++++++------------- spec/spec_helper.rb | 2 +- 9 files changed, 35 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.markdown b/CHANGELOG.markdown index 96c6665492..24cfa45f15 100644 --- a/CHANGELOG.markdown +++ b/CHANGELOG.markdown @@ -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 diff --git a/README.markdown b/README.markdown index 58ebb836d1..29adba3536 100644 --- a/README.markdown +++ b/README.markdown @@ -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. @@ -519,7 +512,6 @@ module CustomFormatter end class Twitter::API < Grape::API - error_format :custom error_formatter :custom, CustomFormatter end ``` diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 41e196c770..6ab4675bc8 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -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) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 5030714c5f..48f97a8e4b 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -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) diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index e5c35ed164..2623e94bff 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -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 diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 4380012595..50d7d03a6e 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -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 diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index a956e891df..8445035ca1 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -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' @@ -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 @@ -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 @@ -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 @@ -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 @@ -929,7 +929,7 @@ 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 @@ -937,7 +937,7 @@ def self.call(message, backtrace, options) 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 diff --git a/spec/grape/validations/presence_spec.rb b/spec/grape/validations/presence_spec.rb index 6680a817e9..1397dfe6ce 100644 --- a/spec/grape/validations/presence_spec.rb +++ b/spec/grape/validations/presence_spec.rb @@ -19,7 +19,7 @@ class API < Grape::API post do {:ret => params[:id]} end - + params do requires :name, :company end @@ -52,7 +52,7 @@ class API < Grape::API end end end - + def app ValidationsSpec::PresenceValidatorSpec::API end @@ -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" @@ -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 @@ -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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4c5cdcadec..9b3e9444aa 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -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'