Merge pull request #16427 from ryandao/full_stack_source_extract

Full stack source extract

9 files changed, 114 insertions, 69 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index c30217b8fe..44b8fa843d 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Extract source code for the entire exception stack trace for
+    better debugging and diagnosis.
+
+    *Ryan Dao*
+
 *   Allows ActionDispatch::Request::LOCALHOST to match any IPv4 127.0.0.0/8
     loopback address.
 
diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
index 0ca1a87645..274f6f2f22 100644
--- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -38,9 +38,7 @@ module ActionDispatch
         template = ActionView::Base.new([RESCUES_TEMPLATE_PATH],
           request: request,
           exception: wrapper.exception,
-          application_trace: wrapper.application_trace,
-          framework_trace: wrapper.framework_trace,
-          full_trace: wrapper.full_trace,
+          traces: traces_from_wrapper(wrapper),
           routes_inspector: routes_inspector(exception),
           source_extract: wrapper.source_extract,
           line_number: wrapper.line_number,
@@ -95,5 +93,36 @@ module ActionDispatch
         ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes)
       end
     end
+
+    # Augment the exception traces by providing ids for all unique stack frame
+    def traces_from_wrapper(wrapper)
+      application_trace = wrapper.application_trace
+      framework_trace = wrapper.framework_trace
+      full_trace = wrapper.full_trace
+
+      if application_trace && framework_trace
+        id_counter = 0
+
+        application_trace = application_trace.map do |trace|
+          prev = id_counter
+          id_counter += 1
+          { id: prev, trace: trace }
+        end
+
+        framework_trace = framework_trace.map do |trace|
+          prev = id_counter
+          id_counter += 1
+          { id: prev, trace: trace }
+        end
+
+        full_trace = application_trace + framework_trace
+      end
+
+      {
+        "Application Trace" => application_trace,
+        "Framework Trace" => framework_trace,
+        "Full Trace" => full_trace
+      }
+    end
   end
 end
diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
index 2326bb043a..b98b553c38 100644
--- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
+++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
@@ -61,12 +61,15 @@ module ActionDispatch
     end
 
     def source_extract
-      if application_trace && trace = application_trace.first
-        file, line, _ = trace.split(":")
-        @file = file
-        @line_number = line.to_i
-        source_fragment(@file, @line_number)
-      end
+      exception.backtrace.map do |trace|
+        file, line = trace.split(":")
+        line_number = line.to_i
+        {
+          code: source_fragment(file, line_number),
+          file: file,
+          line_number: line_number
+        }
+      end if exception.backtrace
     end
 
     private
@@ -110,7 +113,7 @@ module ActionDispatch
     def expand_backtrace
       @exception.backtrace.unshift(
         @exception.to_s.split("\n")
-      ).flatten! 
+      ).flatten!
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb
index 38429cb78e..51660a619b 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb
@@ -1,25 +1,29 @@
 <% if @source_extract %>
-<div class="source">
-<div class="info">
-  Extracted source (around line <strong>#<%= @line_number %></strong>):
-</div>
-<div class="data">
-  <table cellpadding="0" cellspacing="0" class="lines">
-      <tr>
-        <td>
-          <pre class="line_numbers">
-            <% @source_extract.keys.each do |line_number| %>
+  <% @source_extract.each_with_index do |extract_source, index| %>
+    <% if extract_source[:code] %>
+      <div class="source <%="hidden" if index != 0%>" id="frame-source-<%=index%>">
+        <div class="info">
+          Extracted source (around line <strong>#<%= extract_source[:line_number] %></strong>):
+        </div>
+        <div class="data">
+          <table cellpadding="0" cellspacing="0" class="lines">
+            <tr>
+              <td>
+                <pre class="line_numbers">
+                  <% extract_source[:code].keys.each do |line_number| %>
 <span><%= line_number -%></span>
-            <% end %>
-          </pre>
-        </td>
+                  <% end %>
+                </pre>
+              </td>
 <td width="100%">
 <pre>
-<% @source_extract.each do |line, source| -%><div class="line<%= " active" if line == @line_number -%>"><%= source -%></div><% end -%>
+<% extract_source[:code].each do |line, source| -%><div class="line<%= " active" if line == extract_source[:line_number] -%>"><%= source -%></div><% end -%>
 </pre>
 </td>
-    </tr>
-  </table>
-</div>
-</div>
+            </tr>
+          </table>
+        </div>
+      </div>
+    <% end %>
+  <% end %>
 <% end %>
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 b181909bff..f62caf51d7 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb
@@ -1,9 +1,4 @@
-<%
-   traces = { "Application Trace" => @application_trace,
-              "Framework Trace"   => @framework_trace,
-              "Full Trace"        => @full_trace }
-   names = traces.keys
-%>
+<% names = @traces.keys %>
 
 <p><code>Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %></code></p>
 
@@ -16,9 +11,42 @@
     <a href="#" onclick="<%= hide.join %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %>
   <% end %>
 
-  <% traces.each do |name, trace| %>
+  <% @traces.each do |name, trace| %>
     <div id="<%= name.gsub(/\s/, '-') %>" style="display: <%= (name == "Application Trace") ? 'block' : 'none' %>;">
-      <pre><code><%= trace.join "\n" %></code></pre>
+      <pre><code><% trace.each do |frame| %><a class="trace-frames" data-frame-id="<%= frame[:id] %>" href="#"><%= frame[:trace] %></a><br><% end %></code></pre>
     </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;
+        }
+      }
+    }
+  </script>
 </div>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb
index d4af5c9b06..36b01bf952 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb
@@ -1,15 +1,9 @@
-<%
-  traces = { "Application Trace" => @application_trace,
-             "Framework Trace"   => @framework_trace,
-             "Full Trace"        => @full_trace }
-%>
-
 Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %>
 
-<% traces.each do |name, trace| %>
+<% @traces.each do |name, trace| %>
 <% if trace.any? %>
 <%= name %>
-<%= trace.join("\n") %>
+<%= trace.map(&:trace).join("\n") %>
 
 <% end %>
 <% end %>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb
index bc5d03dc10..e0509f56f4 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb
@@ -116,9 +116,15 @@
       background-color: #FFCCCC;
     }
 
+    .hidden {
+      display: none;
+    }
+
     a { color: #980905; }
     a:visited { color: #666; }
+    a.trace-frames { color: #666; }
     a:hover { color: #C52F24; }
+    a.trace-frames.selected { color: #C52F24 }
 
     <%= yield :style %>
   </style>
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 027a0f5b3e..c1e8b6cae3 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
@@ -1,4 +1,3 @@
-<% @source_extract = @exception.source_extract(0, :html) %>
 <header>
   <h1>
     <%= @exception.original_exception.class.to_s %> in
@@ -12,29 +11,7 @@
   </p>
   <pre><code><%= h @exception.message %></code></pre>
 
-  <div class="source">
-    <div class="info">
-      <p>Extracted source (around line <strong>#<%= @exception.line_number %></strong>):</p>
-    </div>
-    <div class="data">
-    <table cellpadding="0" cellspacing="0" class="lines">
-      <tr>
-        <td>
-          <pre class="line_numbers">
-            <% @source_extract.keys.each do |line_number| %>
-<span><%= line_number -%></span>
-            <% end %>
-          </pre>
-        </td>
-<td width="100%">
-<pre>
-<% @source_extract.each do |line, source| -%><div class="line<%= " active" if line == @exception.line_number -%>"><%= source -%></div><% end -%>
-</pre>
-</td>
-    </tr>
-  </table>
-</div>
-</div>
+  <%= render template: "rescues/_source" %>
 
   <p><%= @exception.sub_template_message %></p>
 
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb
index 5da21d9784..77bcd26726 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb
@@ -1,4 +1,3 @@
-<% @source_extract = @exception.source_extract(0, :html) %>
 <%= @exception.original_exception.class.to_s %> in <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %>
 
 Showing <%= @exception.file_name %> where line #<%= @exception.line_number %> raised: