use built-in exception handling in live controllers

when an exception happens in an action before the response has been
committed, then we should re-raise the exception in the main thread.
This lets us reuse the existing exception handling.

2 files changed, 22 insertions, 17 deletions

diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb
index fba17746c0..d60f1b0d44 100644
--- a/actionpack/lib/action_controller/metal/live.rb
+++ b/actionpack/lib/action_controller/metal/live.rb
@@ -203,6 +203,7 @@ module ActionController
       t1 = Thread.current
       locals = t1.keys.map { |key| [key, t1[key]] }
 
+      error = nil
       # This processes the action in a child thread. It lets us return the
       # response code and headers back up the rack stack, and still process
       # the body in parallel with sending data to the client
@@ -217,8 +218,9 @@ module ActionController
         begin
           super(name)
         rescue => e
-          @_response.status = 500 unless @_response.committed?
-          @_response.status = 400 if e.class == ActionController::BadRequest
+          unless @_response.committed?
+            error = e
+          end
           begin
             @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html
             @_response.stream.call_on_error
@@ -234,6 +236,7 @@ module ActionController
       }
 
       @_response.await_commit
+      raise error if error
     end
 
     def log_error(exception)
diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb
index 5fb066cc51..d1025042df 100644
--- a/actionpack/test/controller/live_stream_test.rb
+++ b/actionpack/test/controller/live_stream_test.rb
@@ -90,6 +90,9 @@ module ActionController
   end
 
   class LiveStreamTest < ActionController::TestCase
+    class Exception < StandardError
+    end
+
     class TestController < ActionController::Base
       include ActionController::Live
 
@@ -152,11 +155,12 @@ module ActionController
           response.stream.close
         end
 
+        response.stream.write "" # make sure the response is committed
         raise 'An exception occurred...'
       end
 
       def exception_in_controller
-        raise 'Exception in controller'
+        raise Exception, 'Exception in controller'
       end
 
       def bad_request_error
@@ -168,6 +172,7 @@ module ActionController
         response.stream.on_error do
           raise 'We need to go deeper.'
         end
+        response.stream.write ''
         response.stream.write params[:widget][:didnt_check_for_nil]
       end
     end
@@ -246,24 +251,19 @@ module ActionController
     end
 
     def test_exception_handling_html
-      capture_log_output do |output|
+      assert_raises(ActionView::MissingTemplate) do
         get :exception_in_view
-        assert_match %r((window\.location = "/500\.html"</script></html>)$), response.body
-        assert_match 'Missing template test/doesntexist', output.rewind && output.read
-        assert_stream_closed
       end
+      assert_stream_closed
     end
 
     def test_exception_handling_plain_text
-      capture_log_output do |output|
+      assert_raises(ActionView::MissingTemplate) do
         get :exception_in_view, format: :json
-        assert_equal '', response.body
-        assert_match 'Missing template test/doesntexist', output.rewind && output.read
-        assert_stream_closed
       end
     end
 
-    def test_exception_callback
+    def test_exception_callback_when_committed
       capture_log_output do |output|
         get :exception_with_callback, format: 'text/event-stream'
         assert_equal %(data: "500 Internal Server Error"\n\n), response.body
@@ -273,16 +273,18 @@ module ActionController
     end
 
     def test_exception_in_controller_before_streaming
-      response = get :exception_in_controller, format: 'text/event-stream'
-      assert_equal 500, response.status
+      assert_raises(ActionController::LiveStreamTest::Exception) do
+        get :exception_in_controller, format: 'text/event-stream'
+      end
     end
 
     def test_bad_request_in_controller_before_streaming
-      response = get :bad_request_error, format: 'text/event-stream'
-      assert_equal 400, response.status
+      assert_raises(ActionController::BadRequest) do
+        get :bad_request_error, format: 'text/event-stream'
+      end
     end
 
-    def test_exceptions_raised_handling_exceptions
+    def test_exceptions_raised_handling_exceptions_and_committed
       capture_log_output do |output|
         get :exception_in_exception_callback, format: 'text/event-stream'
         assert_equal '', response.body