Merge pull request #20831 from jmbejar/rails-api-json-error-response

Rails API: Ability to return error responses in json format also in development

actionpack/CHANGELOG.md

+*   Add a `response_format` option to `ActionDispatch::DebugExceptions`
+    to configure the format of the response when errors occur in
+    development mode.
+
+    If `response_format` is `:default` the debug info will be rendered
+    in an HTML page. In the other hand, if the provided value is `:api`
+    the debug info will be rendered in the original response format.
+
+    *Jorge Bejar*
+
 *   Change the `protect_from_forgery` prepend default to `false`
 
     Per this comment

actionpack/lib/action_dispatch/http/mime_negotiation.rb

@@ -67,6 +67,8 @@ def formats
 
           v = if params_readable
             Array(Mime[parameters[:format]])
+          elsif format = format_from_path_extension
+            Array(Mime[format])
           elsif use_accept_header && valid_accept_header
             accepts
           elsif xhr?
@@ -160,6 +162,13 @@ def valid_accept_header
       def use_accept_header
         !self.class.ignore_accept_header
       end
+
+      def format_from_path_extension
+        path = @env['action_dispatch.original_path'] || @env['PATH_INFO']
+        if match = path && path.match(/\.(\w+)\z/)
+          match.captures.first
+        end
+      end
     end
   end
 end

actionpack/lib/action_dispatch/middleware/debug_exceptions.rb

@@ -38,9 +38,10 @@ def debug_hash(object)
       end
     end
 
-    def initialize(app, routes_app = nil)
-      @app        = app
-      @routes_app = routes_app
+    def initialize(app, routes_app = nil, response_format = :default)
+      @app             = app
+      @routes_app      = routes_app
+      @response_format = response_format
     end
 
     def call(env)
@@ -66,41 +67,79 @@ def render_exception(request, exception)
       log_error(request, wrapper)
 
       if request.get_header('action_dispatch.show_detailed_exceptions')
-        traces = wrapper.traces
-
-        trace_to_show = 'Application Trace'
-        if traces[trace_to_show].empty? && wrapper.rescue_template != 'routing_error'
-          trace_to_show = 'Full Trace'
+        case @response_format
+        when :api
+          render_for_api_application(request, wrapper)
+        when :default
+          render_for_default_application(request, wrapper)
         end
+      else
+        raise exception
+      end
+    end
 
-        if source_to_show = traces[trace_to_show].first
-          source_to_show_id = source_to_show[:id]
-        end
+    def render_for_default_application(request, wrapper)
+      template = create_template(request, wrapper)
+      file = "rescues/#{wrapper.rescue_template}"
 
-        template = DebugView.new([RESCUES_TEMPLATE_PATH],
-          request: request,
-          exception: wrapper.exception,
-          traces: traces,
-          show_source_idx: source_to_show_id,
-          trace_to_show: trace_to_show,
-          routes_inspector: routes_inspector(exception),
-          source_extracts: wrapper.source_extracts,
-          line_number: wrapper.line_number,
-          file: wrapper.file
-        )
-        file = "rescues/#{wrapper.rescue_template}"
-
-        if request.xhr?
-          body = template.render(template: file, layout: false, formats: [:text])
-          format = "text/plain"
-        else
-          body = template.render(template: file, layout: 'rescues/layout')
-          format = "text/html"
-        end
-        render(wrapper.status_code, body, format)
+      if request.xhr?
+        body = template.render(template: file, layout: false, formats: [:text])
+        format = "text/plain"
       else
-        raise exception
+        body = template.render(template: file, layout: 'rescues/layout')
+        format = "text/html"
       end
+      render(wrapper.status_code, body, format)
+    end
+
+    def render_for_api_application(request, wrapper)
+      body = {
+        status: wrapper.status_code,
+        error:  Rack::Utils::HTTP_STATUS_CODES.fetch(
+          wrapper.status_code,
+          Rack::Utils::HTTP_STATUS_CODES[500]
+        ),
+        exception: wrapper.exception.inspect,
+        traces: wrapper.traces
+      }
+
+      content_type = request.formats.first
+      to_format = "to_#{content_type.to_sym}"
+
+      if content_type && body.respond_to?(to_format)
+        formatted_body = body.public_send(to_format)
+        format = content_type
+      else
+        formatted_body = body.to_json
+        format = Mime[:json]
+      end
+
+      render(wrapper.status_code, formatted_body, format)
+    end
+
+    def create_template(request, wrapper)
+      traces = wrapper.traces
+
+      trace_to_show = 'Application Trace'
+      if traces[trace_to_show].empty? && wrapper.rescue_template != 'routing_error'
+        trace_to_show = 'Full Trace'
+      end
+
+      if source_to_show = traces[trace_to_show].first
+        source_to_show_id = source_to_show[:id]
+      end
+
+      DebugView.new([RESCUES_TEMPLATE_PATH],
+        request: request,
+        exception: wrapper.exception,
+        traces: traces,
+        show_source_idx: source_to_show_id,
+        trace_to_show: trace_to_show,
+        routes_inspector: routes_inspector(wrapper.exception),
+        source_extracts: wrapper.source_extracts,
+        line_number: wrapper.line_number,
+        file: wrapper.file
+      )
     end
 
     def render(status, body, format)

actionpack/test/controller/mime/respond_to_test.rb

@@ -661,10 +661,6 @@ def test_no_variant_in_variant_setup
   end
 
   def test_variant_inline_syntax
-    get :variant_inline_syntax, format: :js
-    assert_equal "text/javascript", @response.content_type
-    assert_equal "js", @response.body
-
     get :variant_inline_syntax
     assert_equal "text/html", @response.content_type
     assert_equal "none", @response.body
@@ -674,6 +670,12 @@ def test_variant_inline_syntax
     assert_equal "phone", @response.body
   end
 
+  def test_variant_inline_syntax_with_format
+    get :variant_inline_syntax, format: :js
+    assert_equal "text/javascript", @response.content_type
+    assert_equal "js", @response.body
+  end
+
   def test_variant_inline_syntax_without_block
     get :variant_inline_syntax_without_block, params: { v: :phone }
     assert_equal "text/html", @response.content_type

actionpack/test/dispatch/debug_exceptions_test.rb

@@ -75,6 +75,13 @@ def call(env)
     end
   end
 
+  class BoomerAPI < Boomer
+    def call(env)
+      env['action_dispatch.show_detailed_exceptions'] = @detailed
+      raise "puke!"
+    end
+  end
+
   RoutesApp = Struct.new(:routes).new(SharedTestRoutes)
   ProductionApp  = ActionDispatch::DebugExceptions.new(Boomer.new(false), RoutesApp)
   DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp)
@@ -205,6 +212,68 @@ def call(env)
     assert_match(/ActionController::ParameterMissing/, body)
   end
 
+  test "rescue with json error for API request" do
+    @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api)
+
+    get "/", 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)
+
+    get "/not_found", headers: { 'action_dispatch.show_exceptions' => true }
+    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 }
+    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 }
+    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 }
+    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 }
+    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
+    @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)
+
+    get "/index.xml", headers: { 'action_dispatch.show_exceptions' => true }
+    assert_response 500
+    assert_equal "application/xml", response.content_type
+    assert_match(/RuntimeError: puke/, body)
+  end
+
   test "does not show filtered parameters" do
     @app = DevelopmentApp
 

actionpack/test/dispatch/show_exceptions_test.rb

@@ -8,7 +8,7 @@ def call(env)
       case req.path
       when "/not_found"
         raise AbstractController::ActionNotFound
-      when "/bad_params"
+      when "/bad_params", "/bad_params.json"
         begin
           raise StandardError.new
         rescue
@@ -120,4 +120,18 @@ def call(env)
     assert_response 405
     assert_equal "", body
   end
+
+  test "bad params exception is returned in the correct format" do
+    @app = ProductionApp
+
+    get "/bad_params", headers: { 'action_dispatch.show_exceptions' => true }
+    assert_equal "text/html; charset=utf-8", response.headers["Content-Type"]
+    assert_response 400
+    assert_match(/400 error/, body)
+
+    get "/bad_params.json", headers: { 'action_dispatch.show_exceptions' => true }
+    assert_equal "application/json; charset=utf-8", response.headers["Content-Type"]
+    assert_response 400
+    assert_equal("{\"status\":400,\"error\":\"Bad Request\"}", body)
+  end
 end

guides/source/api_app.md

@@ -163,6 +163,14 @@ class definition:
 config.api_only = true
 ```
 
+Optionally, in `config/environments/development.rb` add the following line
+to render error responses using the API format (JSON by default) when it
+is a local request:
+
+```ruby
+config.debug_exception_response_format = :api
+```
+
 Finally, inside `app/controllers/application_controller.rb`, instead of:
 
 ```ruby

railties/CHANGELOG.md

+*   `config.debug_exception_response_format` configures the format used
+    in responses when errors occur in development mode.
+
+    Set `config.debug_exception_response_format` to render an HTML page with
+    debug info (using the value `:default`) or render debug info preserving
+    the response format (using the value `:api`).
+
+    *Jorge Bejar*
+
 *   Fix setting exit status code for rake test tasks. The exit status code
     was not set when tests were fired with `rake`. Now, it is being set and it matches
     behavior of running tests via `rails` command (`rails test`), so no matter if

railties/lib/rails/application/configuration.rb

@@ -24,35 +24,36 @@ class Configuration < ::Rails::Engine::Configuration
       def initialize(*)
         super
         self.encoding = "utf-8"
-        @allow_concurrency             = nil
-        @consider_all_requests_local   = false
-        @filter_parameters             = []
-        @filter_redirect               = []
-        @helpers_paths                 = []
-        @public_file_server            = ActiveSupport::OrderedOptions.new
-        @public_file_server.enabled    = true
-        @public_file_server.index_name = "index"
-        @force_ssl                     = false
-        @ssl_options                   = {}
-        @session_store                 = :cookie_store
-        @session_options               = {}
-        @time_zone                     = "UTC"
-        @beginning_of_week             = :monday
-        @log_level                     = nil
-        @generators                    = app_generators
-        @cache_store                   = [ :file_store, "#{root}/tmp/cache/" ]
-        @railties_order                = [:all]
-        @relative_url_root             = ENV["RAILS_RELATIVE_URL_ROOT"]
-        @reload_classes_only_on_change = true
-        @file_watcher                  = file_update_checker
-        @exceptions_app                = nil
-        @autoflush_log                 = true
-        @log_formatter                 = ActiveSupport::Logger::SimpleFormatter.new
-        @eager_load                    = nil
-        @secret_token                  = nil
-        @secret_key_base               = nil
-        @api_only                      = false
-        @x                             = Custom.new
+        @allow_concurrency               = nil
+        @consider_all_requests_local     = false
+        @filter_parameters               = []
+        @filter_redirect                 = []
+        @helpers_paths                   = []
+        @public_file_server              = ActiveSupport::OrderedOptions.new
+        @public_file_server.enabled      = true
+        @public_file_server.index_name   = "index"
+        @force_ssl                       = false
+        @ssl_options                     = {}
+        @session_store                   = :cookie_store
+        @session_options                 = {}
+        @time_zone                       = "UTC"
+        @beginning_of_week               = :monday
+        @log_level                       = nil
+        @generators                      = app_generators
+        @cache_store                     = [ :file_store, "#{root}/tmp/cache/" ]
+        @railties_order                  = [:all]
+        @relative_url_root               = ENV["RAILS_RELATIVE_URL_ROOT"]
+        @reload_classes_only_on_change   = true
+        @file_watcher                    = file_update_checker
+        @exceptions_app                  = nil
+        @autoflush_log                   = true
+        @log_formatter                   = ActiveSupport::Logger::SimpleFormatter.new
+        @eager_load                      = nil
+        @secret_token                    = nil
+        @secret_key_base                 = nil
+        @api_only                        = false
+        @debug_exception_response_format = nil
+        @x                               = Custom.new
       end
 
       def static_cache_control=(value)
@@ -95,6 +96,16 @@ def encoding=(value)
       def api_only=(value)
         @api_only = value
         generators.api_only = value
+
+        @debug_exception_response_format ||= :api
+      end
+
+      def debug_exception_response_format
+        @debug_exception_response_format || :default
+      end
+
+      def debug_exception_response_format=(value)
+        @debug_exception_response_format = value
       end
 
       def paths

railties/lib/rails/application/default_middleware_stack.rb

@@ -57,7 +57,7 @@ def build_stack
           # Must come after Rack::MethodOverride to properly log overridden methods
           middleware.use ::Rails::Rack::Logger, config.log_tags
           middleware.use ::ActionDispatch::ShowExceptions, show_exceptions_app
-          middleware.use ::ActionDispatch::DebugExceptions, app
+          middleware.use ::ActionDispatch::DebugExceptions, app, config.debug_exception_response_format
           middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies
 
           unless config.cache_classes

railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt

@@ -23,7 +23,6 @@ Rails.application.configure do
     config.action_controller.perform_caching = false
     config.cache_store = :null_store
   end
-
   <%- unless options.skip_action_mailer? -%>
 
   # Don't care if the mailer can't send.

railties/test/application/configuration_test.rb

@@ -1393,5 +1393,45 @@ def index
 
       assert_equal 'unicorn', Rails.application.config.my_custom_config['key']
     end
+
+    test "api_only is false by default" do
+      app 'development'
+      refute Rails.application.config.api_only
+    end
+
+    test "api_only generator config is set when api_only is set" do
+      add_to_config <<-RUBY
+        config.api_only = true
+      RUBY
+      app 'development'
+
+      Rails.application.load_generators
+      assert Rails.configuration.api_only
+    end
+
+    test "debug_exception_response_format is :api by default if only_api is enabled" do
+      add_to_config <<-RUBY
+        config.api_only = true
+      RUBY
+      app 'development'
+
+      assert_equal :api, Rails.configuration.debug_exception_response_format
+    end
+
+    test "debug_exception_response_format can be override" do
+      add_to_config <<-RUBY
+        config.api_only = true
+      RUBY
+
+      app_file 'config/environments/development.rb', <<-RUBY
+      Rails.application.configure do
+        config.debug_exception_response_format = :default
+      end
+      RUBY
+
+      app 'development'
+
+      assert_equal :default, Rails.configuration.debug_exception_response_format
+    end
   end
 end