Decouple variable_info from ErrorPage instance

Author: RobinDaugherty

lib/better_errors/error_page.rb

@@ -5,6 +5,8 @@
 module BetterErrors
   # @private
   class ErrorPage
+    VariableInfo = Struct.new(:frame, :editor_url, :rails_params, :rack_session, :start_time)
+
     def self.template_path(template_name)
       File.expand_path("../templates/#{template_name}.erb", __FILE__)
     end
@@ -13,6 +15,15 @@ def self.template(template_name)
       Erubi::Engine.new(File.read(template_path(template_name)), escape: true)
     end
 
+    def self.render_template(template_name, locals)
+      locals.send(:eval, self.template(template_name).src)
+    rescue => e
+      # Fix the backtrace, which doesn't identify the template that failed (within Better Errors).
+      # We don't know the line number, so just injecting the template path has to be enough.
+      e.backtrace.unshift "#{self.template_path(template_name)}:0"
+      raise
+    end
+
     attr_reader :exception, :env, :repls
 
     def initialize(exception, env)
@@ -26,20 +37,21 @@ def id
       @id ||= SecureRandom.hex(8)
     end
 
-    def render(template_name = "main", csrf_token = nil, csp_nonce = nil)
-      binding.eval(self.class.template(template_name).src)
-    rescue => e
-      # Fix the backtrace, which doesn't identify the template that failed (within Better Errors).
-      # We don't know the line number, so just injecting the template path has to be enough.
-      e.backtrace.unshift "#{self.class.template_path(template_name)}:0"
-      raise
+    def render_main(csrf_token, csp_nonce)
+      frame = backtrace_frames[0]
+      first_frame_variable_info = VariableInfo.new(frame, editor_url(frame), rails_params, rack_session, Time.now.to_f)
+      self.class.render_template('main', binding)
+    end
+
+    def render_text
+      self.class.render_template('text', binding)
     end
 
     def do_variables(opts)
       index = opts["index"].to_i
-      @frame = backtrace_frames[index]
-      @var_start_time = Time.now.to_f
-      { html: render("variable_info") }
+      frame = backtrace_frames[index]
+      variable_info = VariableInfo.new(frame, editor_url(frame), rails_params, rack_session, Time.now.to_f)
+      { html: self.class.render_template("variable_info", variable_info) }
     end
 
     def do_eval(opts)
@@ -113,19 +125,19 @@ def request_path
       env["PATH_INFO"]
     end
 
-    def html_formatted_code_block(frame)
+    def self.html_formatted_code_block(frame)
       CodeFormatter::HTML.new(frame.filename, frame.line).output
     end
 
-    def text_formatted_code_block(frame)
+    def self.text_formatted_code_block(frame)
       CodeFormatter::Text.new(frame.filename, frame.line).output
     end
 
     def text_heading(char, str)
       str + "\n" + char*str.size
     end
 
-    def inspect_value(obj)
+    def self.inspect_value(obj)
       if BetterErrors.ignored_classes.include? obj.class.name
         "<span class='unsupported'>(Instance of ignored class. "\
         "#{obj.class.name ? "Remove #{CGI.escapeHTML(obj.class.name)} from" : "Modify"}"\

lib/better_errors/middleware.rb

@@ -98,9 +98,9 @@ def show_error_page(env, exception=nil)
 
       type, content = if @error_page
         if text?(env)
-          [ 'plain', @error_page.render('text') ]
+          [ 'plain', @error_page.render_text ]
         else
-          [ 'html', @error_page.render('main', csrf_token, csp_nonce) ]
+          [ 'html', @error_page.render_main(csrf_token, csp_nonce) ]
         end
       else
         [ 'html', no_errors_page ]

lib/better_errors/templates/text.erb

@@ -9,7 +9,7 @@
 <%== text_heading("-", "%s, line %i" % [first_frame.pretty_path, first_frame.line]) %>
 
 ``` ruby
-<%== text_formatted_code_block(first_frame) %>```
+<%== ErrorPage.text_formatted_code_block(first_frame) %>```
 
 App backtrace
 -------------

lib/better_errors/templates/variable_info.erb

@@ -1,20 +1,20 @@
 <header class="trace_info clearfix">
     <div class="title">
-        <h2 class="name"><%= @frame.name %></h2>
+        <h2 class="name"><%= frame.name %></h2>
         <div class="location">
           <span class="filename">
             <a
-              href="<%= editor_url(@frame) %>"
+              href="<%= editor_url %>"
               <%= ENV.key?('BETTER_ERRORS_INSIDE_FRAME') ? "target=_blank" : '' %>
-            ><%= @frame.pretty_path %></a>
+            ><%= frame.pretty_path %></a>
           </span>
         </div>
     </div>
     <div class="code_block clearfix">
-        <%== html_formatted_code_block @frame %>
+        <%== ErrorPage.html_formatted_code_block frame %>
     </div>
 
-    <% if BetterErrors.binding_of_caller_available? && @frame.frame_binding %>
+    <% if BetterErrors.binding_of_caller_available? && frame.frame_binding %>
         <div class="be-repl">
             <div class="be-console">
                 <pre></pre>
@@ -24,7 +24,7 @@
     <% end %>
 </header>
 
-<% if BetterErrors.binding_of_caller_available? && @frame.frame_binding %>
+<% if BetterErrors.binding_of_caller_available? && frame.frame_binding %>
     <div class="hint live-console-hint">
         This is a live shell. Type in here.
     </div>
@@ -38,27 +38,28 @@
     </div>
 <% end %>
 
+<%# TODO: move this outside of the frame info. It's not part of the frame. %>
 <div class="sub">
     <h3>Request info</h3>
     <div class='inset variables'>
         <table class="var_table">
             <% if rails_params %>
-                <tr><td class="name">Request parameters</td><td><pre><%== inspect_value rails_params %></pre></td></tr>
+                <tr><td class="name">Request parameters</td><td><pre><%== ErrorPage.inspect_value rails_params %></pre></td></tr>
             <% end %>
             <% if rack_session %>
-                <tr><td class="name">Rack session</td><td><pre><%== inspect_value rack_session %></pre></td></tr>
+                <tr><td class="name">Rack session</td><td><pre><%== ErrorPage.inspect_value rack_session %></pre></td></tr>
             <% end %>
         </table>
     </div>
 </div>
 
-<% if BetterErrors.binding_of_caller_available? && @frame.frame_binding %>
+<% if BetterErrors.binding_of_caller_available? && frame.frame_binding %>
     <div class="sub">
         <h3>Local Variables</h3>
         <div class='inset variables'>
             <table class="var_table">
-                <% @frame.local_variables.each do |name, value| %>
-                    <tr><td class="name"><%= name %></td><td><pre><%== inspect_value value %></pre></td></tr>
+                <% frame.local_variables.each do |name, value| %>
+                    <tr><td class="name"><%= name %></td><td><pre><%== ErrorPage.inspect_value value %></pre></td></tr>
                 <% end %>
             </table>
         </div>
@@ -68,12 +69,14 @@
         <h3>Instance Variables</h3>
         <div class="inset variables">
             <table class="var_table">
-                <% @frame.instance_variables.each do |name, value| %>
-                    <tr><td class="name"><%= name %></td><td><pre><%== inspect_value value %></pre></td></tr>
+                <% frame.instance_variables.each do |name, value| %>
+                    <tr><td class="name"><%= name %></td><td><pre><%== ErrorPage.inspect_value value %></pre></td></tr>
                 <% end %>
             </table>
         </div>
     </div>
+<% end %>
 
-    <!-- <%= Time.now.to_f - @var_start_time %> seconds -->
+<% if start_time %>
+    <!-- variable_info took <%= Time.now.to_f - start_time %> seconds -->
 <% end %>

spec/better_errors/error_page_spec.rb

@@ -13,7 +13,7 @@ module BetterErrors
 
     let(:error_page) { ErrorPage.new exception, { "PATH_INFO" => "/some/path" } }
 
-    let(:response) { error_page.render }
+    let(:response) { error_page.render_main("CSRF_TOKEN", "CSP_NONCE") }
 
     let(:exception_binding) {
       local_a = :value_for_local_a