streaming should change status of response when exception is caught

[kacasey8]

This fixes #12552. #9604 looks like sgrif expected that if streaming and an exception occurs we should just log it and in the view use javascript to display a 500 error while bkudria is saying that we should change the status response (not a 200 for correctly rendering an error message).

[kacasey8]

@tenderlove @matthewd

[tenderlove]

@Fortisque this fix is going on the wrong direction, I think.

This is where the controller is called and the template is processed (of course you found that). But if either controller processing or template processing raises an exception, we just need to set the response status.

In the simplest case, it should look like this:

diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb
index 33014b9..6fb3e44 100644
--- a/actionpack/lib/action_controller/metal/live.rb
+++ b/actionpack/lib/action_controller/metal/live.rb
@@ -205,6 +205,7 @@ module ActionController
         begin
           super(name)
         rescue => e
+          @_response.status = 500 unless @_response.committed?
           begin
             @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html
             @_response.stream.call_on_error

The above patch will fix the reporter's test case. But you should also rescue an ActionController::BadRequest and convert it to a 400 response. Try integrating the reporter's test case in to the actionpack tests and add this fix. Then add a new test case that raises a BadRequest exception and fix that too.

Does it make sense?

[kacasey8]

Yeah that makes sense, I've modified my commit.

[tenderlove]

One more change: change your commit message and add "fixes #12552". If you do that, when I push your changes to the Rails repository, it will automatically close the ticket.

[kacasey8]

I think resolve does the same thing in github at least, I'll change it though

[robin850]

Also could you add a changelog entry as well please ? Nice work though, thank you!