From dbcbbcf2bc58e8971672b143d1c52c0244e33f26 Mon Sep 17 00:00:00 2001 From: Byron Bischoff Date: Thu, 23 Oct 2014 09:53:43 -0700 Subject: =?UTF-8?q?Show=20the=20user=E2=80=99s=20application=20in=20the=20?= =?UTF-8?q?source=20window=20and=20select=20the=20correct=20trace=20list,?= =?UTF-8?q?=20closes=20#17312?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action_dispatch/middleware/debug_exceptions.rb | 56 +++++++++++++--------- .../middleware/templates/rescues/_source.erb | 2 +- .../middleware/templates/rescues/_trace.html.erb | 2 +- actionpack/test/dispatch/debug_exceptions_test.rb | 42 +++++++++++++++- 4 files changed, 76 insertions(+), 26 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 274f6f2f22..ff72592b94 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -35,10 +35,23 @@ module ActionDispatch if env['action_dispatch.show_detailed_exceptions'] request = Request.new(env) + traces = traces_from_wrapper(wrapper) + + trace_to_show = 'Application Trace' + if traces[trace_to_show].empty? + trace_to_show = 'Full Trace' + end + + if source_to_show = traces[trace_to_show].first + source_to_show_id = source_to_show[:id] + end + template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], request: request, exception: wrapper.exception, - traces: traces_from_wrapper(wrapper), + traces: traces, + show_source_idx: source_to_show_id, + trace_to_show: trace_to_show, routes_inspector: routes_inspector(exception), source_extract: wrapper.source_extract, line_number: wrapper.line_number, @@ -97,31 +110,28 @@ module ActionDispatch # 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 } + framework_trace = wrapper.framework_trace + full_trace = wrapper.full_trace + + appplication_trace_with_ids = [] + framework_trace_with_ids = [] + full_trace_with_ids = [] + + if full_trace + full_trace.each_with_index do |trace, idx| + id_trace = { + id: idx, + trace: trace + } + appplication_trace_with_ids << id_trace if application_trace.include? trace + framework_trace_with_ids << id_trace if framework_trace.include? trace + full_trace_with_ids << id_trace end - - full_trace = application_trace + framework_trace end - { - "Application Trace" => application_trace, - "Framework Trace" => framework_trace, - "Full Trace" => full_trace + "Application Trace" => appplication_trace_with_ids, + "Framework Trace" => framework_trace_with_ids, + "Full Trace" => full_trace_with_ids } 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 ba29e26930..eabac3a9d2 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb @@ -1,7 +1,7 @@ <% if @source_extract %> <% @source_extract.each_with_index do |extract_source, index| %> <% if extract_source[:code] %> -
" id="frame-source-<%=index%>"> +
" id="frame-source-<%=index%>">
Extracted source (around line #<%= extract_source[:line_number] %>):
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 f62caf51d7..ab57b11c7d 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb @@ -12,7 +12,7 @@ <% end %> <% @traces.each do |name, trace| %> -
;"> + 
<% trace.each do |frame| %><%= frame[:trace] %>
<% end %>
<% end %> diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 0add9fa3b0..f8851f0152 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -19,6 +19,10 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest @closed = true end + def method_that_raises + raise StandardError.new 'error in framework' + end + def call(env) env['action_dispatch.show_detailed_exceptions'] = @detailed req = ActionDispatch::Request.new(env) @@ -57,7 +61,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest {}) raise ActionView::Template::Error.new(template, e) end - + when "/framework_raises" + method_that_raises else raise "puke!" end @@ -280,4 +285,39 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_select 'pre code', /\(eval\):1: syntax error, unexpected/ end end + + test 'debug exceptions app shows user code that caused the error in source view' do + @app = DevelopmentApp + Rails.stubs(:root).returns(Pathname.new('.')) + cleaner = ActiveSupport::BacktraceCleaner.new.tap do |bc| + bc.add_silencer { |line| line =~ /method_that_raises/ } + bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} } + end + + get '/framework_raises', {}, {'action_dispatch.backtrace_cleaner' => cleaner} + + # Assert correct error + assert_response 500 + assert_select 'h2', /error in framework/ + + # assert source view line is the call to method_that_raises + assert_select 'div.source:not(.hidden)' do + assert_select 'pre .line.active', /method_that_raises/ + end + + # assert first source view (hidden) that throws the error + assert_select 'div.source:first' do + assert_select 'pre .line.active', /raise StandardError\.new/ + 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} + end + + # assert framework trace that that threw the error is first + assert_select '#Framework-Trace' do + assert_select 'pre code a:first', /method_that_raises/ + end + end end -- cgit v1.2.3