From 53adf53bc52d83187d229e0153c98db193adce30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Janko=20Marohni=C4=87?= <janko.marohnic@gmail.com>
Date: Sat, 23 Jan 2021 23:48:25 +0100
Subject: [PATCH] Handle throwing in controller action in log subscriber

When throw was used in a controller action, and there is matching catch
around the request in a Rack middleware, then :exception won't be
present in the event payload.

This is because ActiveSupport::Notifications::Instrumenter.instrument
sets :exception in a rescue handler, but rescue is never called in a
throw/catch scenario:

  catch(:halt) do
    begin
      throw :halt
    rescue Exception => e
      puts "rescue" # never reached
    ensure
      puts "ensure"
    end
  end

Missing :exception was actually handled prior to Rails 6.1.0, but an
optimization updated the code to assume this was present. So this can be
considered a regression fix.
---
 actionpack/CHANGELOG.md                            |  4 ++++
 actionpack/lib/action_controller/log_subscriber.rb |  2 +-
 actionpack/test/controller/log_subscriber_test.rb  | 12 ++++++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index c9f1487b5008e..218cadc42cf0b 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,9 @@
 ## Unreleased
 
+*   Fix error in `ActionController::LogSubscriber` that would happen when throwing inside a controller action.
+
+    *Janko Marohnić*
+
 *   Change the request method to a `GET` when passing failed requests down to `config.exceptions_app`.
 
     *Alex Robbin*
diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb
index be74163230bf1..2c36d7c759b98 100644
--- a/actionpack/lib/action_controller/log_subscriber.rb
+++ b/actionpack/lib/action_controller/log_subscriber.rb
@@ -23,7 +23,7 @@ def process_action(event)
         additions = ActionController::Base.log_process_action(payload)
         status = payload[:status]
 
-        if status.nil? && (exception_class_name = payload[:exception].first)
+        if status.nil? && (exception_class_name = payload[:exception]&.first)
           status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name)
         end
 
diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb
index 8762b8ff377ee..362afc9f166f2 100644
--- a/actionpack/test/controller/log_subscriber_test.rb
+++ b/actionpack/test/controller/log_subscriber_test.rb
@@ -64,6 +64,10 @@ def with_fragment_cache_unless_with_true_condition
       render inline: "<%= cache_unless(true, 'foo') { 'bar' } %>"
     end
 
+    def with_throw
+      throw :halt
+    end
+
     def with_exception
       raise Exception
     end
@@ -190,6 +194,14 @@ def test_process_action_with_view_runtime
     assert_match(/Completed 200 OK in \d+ms/, logs[1])
   end
 
+  def test_process_action_with_throw
+    catch(:halt) do
+      get :with_throw
+      wait
+    end
+    assert_match(/Completed   in \d+ms/, logs[1])
+  end
+
   def test_append_info_to_payload_is_called_even_with_exception
     begin
       get :with_exception