Return proper format on exceptions

3 files changed, 81 insertions, 14 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 815a46a3ca..8c2d887cb3 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Return proper format on exceptions. *Santiago Pastorino*
+
 *   Allow to use mounted_helpers (helpers for accessing mounted engines) in ActionView::TestCase. *Piotr Sarnacki*
 
 *   Include mounted_helpers (helpers for accessing mounted engines) in ActionDispatch::IntegrationTest by default. *Piotr Sarnacki*
diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb
index 85b8d178bf..bf9149ebf2 100644
--- a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb
@@ -3,28 +3,50 @@ module ActionDispatch
   class PublicExceptions
     attr_accessor :public_path
 
-    def initialize(public_path)
+    def initialize(public_path, consider_all_requests_local = false)
       @public_path = public_path
+      @consider_all_requests_local = consider_all_requests_local
     end
 
     def call(env)
-      status      = env["PATH_INFO"][1..-1]
-      locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale
-      path        = "#{public_path}/#{status}.html"
-
-      if locale_path && File.exist?(locale_path)
-        render(status, File.read(locale_path))
-      elsif File.exist?(path)
-        render(status, File.read(path))
+      exception    = env["action_dispatch.exception"]
+      status       = env["PATH_INFO"][1..-1]
+      request      = ActionDispatch::Request.new(env)
+      content_type = request.formats.first
+      format       = (mime = Mime[content_type]) && "to_#{mime.to_sym}"
+      body         = { :status => status, :error => exception.message }
+
+      render(status, body, :format => format, :content_type => content_type)
+    end
+
+    private
+
+    def render(status, body, options)
+      format = options[:format]
+
+      if !@consider_all_requests_local && format && body.respond_to?(format)
+        render_format(status, body.public_send(format), options)
       else
-        [404, { "X-Cascade" => "pass" }, []]
+        render_html(status)
       end
     end
 
-    private
+    def render_format(status, body, options)
+      [status, {'Content-Type' => "#{options[:content_type]}; charset=#{ActionDispatch::Response.default_charset}",
+                'Content-Length' => body.bytesize.to_s}, [body]]
+    end
+
+    def render_html(status)
+      found = false
+      path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale
+      path = "#{public_path}/#{status}.html" unless path && (found = File.exist?(path))
 
-    def render(status, body)
-      [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]]
+      if found || File.exist?(path)
+        body = File.read(path)
+        [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]]
+      else
+        [404, { "X-Cascade" => "pass" }, []]
+      end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb
index 13ab19ed8f..ce7b6b0dc6 100644
--- a/actionpack/test/controller/show_exceptions_test.rb
+++ b/actionpack/test/controller/show_exceptions_test.rb
@@ -22,6 +22,14 @@ module ShowExceptions
     end
   end
 
+  class ShowLocalExceptionsController < ActionController::Base
+    use ActionDispatch::ShowExceptions, ActionDispatch::PublicExceptions.new("#{FIXTURE_LOAD_PATH}/public", true)
+
+    def boom
+      raise 'boom!'
+    end
+  end
+
   class ShowExceptionsTest < ActionDispatch::IntegrationTest
     test 'show error page from a remote ip' do
       @app = ShowExceptionsController.action(:boom)
@@ -68,4 +76,39 @@ module ShowExceptions
       assert_match(/boom/, body)
     end
   end
+
+  class ShowExceptionsFormatsTest < ActionDispatch::IntegrationTest
+    def test_render_json_exception
+      @app = ShowExceptionsOverridenController.action(:boom)
+      get "/", {}, 'HTTP_ACCEPT' => 'application/json'
+      assert_response :internal_server_error
+      assert_equal 'application/json', response.content_type.to_s
+      assert_equal({ :status => '500', :error => 'boom!' }.to_json, response.body)
+    end
+
+    def test_render_xml_exception
+      @app = ShowExceptionsOverridenController.action(:boom)
+      get "/", {}, 'HTTP_ACCEPT' => 'application/xml'
+      assert_response :internal_server_error
+      assert_equal 'application/xml', response.content_type.to_s
+      assert_equal({ :status => '500', :error => 'boom!' }.to_xml, response.body)
+    end
+
+    def test_render_fallback_exception
+      @app = ShowExceptionsOverridenController.action(:boom)
+      get "/", {}, 'HTTP_ACCEPT' => 'text/csv'
+      assert_response :internal_server_error
+      assert_equal 'text/html', response.content_type.to_s
+    end
+  end
+
+  class ShowExceptionsFormatsTest < ActionDispatch::IntegrationTest
+    def test_render_formatted_exception_in_development
+      @app = ShowLocalExceptionsController.action(:boom)
+      get "/", {}, 'HTTP_ACCEPT' => 'application/xml'
+
+      assert_response :internal_server_error
+      assert_equal 'text/html', response.content_type.to_s
+    end
+  end
 end