Merge pull request #32410 from yuki24/show-cause-on-debug-view-2

Show nested exceptions on the debug view

11 files changed, 189 insertions, 77 deletions

diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
index 33edad8bd9..077a83b112 100644
--- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -152,23 +152,13 @@ module ActionDispatch
       end
 
       def create_template(request, wrapper)
-        traces = wrapper.traces
-
-        trace_to_show = "Application Trace"
-        if traces[trace_to_show].empty? && wrapper.rescue_template != "routing_error"
-          trace_to_show = "Full Trace"
-        end
-
-        if source_to_show = traces[trace_to_show].first
-          source_to_show_id = source_to_show[:id]
-        end
-
         DebugView.new([RESCUES_TEMPLATE_PATH],
           request: request,
+          exception_wrapper: wrapper,
           exception: wrapper.exception,
-          traces: traces,
-          show_source_idx: source_to_show_id,
-          trace_to_show: trace_to_show,
+          traces: wrapper.traces,
+          show_source_idx: wrapper.source_to_show_id,
+          trace_to_show: wrapper.trace_to_show,
           routes_inspector: routes_inspector(wrapper.exception),
           source_extracts: wrapper.source_extracts,
           line_number: wrapper.line_number,
diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
index f05c69137b..fb2b2bd3b0 100644
--- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
+++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
@@ -31,11 +31,12 @@ module ActionDispatch
       "ActionController::MissingExactTemplate" => "missing_exact_template",
     )
 
-    attr_reader :backtrace_cleaner, :exception, :line_number, :file
+    attr_reader :backtrace_cleaner, :exception, :wrapped_causes, :line_number, :file
 
     def initialize(backtrace_cleaner, exception)
       @backtrace_cleaner = backtrace_cleaner
       @exception = original_exception(exception)
+      @wrapped_causes = wrapped_causes_for(exception, backtrace_cleaner)
 
       expand_backtrace if exception.is_a?(SyntaxError) || exception.cause.is_a?(SyntaxError)
     end
@@ -66,7 +67,11 @@ module ActionDispatch
       full_trace_with_ids = []
 
       full_trace.each_with_index do |trace, idx|
-        trace_with_id = { id: idx, trace: trace }
+        trace_with_id = {
+          exception_object_id: @exception.object_id,
+          id: idx,
+          trace: trace
+        }
 
         if application_trace.include?(trace)
           application_trace_with_ids << trace_with_id
@@ -99,6 +104,18 @@ module ActionDispatch
       end
     end
 
+    def trace_to_show
+      if traces["Application Trace"].empty? && rescue_template != "routing_error"
+        "Full Trace"
+      else
+        "Application Trace"
+      end
+    end
+
+    def source_to_show_id
+      (traces[trace_to_show].first || {})[:id]
+    end
+
     private
 
       def backtrace
@@ -113,6 +130,16 @@ module ActionDispatch
         end
       end
 
+      def causes_for(exception)
+        return enum_for(__method__, exception) unless block_given?
+
+        yield exception while exception = exception.cause
+      end
+
+      def wrapped_causes_for(exception, backtrace_cleaner)
+        causes_for(exception).map { |cause| self.class.new(backtrace_cleaner, cause) }
+      end
+
       def clean_backtrace(*args)
         if backtrace_cleaner
           backtrace_cleaner.clean(backtrace, *args)
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb
index e7b913bbe4..88a8e6ad83 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb
@@ -1,6 +1,8 @@
-<% @source_extracts.each_with_index do |source_extract, index| %>
+<% error_index = local_assigns[:error_index] || 0 %>
+
+<% source_extracts.each_with_index do |source_extract, index| %>
   <% if source_extract[:code] %>
-    <div class="source <%="hidden" if @show_source_idx != index%>" id="frame-source-<%=index%>">
+    <div class="source <%= "hidden" if show_source_idx != index %>" id="frame-source-<%= error_index %>-<%= index %>">
       <div class="info">
         Extracted source (around line <strong>#<%= source_extract[:line_number] %></strong>):
       </div>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb
index ab57b11c7d..835ca8d260 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb
@@ -1,52 +1,62 @@
-<% names = @traces.keys %>
+<% names = traces.keys %>
+<% error_index = local_assigns[:error_index] || 0 %>
 
 <p><code>Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %></code></p>
 
-<div id="traces">
+<div id="traces-<%= error_index %>">
   <% names.each do |name| %>
     <%
-      show = "show('#{name.gsub(/\s/, '-')}');"
-      hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}');"}
+      show = "show('#{name.gsub(/\s/, '-')}-#{error_index}');"
+      hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}-#{error_index}');"}
     %>
     <a href="#" onclick="<%= hide.join %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %>
   <% end %>
 
-  <% @traces.each do |name, trace| %>
-    <div id="<%= name.gsub(/\s/, '-') %>" style="display: <%= (name == @trace_to_show) ? 'block' : 'none' %>;">
-      <pre><code><% trace.each do |frame| %><a class="trace-frames" data-frame-id="<%= frame[:id] %>" href="#"><%= frame[:trace] %></a><br><% end %></code></pre>
+  <% traces.each do |name, trace| %>
+    <div id="<%= "#{name.gsub(/\s/, '-')}-#{error_index}" %>" style="display: <%= (name == trace_to_show) ? 'block' : 'none' %>;">
+      <code style="font-size: 11px;">
+        <% trace.each do |frame| %>
+          <a class="trace-frames trace-frames-<%= error_index %>" data-exception-object-id="<%= frame[:exception_object_id] %>" data-frame-id="<%= frame[:id] %>" href="#">
+            <%= frame[:trace] %>
+          </a>
+          <br>
+        <% end %>
+      </code>
     </div>
   <% end %>
 
   <script type="text/javascript">
-    var traceFrames = document.getElementsByClassName('trace-frames');
-    var selectedFrame, currentSource = document.getElementById('frame-source-0');
-
-    // Add click listeners for all stack frames
-    for (var i = 0; i < traceFrames.length; i++) {
-      traceFrames[i].addEventListener('click', function(e) {
-        e.preventDefault();
-        var target = e.target;
-        var frame_id = target.dataset.frameId;
-
-        if (selectedFrame) {
-          selectedFrame.className = selectedFrame.className.replace("selected", "");
-        }
-
-        target.className += " selected";
-        selectedFrame = target;
-
-        // Change the extracted source code
-        changeSourceExtract(frame_id);
-      });
-
-      function changeSourceExtract(frame_id) {
-        var el = document.getElementById('frame-source-' + frame_id);
-        if (currentSource && el) {
-          currentSource.className += " hidden";
-          el.className = el.className.replace(" hidden", "");
-          currentSource = el;
+    (function() {
+      var traceFrames = document.getElementsByClassName('trace-frames-<%= error_index %>');
+      var selectedFrame, currentSource = document.getElementById('frame-source-<%= error_index %>-0');
+
+      // Add click listeners for all stack frames
+      for (var i = 0; i < traceFrames.length; i++) {
+        traceFrames[i].addEventListener('click', function(e) {
+          e.preventDefault();
+          var target = e.target;
+          var frame_id = target.dataset.frameId;
+
+          if (selectedFrame) {
+            selectedFrame.className = selectedFrame.className.replace("selected", "");
+          }
+
+          target.className += " selected";
+          selectedFrame = target;
+
+          // Change the extracted source code
+          changeSourceExtract(frame_id);
+        });
+
+        function changeSourceExtract(frame_id) {
+          var el = document.getElementById('frame-source-<%= error_index %>-' + frame_id);
+          if (currentSource && el) {
+            currentSource.className += " hidden";
+            el.className = el.className.replace(" hidden", "");
+            currentSource = el;
+          }
         }
       }
-    }
+    })();
   </script>
 </div>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb
index f154021ae6..bde26f46c2 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb
@@ -10,7 +10,25 @@
 <div id="container">
   <h2><%= h @exception.message %></h2>
 
-  <%= render template: "rescues/_source" %>
-  <%= render template: "rescues/_trace" %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_index: 0 %>
+  <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show, error_index: 0 %>
+
+  <% if @exception.cause %>
+    <h2>Exception Causes</h2>
+  <% end %>
+
+  <% @exception_wrapper.wrapped_causes.each.with_index(1) do |wrapper, index| %>
+    <div class="details">
+      <a class="summary" href="#" style="color: #F0F0F0; text-decoration: none; background: #C52F24; border-bottom: none;" onclick="return toggle(<%= wrapper.exception.object_id %>)">
+        <%= wrapper.exception.class.name %>: <%= h wrapper.exception.message %>
+      </a>
+    </div>
+
+    <div id="<%= wrapper.exception.object_id %>" style="display: none;">
+      <%= render "rescues/source", source_extracts: wrapper.source_extracts, show_source_idx: wrapper.source_to_show_id, error_index: index %>
+      <%= render "rescues/trace", traces: wrapper.traces, trace_to_show: wrapper.trace_to_show, error_index: index %>
+    </div>
+  <% end %>
+
   <%= render template: "rescues/_request_and_response" %>
 </div>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb
index e1b129ccc5..056d99b03f 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb
@@ -15,7 +15,7 @@
     <% end %>
   </h2>
 
-  <%= render template: "rescues/_source" %>
-  <%= render template: "rescues/_trace" %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
+  <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
 </div>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb
index 2a65fd06ad..22eb6e9b4e 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb
@@ -5,7 +5,7 @@
 <div id="container">
   <h2><%= h @exception.message %></h2>
 
-  <%= render template: "rescues/_source" %>
-  <%= render template: "rescues/_trace" %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
+  <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
 </div>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb
index 55dd5ddc7b..2b8f3f2a5e 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb
@@ -14,7 +14,7 @@
     </p>
   <% end %>
 
-  <%= render template: "rescues/_trace" %>
+  <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
 
   <% if @routes_inspector %>
     <h2>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb
index 5060da9369..324ef1567a 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb
@@ -11,10 +11,10 @@
   </p>
   <pre><code><%= h @exception.message %></code></pre>
 
-  <%= render template: "rescues/_source" %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
 
   <p><%= @exception.sub_template_message %></p>
 
-  <%= render template: "rescues/_trace" %>
+  <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
 </div>
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index 045567ff83..44b79c0e5d 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -26,6 +26,18 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
       raise StandardError.new "error in framework"
     end
 
+    def raise_nested_exceptions
+      begin
+        raise "First error"
+      rescue
+        begin
+          raise "Second error"
+        rescue
+          raise "Third error"
+        end
+      end
+    end
+
     def call(env)
       env["action_dispatch.show_detailed_exceptions"] = @detailed
       req = ActionDispatch::Request.new(env)
@@ -74,6 +86,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
         end
       when %r{/framework_raises}
         method_that_raises
+      when %r{/nested_exceptions}
+        raise_nested_exceptions
       else
         raise "puke!"
       end
@@ -440,8 +454,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
     get "/original_syntax_error", headers: { "action_dispatch.backtrace_cleaner" => ActiveSupport::BacktraceCleaner.new }
 
     assert_response 500
-    assert_select "#Application-Trace" do
-      assert_select "pre code", /syntax error, unexpected/
+    assert_select "#Application-Trace-0" do
+      assert_select "code", /syntax error, unexpected/
     end
   end
 
@@ -454,9 +468,9 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
 
     assert_select "#container h2", /^Missing template/
 
-    assert_select "#Application-Trace"
-    assert_select "#Framework-Trace"
-    assert_select "#Full-Trace"
+    assert_select "#Application-Trace-0"
+    assert_select "#Framework-Trace-0"
+    assert_select "#Full-Trace-0"
 
     assert_select "h2", /Request/
   end
@@ -467,8 +481,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
     get "/syntax_error_into_view", headers: { "action_dispatch.backtrace_cleaner" => ActiveSupport::BacktraceCleaner.new }
 
     assert_response 500
-    assert_select "#Application-Trace" do
-      assert_select "pre code", /syntax error, unexpected/
+    assert_select "#Application-Trace-0" do
+      assert_select "code", /syntax error, unexpected/
     end
   end
 
@@ -497,13 +511,13 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
       end
 
       # assert application trace refers to line that calls method_that_raises is first
-      assert_select "#Application-Trace" do
-        assert_select "pre code a:first", %r{test/dispatch/debug_exceptions_test\.rb:\d+:in `call}
+      assert_select "#Application-Trace-0" do
+        assert_select "code a:first", %r{test/dispatch/debug_exceptions_test\.rb:\d+:in `call}
       end
 
       # assert framework trace that threw the error is first
-      assert_select "#Framework-Trace" do
-        assert_select "pre code a:first", /method_that_raises/
+      assert_select "#Framework-Trace-0" do
+        assert_select "code a:first", /method_that_raises/
       end
     end
   end
@@ -523,4 +537,39 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
     assert_response 500
     assert_match(/puke/, body)
   end
+
+  test "debug exceptions app shows all the nested exceptions in source view" do
+    @app = DevelopmentApp
+    Rails.stub :root, Pathname.new(".") do
+      cleaner = ActiveSupport::BacktraceCleaner.new.tap do |bc|
+        bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} }
+      end
+
+      get "/nested_exceptions", headers: { "action_dispatch.backtrace_cleaner" => cleaner }
+
+      # Assert correct error
+      assert_response 500
+      assert_select "h2", /Third error/
+
+      # assert source view line shows the last error
+      assert_select "div.source:not(.hidden)" do
+        assert_select "pre .line.active", /raise "Third error"/
+      end
+
+      # assert application trace refers to line that raises the last exception
+      assert_select "#Application-Trace-0" do
+        assert_select "code a:first", %r{in `rescue in rescue in raise_nested_exceptions'}
+      end
+
+      # assert the second application trace refers to the line that raises the second exception
+      assert_select "#Application-Trace-1" do
+        assert_select "code a:first", %r{in `rescue in raise_nested_exceptions'}
+      end
+
+      # assert the third application trace refers to the line that raises the first exception
+      assert_select "#Application-Trace-2" do
+        assert_select "code a:first", %r{in `raise_nested_exceptions'}
+      end
+    end
+  end
 end
diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb
index f6e70382a8..600280d6b3 100644
--- a/actionpack/test/dispatch/exception_wrapper_test.rb
+++ b/actionpack/test/dispatch/exception_wrapper_test.rb
@@ -108,11 +108,27 @@ module ActionDispatch
       wrapper = ExceptionWrapper.new(@cleaner, exception)
 
       assert_equal({
-        "Application Trace" => [ id: 0, trace: "lib/file.rb:42:in `index'" ],
-        "Framework Trace" => [ id: 1, trace: "/gems/rack.rb:43:in `index'" ],
+        "Application Trace" => [
+          exception_object_id: exception.object_id,
+          id: 0,
+          trace: "lib/file.rb:42:in `index'"
+        ],
+        "Framework Trace" => [
+          exception_object_id: exception.object_id,
+          id: 1,
+          trace: "/gems/rack.rb:43:in `index'"
+        ],
         "Full Trace" => [
-          { id: 0, trace: "lib/file.rb:42:in `index'" },
-          { id: 1, trace: "/gems/rack.rb:43:in `index'" }
+          {
+            exception_object_id: exception.object_id,
+            id: 0,
+            trace: "lib/file.rb:42:in `index'"
+          },
+          {
+            exception_object_id: exception.object_id,
+            id: 1,
+            trace: "/gems/rack.rb:43:in `index'"
+          }
         ]
       }, wrapper.traces)
     end