Merge pull request #24912 from prathamesh-sonpatki/api-fix-response-format

API only apps: Preserve request format for HTML requests too

2 files changed, 62 insertions, 40 deletions

diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
index 51a471fb23..5f758d641a 100644
--- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -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
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index f6e0076a6a..5a39db145e 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -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!"
@@ -205,61 +205,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
@@ -267,6 +266,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