kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

API only apps: Preserve request format for HTML requests too

Browse source 
- Earlier we were responding with JSON format for HTML requests in a API
  app.
- Now we will respond with HTML format for such requests in API apps.
- Also earlier we were not testing the API app's JSON requests
  properly. We were actually sending HTML requests. Now we send correct
  JSON requests. Also added more test coverage.
- Based on the discussion from this commit -
  05d89410bf.

[Prathamesh Sonpatki, Jorge Bejar]
This commit is contained in:
Prathamesh Sonpatki 2016-05-07 20:44:38 -05:00
parent 40d5c3370f
commit c33bda875e
No known key found for this signature in database
GPG key ID: 8B90F6B89E2BCB71
2 changed files with 62 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
View file

@ -67,18 +67,19 @@ module ActionDispatch
log_error(request, wrapper)
if request.get_header('action_dispatch.show_detailed_exceptions')
case @response_format
when :api
render_for_api_application(request, wrapper)
when :default
render_for_default_application(request, wrapper)
content_type = request.formats.first
if api_request?(content_type)
render_for_api_request(content_type, wrapper)
else
render_for_browser_request(request, wrapper)
end
else
raise exception
end
end
def render_for_default_application(request, wrapper)
def render_for_browser_request(request, wrapper)
template = create_template(request, wrapper)
file = "rescues/#{wrapper.rescue_template}"
@ -92,7 +93,7 @@ module ActionDispatch
render(wrapper.status_code, body, format)
end
def render_for_api_application(request, wrapper)
def render_for_api_request(content_type, wrapper)
body = {
status: wrapper.status_code,
error: Rack::Utils::HTTP_STATUS_CODES.fetch(
@ -103,7 +104,6 @@ module ActionDispatch
traces: wrapper.traces
}
content_type = request.formats.first
to_format = "to_#{content_type.to_sym}"
if content_type && body.respond_to?(to_format)
@ -181,5 +181,9 @@ module ActionDispatch
ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes)
end
end
def api_request?(content_type)
@response_format == :api && !content_type.html?
end
end
end

82
actionpack/test/dispatch/debug_exceptions_test.rb
View file

@ -27,37 +27,37 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
env['action_dispatch.show_detailed_exceptions'] = @detailed
req = ActionDispatch::Request.new(env)
case req.path
when "/pass"
when %r{/pass}
[404, { "X-Cascade" => "pass" }, self]
when "/not_found"
when %r{/not_found}
raise AbstractController::ActionNotFound
when "/runtime_error"
when %r{/runtime_error}
raise RuntimeError
when "/method_not_allowed"
when %r{/method_not_allowed}
raise ActionController::MethodNotAllowed
when "/unknown_http_method"
when %r{/unknown_http_method}
raise ActionController::UnknownHttpMethod
when "/not_implemented"
when %r{/not_implemented}
raise ActionController::NotImplemented
when "/unprocessable_entity"
when %r{/unprocessable_entity}
raise ActionController::InvalidAuthenticityToken
when "/not_found_original_exception"
when %r{/not_found_original_exception}
begin
raise AbstractController::ActionNotFound.new
rescue
raise ActionView::Template::Error.new('template')
end
when "/missing_template"
when %r{/missing_template}
raise ActionView::MissingTemplate.new(%w(foo), 'foo/index', %w(foo), false, 'mailer')
when "/bad_request"
when %r{/bad_request}
raise ActionController::BadRequest
when "/missing_keys"
when %r{/missing_keys}
raise ActionController::UrlGenerationError, "No route matches"
when "/parameter_missing"
when %r{/parameter_missing}
raise ActionController::ParameterMissing, :missing_param_key
when "/original_syntax_error"
when %r{/original_syntax_error}
eval 'broke_syntax =' # `eval` need for raise native SyntaxError at runtime
when "/syntax_error_into_view"
when %r{/syntax_error_into_view}
begin
eval 'broke_syntax ='
rescue Exception
@ -67,7 +67,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
{})
raise ActionView::Template::Error.new(template)
end
when "/framework_raises"
when %r{/framework_raises}
method_that_raises
else
raise "puke!"
@ -212,61 +212,60 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
assert_match(/ActionController::ParameterMissing/, body)
end
test "rescue with json error for API request" do
test "rescue with JSON error for JSON API request" do
@app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api)
get "/", headers: { 'action_dispatch.show_exceptions' => true }
get "/", headers: { 'action_dispatch.show_exceptions' => true }, as: :json
assert_response 500
assert_no_match(/<header>/, body)
assert_no_match(/<body>/, body)
assert_equal "application/json", response.content_type
assert_match(/RuntimeError: puke/, body)
get "/not_found", headers: { 'action_dispatch.show_exceptions' => true }
get "/not_found", headers: { 'action_dispatch.show_exceptions' => true }, as: :json
assert_response 404
assert_no_match(/<body>/, body)
assert_equal "application/json", response.content_type
assert_match(/#{AbstractController::ActionNotFound.name}/, body)
get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true }
get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true }, as: :json
assert_response 405
assert_no_match(/<body>/, body)
assert_equal "application/json", response.content_type
assert_match(/ActionController::MethodNotAllowed/, body)
get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true }
get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true }, as: :json
assert_response 405
assert_no_match(/<body>/, body)
assert_equal "application/json", response.content_type
assert_match(/ActionController::UnknownHttpMethod/, body)
get "/bad_request", headers: { 'action_dispatch.show_exceptions' => true }
get "/bad_request", headers: { 'action_dispatch.show_exceptions' => true }, as: :json
assert_response 400
assert_no_match(/<body>/, body)
assert_equal "application/json", response.content_type
assert_match(/ActionController::BadRequest/, body)
get "/parameter_missing", headers: { 'action_dispatch.show_exceptions' => true }
get "/parameter_missing", headers: { 'action_dispatch.show_exceptions' => true }, as: :json
assert_response 400
assert_no_match(/<body>/, body)
assert_equal "application/json", response.content_type
assert_match(/ActionController::ParameterMissing/, body)
end
test "rescue with json on API request returns only allowed formats or json as a fallback" do
test "rescue with HTML format for HTML API request" do
@app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api)
get "/index.json", headers: { 'action_dispatch.show_exceptions' => true }
assert_response 500
assert_equal "application/json", response.content_type
assert_match(/RuntimeError: puke/, body)
get "/index.html", headers: { 'action_dispatch.show_exceptions' => true }
assert_response 500
assert_no_match(/<header>/, body)
assert_no_match(/<body>/, body)
assert_equal "application/json", response.content_type
assert_match(/RuntimeError: puke/, body)
assert_match(/<header>/, body)
assert_match(/<body>/, body)
assert_equal "text/html", response.content_type
assert_match(/puke/, body)
end
test "rescue with XML format for XML API requests" do
@app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api)
get "/index.xml", headers: { 'action_dispatch.show_exceptions' => true }
assert_response 500
@ -274,6 +273,25 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
assert_match(/RuntimeError: puke/, body)
end
test "rescue with JSON format as fallback if API request format is not supported" do
begin
Mime::Type.register 'text/wibble', :wibble
ActionDispatch::IntegrationTest.register_encoder(:wibble,
param_encoder: -> params { params })
@app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api)
get "/index", headers: { 'action_dispatch.show_exceptions' => true }, as: :wibble
assert_response 500
assert_equal "application/json", response.content_type
assert_match(/RuntimeError: puke/, body)
ensure
Mime::Type.unregister :wibble
end
end
test "does not show filtered parameters" do
@app = DevelopmentApp