diff options
author | Genadi Samokovarov <gsamokovarov@gmail.com> | 2014-11-16 17:17:06 +0200 |
---|---|---|
committer | Genadi Samokovarov <gsamokovarov@gmail.com> | 2014-11-16 17:17:06 +0200 |
commit | e05714fdbc4d6a767f207b08a94ba3ebf147213e (patch) | |
tree | 9d8558c201bd4c8e27d4c4cee067c51c988ba3f8 /actionpack | |
parent | 4b4dca48a590fe9380cc0b727788a36a8c40296f (diff) | |
download | rails-e05714fdbc4d6a767f207b08a94ba3ebf147213e.tar.gz rails-e05714fdbc4d6a767f207b08a94ba3ebf147213e.tar.bz2 rails-e05714fdbc4d6a767f207b08a94ba3ebf147213e.zip |
Don't let #{application,framework,full}_trace be nil
Those three can be nil when exception backtrace is nil. This happens and
that forced a couple of nil guards in the code. I'm proposing to make
those always return an array, even on nil backtrace.
Diffstat (limited to 'actionpack')
3 files changed, 62 insertions, 31 deletions
diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index e0140b0692..b8381aba70 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -62,14 +62,12 @@ module ActionDispatch framework_trace_with_ids = [] full_trace_with_ids = [] - if full_trace - full_trace.each_with_index do |trace, idx| - trace_with_id = { id: idx, trace: trace } + full_trace.each_with_index do |trace, idx| + trace_with_id = { id: idx, trace: trace } - appplication_trace_with_ids << trace_with_id if application_trace.include?(trace) - framework_trace_with_ids << trace_with_id if framework_trace.include?(trace) - full_trace_with_ids << trace_with_id - end + appplication_trace_with_ids << trace_with_id if application_trace.include?(trace) + framework_trace_with_ids << trace_with_id if framework_trace.include?(trace) + full_trace_with_ids << trace_with_id end { @@ -84,7 +82,7 @@ module ActionDispatch end def source_extract - exception.backtrace.map do |trace| + backtrace.map do |trace| file, line = trace.split(":") line_number = line.to_i { @@ -92,11 +90,15 @@ module ActionDispatch file: file, line_number: line_number } - end if exception.backtrace + end end private + def backtrace + Array(@exception.backtrace) + end + def original_exception(exception) if registered_original_exception?(exception) exception.original_exception @@ -111,9 +113,9 @@ module ActionDispatch def clean_backtrace(*args) if backtrace_cleaner - backtrace_cleaner.clean(@exception.backtrace, *args) + backtrace_cleaner.clean(backtrace, *args) else - @exception.backtrace + backtrace 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 eabac3a9d2..101cea13f9 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb @@ -1,29 +1,27 @@ -<% if @source_extract %> - <% @source_extract.each_with_index do |extract_source, index| %> - <% if extract_source[:code] %> - <div class="source <%="hidden" if @show_source_idx != index%>" 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].each_key do |line_number| %> +<% @source_extract.each_with_index do |extract_source, index| %> + <% if extract_source[:code] %> + <div class="source <%="hidden" if @show_source_idx != index%>" 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].each_key do |line_number| %> <span><%= line_number -%></span> - <% end %> - </pre> - </td> + <% end %> + </pre> + </td> <td width="100%"> <pre> <% 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> + </tr> + </table> </div> - <% end %> + </div> <% end %> <% end %> diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index 3ddaa7294b..57292d3191 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -10,6 +10,12 @@ module ActionDispatch end end + class BadlyDefinedError < StandardError + def backtrace + nil + end + end + setup do Rails.stubs(:root).returns(Pathname.new('.')) @@ -28,6 +34,7 @@ module ActionDispatch assert_equal [ code: 'foo', file: 'lib/file.rb', line_number: 42 ], wrapper.source_extract end + test '#application_trace returns traces only from the application' do exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'")) wrapper = ExceptionWrapper.new(@environment, exception) @@ -35,6 +42,14 @@ module ActionDispatch assert_equal [ "lib/file.rb:42:in `index'" ], wrapper.application_trace end + test '#application_trace cannot be nil' do + nil_backtrace_wrapper = ExceptionWrapper.new(@environment, BadlyDefinedError.new) + nil_cleaner_wrapper = ExceptionWrapper.new({}, BadlyDefinedError.new) + + assert_equal [], nil_backtrace_wrapper.application_trace + assert_equal [], nil_cleaner_wrapper.application_trace + end + test '#framework_trace returns traces outside the application' do exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'")) wrapper = ExceptionWrapper.new(@environment, exception) @@ -42,6 +57,14 @@ module ActionDispatch assert_equal caller, wrapper.framework_trace end + test '#framework_trace cannot be nil' do + nil_backtrace_wrapper = ExceptionWrapper.new(@environment, BadlyDefinedError.new) + nil_cleaner_wrapper = ExceptionWrapper.new({}, BadlyDefinedError.new) + + assert_equal [], nil_backtrace_wrapper.framework_trace + assert_equal [], nil_cleaner_wrapper.framework_trace + end + test '#full_trace returns application and framework traces' do exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'")) wrapper = ExceptionWrapper.new(@environment, exception) @@ -49,6 +72,14 @@ module ActionDispatch assert_equal exception.backtrace, wrapper.full_trace end + test '#full_trace cannot be nil' do + nil_backtrace_wrapper = ExceptionWrapper.new(@environment, BadlyDefinedError.new) + nil_cleaner_wrapper = ExceptionWrapper.new({}, BadlyDefinedError.new) + + assert_equal [], nil_backtrace_wrapper.full_trace + assert_equal [], nil_cleaner_wrapper.full_trace + end + test '#traces returns every trace by category enumerated with an index' do exception = TestError.new("lib/file.rb:42:in `index'", "/gems/rack.rb:43:in `index'") wrapper = ExceptionWrapper.new(@environment, exception) |