Commit 26f14111f08cd56e6ef436e0b61353abd6f72446

Authored by Nathan Broadbent
1 parent 6035cb2c
Exists in master and in 1 other branch production

Improved backtrace formatting.

Make whole (path/file) into github link,
hide [PROJECT_ROOT],
and replace [GEM_ROOT]/gems/****-*.*.* with bold gem name.
Improved CSS colors.
app/assets/javascripts/errbit.js
@@ -71,6 +71,9 @@ $(function() { @@ -71,6 +71,9 @@ $(function() {
71 tab.closest('.tab-bar').find('a.active').removeClass('active'); 71 tab.closest('.tab-bar').find('a.active').removeClass('active');
72 tab.addClass('active'); 72 tab.addClass('active');
73 73
  74 + // If clicking into 'backtrace' tab, hide external backtrace
  75 + if (tab.attr('rel') == "backtrace") { hide_external_backtrace(); }
  76 +
74 $('.panel').hide(); 77 $('.panel').hide();
75 panel.show(); 78 panel.show();
76 } 79 }
@@ -94,11 +97,17 @@ $(function() { @@ -94,11 +97,17 @@ $(function() {
94 }); 97 });
95 } 98 }
96 99
97 - init();  
98 -  
99 - // Show external backtrace lines when clicking separator  
100 - $('td.backtrace_separator span').live('click', function(){  
101 - $('tr.hidden_external_backtrace').removeClass('hidden_external_backtrace'); 100 + function hide_external_backtrace() {
  101 + $('tr.toggle_external_backtrace').hide();
  102 + $('td.backtrace_separator').show();
  103 + }
  104 + function show_external_backtrace() {
  105 + $('tr.toggle_external_backtrace').show();
102 $('td.backtrace_separator').hide(); 106 $('td.backtrace_separator').hide();
103 - }); 107 + }
  108 + // Show external backtrace lines when clicking separator
  109 + $('td.backtrace_separator span').live('click', show_external_backtrace);
  110 + // Hide external backtrace on page load
  111 + hide_external_backtrace();
  112 + init();
104 }); 113 });
app/assets/stylesheets/errbit.css
@@ -741,27 +741,28 @@ table.backtrace td.line { @@ -741,27 +741,28 @@ table.backtrace td.line {
741 vertical-align: top; 741 vertical-align: top;
742 white-space: nowrap; 742 white-space: nowrap;
743 } 743 }
744 -  
745 table.backtrace td.line .file { 744 table.backtrace td.line .file {
746 - color: #ededed;  
747 - font-weight:bold;  
748 -}  
749 -table.backtrace td.line .file a {  
750 - color: #21A4FF; 745 + font-weight: bold;
751 } 746 }
752 -  
753 table.backtrace td.line .method { 747 table.backtrace td.line .method {
754 color: #aaa; 748 color: #aaa;
755 - font-weight:bold; 749 + font-weight: bold;
756 } 750 }
757 751
758 table.backtrace td.line.in-app { 752 table.backtrace td.line.in-app {
759 color: #2adb2e; 753 color: #2adb2e;
760 background-color: #2f2f2f; 754 background-color: #2f2f2f;
761 } 755 }
762 -table.backtrace td.line.in-app .file { color: #2AEB2E; } 756 +table.backtrace td.line.in-app .path,
  757 +table.backtrace td.line.in-app .number { color: #2ACB2E; }
  758 +table.backtrace td.line.in-app .file { color: #3AFB3E; }
763 table.backtrace td.line.in-app .method { color: #2ACB2E; } 759 table.backtrace td.line.in-app .method { color: #2ACB2E; }
764 760
  761 +table.backtrace td.line.in-app a .path,
  762 +table.backtrace td.line.in-app a .number,
  763 +table.backtrace td.line.in-app a:hover { color: #21B4FF; }
  764 +table.backtrace td.line.in-app a .file { color: #31C4FF; }
  765 +
765 /* External backtrace classes and separators */ 766 /* External backtrace classes and separators */
766 table.backtrace tr.hidden_external_backtrace { 767 table.backtrace tr.hidden_external_backtrace {
767 display: none; 768 display: none;
app/helpers/notices_helper.rb
1 # encoding: utf-8 1 # encoding: utf-8
2 module NoticesHelper 2 module NoticesHelper
3 - def notice_atom_summary notice 3 + def in_app_backtrace_line?(line)
  4 + !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))})
  5 + end
  6 +
  7 + def notice_atom_summary(notice)
4 render :partial => "notices/atom_entry.html.haml", :locals => {:notice => notice} 8 render :partial => "notices/atom_entry.html.haml", :locals => {:notice => notice}
5 end 9 end
6 10
7 - def link_to_source_file(app, line, text)  
8 - if Notice.in_app_backtrace_line?(line) 11 + def link_to_source_file(app, line, &block)
  12 + text = capture_haml(&block)
  13 + if in_app_backtrace_line?(line)
9 return link_to_github(app, line, text) if app.github_url? 14 return link_to_github(app, line, text) if app.github_url?
10 if app.issue_tracker && app.issue_tracker.respond_to?(:url_to_file) 15 if app.issue_tracker && app.issue_tracker.respond_to?(:url_to_file)
11 # Return link to file on tracker if issue tracker supports this 16 # Return link to file on tracker if issue tracker supports this
@@ -36,7 +41,7 @@ module NoticesHelper @@ -36,7 +41,7 @@ module NoticesHelper
36 def grouped_lines(lines) 41 def grouped_lines(lines)
37 line_groups = [] 42 line_groups = []
38 lines.each do |line| 43 lines.each do |line|
39 - in_app = Notice.in_app_backtrace_line?(line) 44 + in_app = in_app_backtrace_line?(line)
40 if line_groups.last && line_groups.last[0] == in_app 45 if line_groups.last && line_groups.last[0] == in_app
41 line_groups.last[1] << line 46 line_groups.last[1] << line
42 else 47 else
@@ -48,12 +53,16 @@ module NoticesHelper @@ -48,12 +53,16 @@ module NoticesHelper
48 53
49 def path_for_backtrace_line(line) 54 def path_for_backtrace_line(line)
50 path = File.dirname(line['file']) 55 path = File.dirname(line['file'])
51 - path == "." ? "" : path + '/' 56 + return '' if path == '.'
  57 + # Remove [PROJECT_ROOT]
  58 + path.gsub!('[PROJECT_ROOT]/', '')
  59 + # Make gem name bold if starts with [GEM_ROOT]/gems
  60 + path.gsub!(/\[GEM_ROOT\]\/gems\/([^\/]+)/, "<strong>\\1</strong>")
  61 + (path << '/').html_safe
52 end 62 end
53 63
54 def file_for_backtrace_line(line) 64 def file_for_backtrace_line(line)
55 file = File.basename(line['file']) 65 file = File.basename(line['file'])
56 - line['number'].present? ? "#{file}:#{line['number']}" : file  
57 end 66 end
58 end 67 end
59 68
app/models/notice.rb
@@ -62,10 +62,6 @@ class Notice @@ -62,10 +62,6 @@ class Notice
62 where 62 where
63 end 63 end
64 64
65 - def self.in_app_backtrace_line?(line)  
66 - !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))})  
67 - end  
68 -  
69 def request 65 def request
70 read_attribute(:request) || {} 66 read_attribute(:request) || {}
71 end 67 end
app/views/notices/_backtrace.html.haml
@@ -2,11 +2,11 @@ @@ -2,11 +2,11 @@
2 %table.backtrace 2 %table.backtrace
3 -# Group lines into internal / external so we can toggle the external backtrace 3 -# Group lines into internal / external so we can toggle the external backtrace
4 -# Includes a margin of x lines that are not toggled. 4 -# Includes a margin of x lines that are not toggled.
5 - - em = 3 # (external backtrace margin) 5 + - em = 4 # (external backtrace margin)
6 - grouped_lines(lines).each do |in_app, line_group| 6 - grouped_lines(lines).each do |in_app, line_group|
7 - if !in_app && line_group.size > (em * 3) 7 - if !in_app && line_group.size > (em * 3)
8 = render :partial => 'notices/backtrace_line', :collection => line_group[0, em], :as => :line 8 = render :partial => 'notices/backtrace_line', :collection => line_group[0, em], :as => :line
9 - = render :partial => 'notices/backtrace_line', :collection => line_group[em, line_group.size - (em * 2)], :as => :line, :locals => {:row_class => 'hidden_external_backtrace'} 9 + = render :partial => 'notices/backtrace_line', :collection => line_group[em, line_group.size - (em * 2)], :as => :line, :locals => {:row_class => 'toggle_external_backtrace'}
10 %tr 10 %tr
11 %td.line.backtrace_separator 11 %td.line.backtrace_separator
12 %span ... 12 %span ...
app/views/notices/_backtrace_line.html.haml
1 %tr{:class => defined?(row_class) ? row_class : nil} 1 %tr{:class => defined?(row_class) ? row_class : nil}
2 - %td.line{:class => (Notice.in_app_backtrace_line?(line) ? 'in-app' : nil)}  
3 - %span.path>= path_for_backtrace_line(line)  
4 - %span.file= link_to_source_file(@app, line, file_for_backtrace_line(line)).html_safe 2 + %td.line{:class => (in_app_backtrace_line?(line) ? 'in-app' : nil)}
  3 + = link_to_source_file(@app, line) do
  4 + %span.path>= path_for_backtrace_line(line)
  5 + %span.file>= file_for_backtrace_line(line)
  6 + - if line['number'].present?
  7 + %span.number>= ":#{line['number']}"
5 &rarr; 8 &rarr;
6 %span.method= line['method'] 9 %span.method= line['method']
spec/models/notice_spec.rb
@@ -24,36 +24,6 @@ describe Notice do @@ -24,36 +24,6 @@ describe Notice do
24 end 24 end
25 25
26 26
27 - context '.in_app_backtrace_line?' do  
28 - let(:backtrace) do [{  
29 - 'number' => rand(999),  
30 - 'file' => '[GEM_ROOT]/gems/actionpack-3.0.4/lib/action_controller/metal/rescue.rb',  
31 - 'method' => ActiveSupport.methods.shuffle.first  
32 - }, {  
33 - 'number' => rand(999),  
34 - 'file' => '[PROJECT_ROOT]/vendor/plugins/seamless_database_pool/lib/seamless_database_pool/controller_filter.rb',  
35 - 'method' => ActiveSupport.methods.shuffle.first  
36 - }, {  
37 - 'number' => rand(999),  
38 - 'file' => '[PROJECT_ROOT]/lib/set_headers.rb',  
39 - 'method' => ActiveSupport.methods.shuffle.first  
40 - }]  
41 - end  
42 -  
43 - it "should be false for line not starting with PROJECT_ROOT" do  
44 - Notice.in_app_backtrace_line?(backtrace[0]).should == false  
45 - end  
46 -  
47 - it "should be false for file in vendor dir" do  
48 - Notice.in_app_backtrace_line?(backtrace[1]).should == false  
49 - end  
50 -  
51 - it "should be true for application file" do  
52 - Notice.in_app_backtrace_line?(backtrace[2]).should == true  
53 - end  
54 - end  
55 -  
56 -  
57 describe "key sanitization" do 27 describe "key sanitization" do
58 before do 28 before do
59 @hash = { "some.key" => { "$nested.key" => {"$Path" => "/", "some$key" => "key"}}} 29 @hash = { "some.key" => { "$nested.key" => {"$Path" => "/", "some$key" => "key"}}}