From fe12e4650802d8f28136660fc9ce62c6a28f19e1 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sun, 29 Apr 2012 18:26:37 -0500 Subject: [PATCH 1/8] Add source extract to detailed exception page --- .../middleware/debug_exceptions.rb | 5 ++++- .../middleware/exception_wrapper.rb | 22 ++++++++++++++++++- .../middleware/templates/rescues/_source.erb | 8 +++++++ .../templates/rescues/diagnostics.erb | 1 + 4 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 6705e531cb2b1..ac8f6af7fe263 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -42,7 +42,10 @@ def render_exception(env, exception) :application_trace => wrapper.application_trace, :framework_trace => wrapper.framework_trace, :full_trace => wrapper.full_trace, - :routes => formatted_routes(exception) + :routes => formatted_routes(exception), + :source_extract => wrapper.source_extract, + :line_number => wrapper.line_number, + :file => wrapper.file ) file = "rescues/#{wrapper.rescue_template}" diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index ae38c56a670d0..d1cadbd082c6d 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -25,7 +25,7 @@ class ExceptionWrapper 'ActionView::Template::Error' => 'template_error' ) - attr_reader :env, :exception + attr_reader :env, :exception, :line_number, :file def initialize(env, exception) @env = env @@ -56,6 +56,15 @@ def self.status_code_for_exception(class_name) Rack::Utils.status_code(@@rescue_responses[class_name]) end + def source_extract + if trace = application_trace.first + file, line, _ = trace.split(":") + @file = file + @line_number = line.to_i + source_fragment(@file, @line_number) + end + end + private def original_exception(exception) @@ -81,5 +90,16 @@ def clean_backtrace(*args) def backtrace_cleaner @backtrace_cleaner ||= @env['action_dispatch.backtrace_cleaner'] end + + def source_fragment(path, line) + full_path = Rails.root.join(path) + if File.exists?(full_path) + File.open(full_path, "r") do |file| + start = [line - 3, 0].max + lines = file.lines.drop(start).take(6) + Hash[*(start+1..(lines.count+start)).zip(lines).flatten] + end + end + 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 new file mode 100644 index 0000000000000..032705c01a737 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb @@ -0,0 +1,8 @@ +<% if @source_extract %> +Extracted source (around line #<%= @line_number %>): +
+<% @source_extract.each do |line, source| %>
+<%= "#{(@line_number == line) ? "> " : "  "}#{line}: #{source}" -%>
+<% end %>
+
+<% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb index c5043c5e7b743..5ad96cc6575e1 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -6,5 +6,6 @@ 
<%=h @exception.message %>
+<%= render template: "rescues/_source" %> <%= render template: "rescues/_trace" %> <%= render template: "rescues/_request_and_response" %> From 048cd254e6bb77cf223a714cd7cb86b309cd3c15 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Fri, 7 Sep 2012 20:20:09 -0500 Subject: [PATCH 2/8] Styling for exception page --- .../rescues/_request_and_response.erb | 15 +-- .../middleware/templates/rescues/_source.erb | 25 ++++- .../templates/rescues/diagnostics.erb | 25 +++-- .../middleware/templates/rescues/layout.erb | 97 ++++++++++++++++++- .../templates/rescues/missing_template.erb | 9 +- .../templates/rescues/unknown_action.erb | 8 +- 6 files changed, 150 insertions(+), 29 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb index 823f5d25b64f5..9688bf6a572a3 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -20,12 +20,15 @@

Request

Parameters: 

<%=h request_dump %>

-

Show session dump

- - -

Show env dump

- - +
+Show session dump +

<%= debug_hash @request.session %>

+
+ +
+Show env dump +

<%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %>

+

Response

Headers: 

<%=h defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %>

diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb index 032705c01a737..38429cb78e4d8 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb @@ -1,8 +1,25 @@ <% if @source_extract %> -Extracted source (around line #<%= @line_number %>): +
+
+ Extracted source (around line #<%= @line_number %>): +
+
+ + + + + +
+ 
+            <% @source_extract.keys.each do |line_number| %>
+<%= line_number -%>
+            <% end %>
+
+ 
-<% @source_extract.each do |line, source| %>
-<%= "#{(@line_number == line) ? "> " : "  "}#{line}: #{source}" -%>
-<% end %>
+<% @source_extract.each do |line, source| -%>
"><%= source -%>
<% end -%>
+
+
+
<% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb index 5ad96cc6575e1..1c6b5010a3fa2 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -1,11 +1,16 @@ -

- <%=h @exception.class.to_s %> - <% if @request.parameters['controller'] %> - in <%=h @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> - <% end %> -

-
<%=h @exception.message %>
+
+

+ <%=h @exception.class.to_s %> + <% if @request.parameters['controller'] %> + in <%=h @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> + <% end %> +

+
-<%= render template: "rescues/_source" %> -<%= render template: "rescues/_trace" %> -<%= render template: "rescues/_request_and_response" %> +
+

<%=h @exception.message %>

+ + <%= render template: "rescues/_source" %> + <%= render template: "rescues/_trace" %> + <%= render template: "rescues/_request_and_response" %> +
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 1a308707d1652..0aba130f05309 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -4,7 +4,7 @@ Action Controller: Exception caught diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb index dbfdf76947b32..c5917b9acb9a2 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb @@ -1,2 +1,7 @@ -

Template is missing

-

<%=h @exception.message %>

+
+

Template is missing

+
+ +
+

<%=h @exception.message %>

+
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb index 683379da10838..65fc34df90ac6 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb @@ -1,2 +1,6 @@ -

Unknown action

-

<%=h @exception.message %>

+
+

Unknown action

+
+
+

<%=h @exception.message %>

+
From 78a0d86b0a4e9ce791f684d424f8aca96ff6ff92 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Fri, 7 Sep 2012 23:22:17 -0500 Subject: [PATCH 3/8] Add new style to Routing Error page --- .../templates/rescues/routing_error.erb | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb index 6c903d6a1743d..ca85e6d048f76 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb @@ -1,24 +1,27 @@ -

Routing Error

-

<%=h @exception.message %>

-<% unless @exception.failures.empty? %> -

-

Failure reasons:

-
    - <% @exception.failures.each do |route, reason| %> -
  1. <%=h route.inspect.gsub('\\', '') %> failed because <%=h reason.downcase %>
    2. - <% end %> -
-

-<% end %> -<%= render template: "rescues/_trace" %> +
+

Routing Error

+
+
+

<%=h @exception.message %>

+ <% unless @exception.failures.empty? %> +

+

Failure reasons:

+
    + <% @exception.failures.each do |route, reason| %> +
  1. <%=h route.inspect.gsub('\\', '') %> failed because <%=h reason.downcase %>
    2. + <% end %> +
+

+ <% end %> + <%= render template: "rescues/_trace" %> -

- Routes -

+

+ Routes +

-

- Routes match in priority from top to bottom -

+

+ Routes match in priority from top to bottom +

<%= render layout: "routes/route_wrapper" do %> <%= render partial: "routes/route", collection: @routes %> From 9b79bb469b02c9c51440960cfc33bd928c9fbd1d Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sat, 8 Sep 2012 22:28:46 -0500 Subject: [PATCH 4/8] Improve line-height to have better line spacing in exception message --- .../lib/action_dispatch/middleware/templates/rescues/layout.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 0aba130f05309..12c4d98e1932b 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -36,6 +36,7 @@ h2 { color: #C52F24; padding: 2px; + line-height: 25px; } details { From 5a1b885dd620e6ab465c0f64b7cd0f025a46fb37 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Mon, 31 Dec 2012 14:36:23 -0500 Subject: [PATCH 5/8] Add style to AV::Template::Error exception page --- .../middleware/exception_wrapper.rb | 3 +- .../templates/rescues/template_error.erb | 53 ++++++++++++++----- actionpack/lib/action_view/template/error.rb | 16 ++++-- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index d1cadbd082c6d..ee69c8b49dd71 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -57,7 +57,7 @@ def self.status_code_for_exception(class_name) end def source_extract - if trace = application_trace.first + if application_trace && trace = application_trace.first file, line, _ = trace.split(":") @file = file @line_number = line.to_i @@ -92,6 +92,7 @@ def backtrace_cleaner end def source_fragment(path, line) + return unless Rails.respond_to?(:root) && Rails.root full_path = Rails.root.join(path) if File.exists?(full_path) File.open(full_path, "r") do |file| diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb index a1b377f68cd5d..01faf5a47524e 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb @@ -1,17 +1,44 @@ -

- <%=h @exception.original_exception.class.to_s %> in - <%=h @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%=h @request.parameters["action"] %> -

+<% @source_extract = @exception.source_extract(0, :html) %> +
+

+ <%=h @exception.original_exception.class.to_s %> in + <%=h @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%=h @request.parameters["action"] %> +

+
-

- Showing <%=h @exception.file_name %> where line #<%=h @exception.line_number %> raised: - 

<%=h @exception.message %>
-

+
+

+ Showing <%=h @exception.file_name %> where line #<%=h @exception.line_number %> raised: + 

<%=h @exception.message %>
+

-

Extracted source (around line #<%=h @exception.line_number %>): -

<%=h @exception.source_extract %>

+
+
+

Extracted source (around line #<%=h @exception.line_number %>): +

+
+ + + + + +
+ 
+            <% @source_extract.keys.each do |line_number| %>
+<%= line_number -%>
+            <% end %>
+
+ 		+
+<% @source_extract.each do |line, source| -%>
"><%= source -%>
<% end -%>
+
+
+
+
-

<%=h @exception.sub_template_message %>

+

<%=h @exception.sub_template_message %>

-<%= render template: "rescues/_trace" %> -<%= render template: "rescues/_request_and_response" %> + <%= render template: "rescues/_trace" %> + <%= render template: "rescues/_request_and_response" %> +
+
diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index e00056781dc32..662e1ac66bf03 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -78,7 +78,7 @@ def sub_template_message end end - def source_extract(indentation = 0) + def source_extract(indentation = 0, output = :console) return unless num = line_number num = num.to_i @@ -88,12 +88,20 @@ def source_extract(indentation = 0) end_on_line = [ num + SOURCE_CODE_RADIUS - 1, source_code.length].min indent = end_on_line.to_s.size + indentation - line_counter = start_on_line return unless source_code = source_code[start_on_line..end_on_line] - source_code.sum do |line| + formatted_code_for(source_code, start_on_line, indent, output) + end + + def formatted_code_for(source_code, line_counter, indent, output) + start_value = (output == :html) ? {} : "" + source_code.inject(start_value) do |result, line| line_counter += 1 - "%#{indent}s: %s\n" % [line_counter, line] + if output == :html + result.update(line_counter.to_s => "%#{indent}s %s\n" % ["", line]) + else + result << "%#{indent}s: %s\n" % [line_counter, line] + end end end From a27c4f6095e8f15adc4b1e06faf6d63828e94e6a Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Mon, 31 Dec 2012 16:48:10 -0500 Subject: [PATCH 6/8] Fix test for DebugExceptions due to template change --- actionpack/test/dispatch/debug_exceptions_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index d236b14e02ab9..39e791b4f495a 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -135,7 +135,7 @@ def call(env) } }) assert_response 500 - assert_match(/RuntimeError\n in FeaturedTileController/, body) + assert_match(/RuntimeError\n\s+in FeaturedTileController/, body) end test "sets the HTTP charset parameter" do From 8ecaf133f3adbda93ecae0af0ffbd29f5efbe7f0 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Mon, 31 Dec 2012 17:49:05 -0500 Subject: [PATCH 7/8] Summary and Details HTML elements aren't supported in all modern browsers --- .../templates/rescues/_request_and_response.erb | 16 ++++++++-------- .../middleware/templates/rescues/layout.erb | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb index 9688bf6a572a3..55079848bd5f5 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -20,15 +20,15 @@

Request

Parameters: 

<%=h request_dump %>

-
-Show session dump -

<%= debug_hash @request.session %>

-
+
+
Show session dump
+ +
-
-Show env dump -

<%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %>

-
+
+
Show env dump
+ +

Response

Headers: 

<%=h defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %>

diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 12c4d98e1932b..bcab5959a007f 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -39,7 +39,7 @@ line-height: 25px; } - details { + .details { border: 1px solid #E5E5E5; -webkit-border-radius: 4px; -moz-border-radius: 4px; @@ -49,14 +49,14 @@ width: 978px; } - summary { + .summary { padding: 8px 15px; border-bottom: 1px solid #E5E5E5; display: block; } - details pre { - margin: 0px; + .details pre { + margin: 5px; border: none; } From 25c8770a6cbbc4922446085addaa5a41d0e4e1b6 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Mon, 31 Dec 2012 18:07:21 -0500 Subject: [PATCH 8/8] formatted_code_for should be private --- actionpack/lib/action_view/template/error.rb | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index 662e1ac66bf03..b479f991bc5bf 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -93,18 +93,6 @@ def source_extract(indentation = 0, output = :console) formatted_code_for(source_code, start_on_line, indent, output) end - def formatted_code_for(source_code, line_counter, indent, output) - start_value = (output == :html) ? {} : "" - source_code.inject(start_value) do |result, line| - line_counter += 1 - if output == :html - result.update(line_counter.to_s => "%#{indent}s %s\n" % ["", line]) - else - result << "%#{indent}s: %s\n" % [line_counter, line] - end - end - end - def sub_template_of(template_path) @sub_templates ||= [] @sub_templates << template_path @@ -131,6 +119,18 @@ def source_location 'in ' end + file_name end + + def formatted_code_for(source_code, line_counter, indent, output) + start_value = (output == :html) ? {} : "" + source_code.inject(start_value) do |result, line| + line_counter += 1 + if output == :html + result.update(line_counter.to_s => "%#{indent}s %s\n" % ["", line]) + else + result << "%#{indent}s: %s\n" % [line_counter, line] + end + end + end end end