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

Capture exception messages of raised NotFound and BadRequest #1210

Merged
merged 1 commit into from
Jan 14, 2017
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
Capture exception messages of raised NotFound and BadRequest
* Partially reverts b4fdb72 and un-reverts 8f8df53 (oy vey!)
  • Loading branch information
mwpastore committed Jan 14, 2017
commit 0393f55af635ea176b03c06d271e134d850923d5
10 changes: 9 additions & 1 deletion lib/sinatra/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,11 @@ def not_found?
status == 404
end

# whether or not the status is set to 400
def bad_request?
status == 400
end

# Generates a Time object from the given value.
# Used by #expires and #last_modified.
def time_for(value)
Expand Down Expand Up @@ -1133,12 +1138,15 @@ def handle_exception!(boom)

status(500) unless status.between? 400, 599

boom_message = boom.message if boom.message && boom.message != boom.class.name
if server_error?
dump_errors! boom if settings.dump_errors?
raise boom if settings.show_exceptions? and settings.show_exceptions != :after_handler
elsif not_found?
headers['X-Cascade'] = 'pass' if settings.x_cascade?
body '<h1>Not Found</h1>'
body boom_message || '<h1>Not Found</h1>'
elsif bad_request?
body boom_message || '<h1>Bad Request</h1>'
end

res = error_block!(boom.class, boom) || error_block!(status, boom)
Expand Down
17 changes: 17 additions & 0 deletions test/helpers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,23 @@ def status_app(code, &block)
end
end

describe 'bad_request?' do
it 'is true for status == 400' do
status_app(400) { bad_request? }
assert_body 'true'
end

it 'is false for status gt 400' do
status_app(401) { bad_request? }
assert_body 'false'
end

it 'is false for status lt 400' do
status_app(399) { bad_request? }
assert_body 'false'
end
end

describe 'not_found?' do
it 'is true for status == 404' do
status_app(404) { not_found? }
Expand Down
28 changes: 27 additions & 1 deletion test/routing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class RoutingTest < Minitest::Test

request = Rack::MockRequest.new(@app)
response = request.request('GET', '/foo?bar=&bar[]=', {})
assert_equal 400, response.status
assert response.bad_request?
end

it "404s when no route satisfies the request" do
Expand Down Expand Up @@ -175,6 +175,32 @@ class RoutingTest < Minitest::Test
assert_equal 404, status
end

it "captures the exception message of a raised NotFound" do
mock_app {
get '/' do
raise Sinatra::NotFound, "This is not a drill"
end
}

get "/"
assert_equal "19", response["Content-Length"]
assert_equal 404, status
assert_equal "This is not a drill", response.body
end

it "captures the exception message of a raised BadRequest" do
mock_app {
get '/' do
raise Sinatra::BadRequest, "This is not a drill either"
end
}

get "/"
assert_equal "26", response["Content-Length"]
assert_equal 400, status
assert_equal "This is not a drill either", response.body
end

it "uses 404 error handler for not matching route" do
mock_app {
not_found do
Expand Down