diff --git a/CHANGELOG.md b/CHANGELOG.md index 230794ad62..64d5cb43ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ Next Release * [#510](https://github.com/intridea/grape/pull/510): Support lambda-based default values for params - [@myitcv](https://github.com/myitcv). * [#511](https://github.com/intridea/grape/pull/511): Add `required` option for OAuth2 middleware - [@bcm](https://github.com/bcm). +* [#520](https://github.com/intridea/grape/pull/520): Use `default_error_status` to specify the default status code returned from `error!` - [@salimane](https://github.com/salimane). +* [#525](https://github.com/intridea/grape/pull/525): The default status code returned from `error!` has been changed from 403 to 500 - [@dblock](https://github.com/dblock). * Your contribution here. #### Fixes diff --git a/README.md b/README.md index cbb2923f83..25a1a0b642 100644 --- a/README.md +++ b/README.md @@ -688,6 +688,19 @@ instead of a message. error!({ "error" => "unexpected error", "detail" => "missing widget" }, 500) ``` +### Default Error HTTP Status Code + +By default Grape returns a 500 status code from `error!`. You can change this with `default_error_status`. + +``` ruby +class API < Grape::API + default_error_status 400 + get '/example' do + error! "This should have http status code 400" + end +end +``` + ### Handling 404 For Grape to handle all the 404s for your API, it can be useful to use a catch-all. diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 884785f1c1..0665d7e1f7 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -202,8 +202,9 @@ def version # end user with the specified message. # # @param message [String] The message to display. - # @param status [Integer] the HTTP Status Code. Defaults to 403. - def error!(message, status = 403) + # @param status [Integer] the HTTP Status Code. Defaults to default_error_status, 500 if not set. + def error!(message, status = nil) + status = settings[:default_error_status] unless status throw :error, message: message, status: status end @@ -405,7 +406,7 @@ def build_middleware b.use Rack::Head b.use Grape::Middleware::Error, format: settings[:format], - default_status: settings[:default_error_status] || 403, + default_status: settings[:default_error_status] || 500, rescue_all: settings[:rescue_all], rescued_errors: aggregate_setting(:rescued_errors), default_error_formatter: settings[:default_error_formatter], diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 5223d2acc9..d145391137 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -5,7 +5,7 @@ module Middleware class Error < Base def default_options { - default_status: 403, # default status returned on error + default_status: 500, # default status returned on error default_message: "", format: :txt, formatters: {}, diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index c942a12b21..15e974bc15 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -993,7 +993,7 @@ def three raise "rain!" end get '/exception' - last_response.status.should eql 403 + last_response.status.should eql 500 end it 'rescues only certain errors if rescue_from is called with specific errors' do @@ -1002,7 +1002,7 @@ def three subject.get('/unrescued') { raise "beefcake" } get '/rescued' - last_response.status.should eql 403 + last_response.status.should eql 500 lambda { get '/unrescued' }.should raise_error end @@ -1477,7 +1477,16 @@ def self.call(object, env) raise "rain!" end get '/exception' - last_response.status.should eql 403 + last_response.status.should eql 500 + end + it 'uses the default error status in error!' do + subject.rescue_from :all + subject.default_error_status 400 + subject.get '/exception' do + error! "rain!" + end + get '/exception' + last_response.status.should eql 400 end end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index ec83fa4768..d2b66211a0 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -464,7 +464,7 @@ def app end get '/hey' - last_response.status.should == 403 + last_response.status.should == 500 last_response.body.should == "This is not valid." end diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index f40522fe69..a6f7ad4321 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_spec.rb @@ -31,10 +31,10 @@ def app last_response.body.should == 'Awesome stuff.' end - it 'defaults to a 403 status' do + it 'defaults to a 500 status' do ErrApp.error = {} get '/' - last_response.status.should == 403 + last_response.status.should == 500 end it 'has a default message' do diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index 49412daed3..6f31171c5d 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_spec.rb @@ -15,7 +15,7 @@ def call(env) # raises a hash error class ErrorHashApp class << self - def error!(message, status = 403) + def error!(message, status) throw :error, message: { error: message, detail: "missing widget" }, status: status end @@ -28,7 +28,7 @@ def call(env) # raises an error! class AccessDeniedApp class << self - def error!(message, status = 403) + def error!(message, status) throw :error, message: message, status: status end @@ -70,13 +70,13 @@ def call(env) last_response.body.should == "rain!" end - it 'defaults to a 403 status' do + it 'defaults to a 500 status' do @app ||= Rack::Builder.app do use Grape::Middleware::Error, rescue_all: true run ExceptionApp end get '/' - last_response.status.should == 403 + last_response.status.should == 500 end it 'is possible to specify a different default status code' do