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

Default error status 500 #525

Merged
merged 2 commits into from
Dec 4, 2013
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 4 additions & 3 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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],
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
Expand Down
15 changes: 12 additions & 3 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/grape/middleware/error_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions spec/grape/middleware/exception_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down