Merge pull request #14090 from Fortisque/kevin/stream_error_in_main_thread_if_not_committed

re-raise error if error occurs before committing in streaming

2 files changed, 30 insertions, 10 deletions

diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb
index 41b997a755..acf40b2e16 100644
--- a/actionpack/lib/action_controller/metal/live.rb
+++ b/actionpack/lib/action_controller/metal/live.rb
@@ -228,18 +228,19 @@ module ActionController
         begin
           super(name)
         rescue => e
-          unless @_response.committed?
+          if @_response.committed?
+            begin
+              @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html
+              @_response.stream.call_on_error
+            rescue => exception
+              log_error(exception)
+            ensure
+              log_error(e)
+              @_response.stream.close
+            end
+          else
             error = e
           end
-          begin
-            @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html
-            @_response.stream.call_on_error
-          rescue => exception
-            log_error(exception)
-          ensure
-            log_error(e)
-            @_response.stream.close
-          end
         ensure
           @_response.commit!
         end
diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb
index 2fd5c930ba..947f64176b 100644
--- a/actionpack/test/controller/live_stream_test.rb
+++ b/actionpack/test/controller/live_stream_test.rb
@@ -153,6 +153,11 @@ module ActionController
         render 'doesntexist'
       end
 
+      def exception_in_view_after_commit
+        response.stream.write ""
+        render 'doesntexist'
+      end
+
       def exception_with_callback
         response.headers['Content-Type'] = 'text/event-stream'
 
@@ -269,6 +274,13 @@ module ActionController
       assert_raises(ActionView::MissingTemplate) do
         get :exception_in_view
       end
+
+      capture_log_output do |output|
+        get :exception_in_view_after_commit
+        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 response.body
       assert_stream_closed
     end
@@ -277,6 +289,13 @@ module ActionController
       assert_raises(ActionView::MissingTemplate) do
         get :exception_in_view, format: :json
       end
+
+      capture_log_output do |output|
+        get :exception_in_view_after_commit, 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_when_committed