Download as
Browse Code »

Commit 5d02ba79907cc025d986395e6ab462237bd19736

Authored by Marcin Ciunelis
1 parent f761cda4
Exists in master and in 1 other branch production

backtrace extraced to it's class

Showing 20 changed files with 150 additions and 64 deletions   Show diff stats
app/models/backtrace.rb 0 → 100644
  Diff comments   View file @5d02ba7
... ... @@ -0,0 +1,32 @@
  1 +class Backtrace
  2 + include Mongoid::Document
  3 + include Mongoid::Timestamps
  4 +
  5 + field :fingerprint
  6 + index :fingerprint
  7 +
  8 + has_many :notices
  9 + embeds_many :lines, :class_name => "BacktraceLine"
  10 +
  11 + after_initialize :generate_fingerprint
  12 +
  13 + def self.find_or_create(attributes = {})
  14 + new(attributes).similar || create(attributes)
  15 + end
  16 +
  17 + def similar
  18 + Backtrace.first(:conditions => { :fingerprint => fingerprint } )
  19 + end
  20 +
  21 + def raw=(raw)
  22 + raw.each do |raw_line|
  23 + lines << BacktraceLine.new(BacktraceLineNormalizer.new(raw_line).call)
  24 + end
  25 + end
  26 +
  27 + private
  28 + def generate_fingerprint
  29 + self.fingerprint = Digest::SHA1.hexdigest(lines.join)
  30 + end
  31 +
  32 +end
... ...
app/models/backtrace_line.rb 0 → 100644
  Diff comments   View file @5d02ba7
... ... @@ -0,0 +1,10 @@
  1 +class BacktraceLine
  2 + include Mongoid::Document
  3 +
  4 + field :number, type: Integer
  5 + field :file
  6 + field :method
  7 +
  8 + embedded_in :backtrace
  9 +end
  10 +
... ...
app/models/backtrace_line_normalizer.rb 0 → 100644
  Diff comments   View file @5d02ba7
... ... @@ -0,0 +1,11 @@
  1 +class BacktraceLineNormalizer
  2 + def initialize(raw_line)
  3 + @raw_line = raw_line
  4 + end
  5 +
  6 + def call
  7 + @raw_line.merge! 'file' => "[unknown source]" if @raw_line['file'].blank?
  8 + @raw_line.merge! 'method' => @raw_line['method'].gsub(/[0-9_]{10,}+/, "__FRAGMENT__")
  9 + end
  10 +
  11 +end
... ...
app/models/error_report.rb
  Diff comments   View file @5d02ba7
... ... @@ -2,7 +2,7 @@ require &#39;digest/sha1&#39;
2 2 require 'hoptoad_notifier'
3 3  
4 4 class ErrorReport
5   - attr_reader :error_class, :message, :backtrace, :request, :server_environment, :api_key, :notifier, :user_attributes, :current_user
  5 + attr_reader :error_class, :message, :request, :server_environment, :api_key, :notifier, :user_attributes, :current_user
6 6  
7 7 def initialize(xml_or_attributes)
8 8 @attributes = (xml_or_attributes.is_a?(String) ? Hoptoad.parse_xml!(xml_or_attributes) : xml_or_attributes).with_indifferent_access
... ... @@ -29,11 +29,15 @@ class ErrorReport
29 29 @app ||= App.find_by_api_key!(api_key)
30 30 end
31 31  
  32 + def backtrace
  33 + @normalized_backtrace ||= Backtrace.find_or_create(:raw => @backtrace)
  34 + end
  35 +
32 36 def generate_notice!
33 37 notice = Notice.new(
34 38 :message => message,
35 39 :error_class => error_class,
36   - :backtrace => backtrace,
  40 + :backtrace_id => backtrace.id,
37 41 :request => request,
38 42 :server_environment => server_environment,
39 43 :notifier => notifier,
... ... @@ -55,7 +59,7 @@ class ErrorReport
55 59 private
56 60 def fingerprint_source
57 61 {
58   - :backtrace => normalized_backtrace.to_s,
  62 + :backtrace => backtrace.lines[0..3],
59 63 :error_class => error_class,
60 64 :component => component,
61 65 :action => action,
... ... @@ -64,11 +68,5 @@ class ErrorReport
64 68 }
65 69 end
66 70  
67   - def normalized_backtrace
68   - backtrace[0...3].map do |trace|
69   - trace.merge 'method' => trace['method'].gsub(/[0-9_]{10,}+/, "__FRAGMENT__")
70   - end
71   - end
72   -
73 71 end
74 72  
... ...
app/models/notice.rb
  Diff comments   View file @5d02ba7
... ... @@ -6,15 +6,16 @@ class Notice
6 6 include Mongoid::Timestamps
7 7  
8 8 field :message
9   - field :backtrace, :type => Array
10 9 field :server_environment, :type => Hash
11 10 field :request, :type => Hash
12 11 field :notifier, :type => Hash
13 12 field :user_attributes, :type => Hash
14 13 field :current_user, :type => Hash
15 14 field :error_class
  15 + delegate :lines, :to => :backtrace, :prefix => true
16 16  
17 17 belongs_to :err
  18 + belongs_to :backtrace, :index => true
18 19 index :created_at
19 20 index(
20 21 [
... ... @@ -28,7 +29,7 @@ class Notice
28 29 before_save :sanitize
29 30 before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem
30 31  
31   - validates_presence_of :backtrace, :server_environment, :notifier
  32 + validates_presence_of :server_environment, :notifier, :backtrace
32 33  
33 34 scope :ordered, order_by(:created_at.asc)
34 35 scope :reverse_ordered, order_by(:created_at.desc)
... ... @@ -92,15 +93,7 @@ class Notice
92 93  
93 94 # Backtrace containing only files from the app itself (ignore gems)
94 95 def app_backtrace
95   - backtrace.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") }
96   - end
97   -
98   - def backtrace
99   - # If gems are vendored into project, treat vendored gem dir as [GEM_ROOT]
100   - (read_attribute(:backtrace) || []).map do |line|
101   - # Changes "[PROJECT_ROOT]/rubygems/ruby/1.9.1/gems" to "[GEM_ROOT]/gems"
102   - line.merge 'file' => line['file'].to_s.gsub(/\[PROJECT_ROOT\]\/.*\/ruby\/[0-9.]+\/gems/, '[GEM_ROOT]/gems')
103   - end
  96 + backtrace_lines.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") }
104 97 end
105 98  
106 99 protected
... ... @@ -129,8 +122,6 @@ class Notice
129 122 [:server_environment, :request, :notifier].each do |h|
130 123 send("#{h}=",sanitize_hash(send(h)))
131 124 end
132   - # Set unknown backtrace files
133   - read_attribute(:backtrace).each{|line| line['file'] = "[unknown source]" if line['file'].blank? }
134 125 end
135 126  
136 127 def sanitize_hash(h)
... ...
app/views/issue_trackers/fogbugz_body.txt.erb
  Diff comments   View file @5d02ba7
... ... @@ -19,7 +19,7 @@
19 19 <%= pretty_hash(notice.session) %>
20 20  
21 21 Backtrace
22   - <% for line in notice.backtrace %>
  22 + <% for line in notice.backtrace_lines %>
23 23 <%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %>
24 24 <% end %>
25 25  
... ...
app/views/issue_trackers/github_issues_body.txt.erb
  Diff comments   View file @5d02ba7
... ... @@ -27,7 +27,7 @@
27 27  
28 28 ## Backtrace ##
29 29 ```
30   -<% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>**
  30 +<% for line in notice.backtrace_lines %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>**
31 31 <% end %>
32 32 ```
33 33  
... ...
app/views/issue_trackers/lighthouseapp_body.txt.erb
  Diff comments   View file @5d02ba7
... ... @@ -23,7 +23,7 @@
23 23  
24 24 ## Backtrace ##
25 25 <code>
26   - <% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>**
  26 + <% for line in notice.backtrace_lines %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>**
27 27 <% end %>
28 28 </code>
29 29  
... ...
app/views/issue_trackers/pivotal_body.txt.erb
  Diff comments   View file @5d02ba7
... ... @@ -12,6 +12,6 @@ See this exception on Errbit: &lt;%= app_problem_url problem.app, problem %&gt;
12 12 <%= pretty_hash notice.session %>
13 13  
14 14 Backtrace:
15   - <%= notice.backtrace[0..4].map { |line| "#{line['number']}: #{line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '')} -> *#{line['method']}*" }.join "\n" %>
  15 + <%= notice.backtrace_lines[0..4].map { |line| "#{line['number']}: #{line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '')} -> *#{line['method']}*" }.join "\n" %>
16 16 <% end %>
17 17  
... ...
app/views/issue_trackers/textile_body.txt.erb
  Diff comments   View file @5d02ba7
... ... @@ -32,7 +32,7 @@ h2. Session
32 32 h2. Backtrace
33 33  
34 34 | Line | File | Method |
35   -<% for line in notice.backtrace %>| <%= line['number'] %> | <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> | *<%= line['method'] %>* |
  35 +<% for line in notice.backtrace_lines %>| <%= line['number'] %> | <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> | *<%= line['method'] %>* |
36 36 <% end %>
37 37  
38 38 h2. Environment
... ...
app/views/mailer/err_notification.html.haml
  Diff comments   View file @5d02ba7
... ... @@ -36,7 +36,7 @@
36 36 - if @notice.request['url'].present?
37 37 = link_to @notice.request['url'], @notice.request['url']
38 38 %p.heading BACKTRACE:
39   - - @notice.backtrace.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line|
  39 + - @notice.backtrace_lines.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line|
40 40 %p.backtrace= line
41 41 %br
42 42  
... ...
app/views/mailer/err_notification.text.erb
  Diff comments   View file @5d02ba7
... ... @@ -26,7 +26,7 @@ URL:
26 26  
27 27 BACKTRACE:
28 28  
29   -<% @notice.backtrace.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| %>
  29 +<% @notice.backtrace_lines.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| %>
30 30 <%= line %>
31 31 <% end %>
32 32  
... ...
app/views/notices/_atom_entry.html.haml
  Diff comments   View file @5d02ba7
... ... @@ -22,7 +22,7 @@
22 22  
23 23 %h3 Backtrace
24 24 %table
25   - - for line in notice.backtrace
  25 + - for line in notice.backtrace_lines
26 26 %tr
27 27 %td
28 28 = "#{line['number']}:"
... ...
app/views/problems/show.html.haml
  Diff comments   View file @5d02ba7
... ... @@ -63,7 +63,7 @@
63 63  
64 64 #backtrace
65 65 %h3 Backtrace
66   - = render 'notices/backtrace', :lines => @notice.backtrace
  66 + = render 'notices/backtrace', :lines => @notice.backtrace_lines
67 67  
68 68 - if @notice.user_attributes.present?
69 69 #user_attributes
... ...
spec/fabricators/err_fabricator.rb
  Diff comments   View file @5d02ba7
... ... @@ -9,19 +9,19 @@ end
9 9 Fabricator :notice do
10 10 err!
11 11 message 'FooError: Too Much Bar'
12   - backtrace { random_backtrace }
  12 + backtrace!
13 13 server_environment { {'environment-name' => 'production'} }
14 14 request {{ 'component' => 'foo', 'action' => 'bar' }}
15 15 notifier {{ 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }}
16 16 end
17 17  
18   -def random_backtrace
19   - backtrace = []
20   - 99.times {|t| backtrace << {
21   - 'number' => rand(999),
22   - 'file' => "/path/to/file/#{SecureRandom.hex(4)}.rb",
23   - 'method' => ActiveSupport.methods.shuffle.first
24   - }}
25   - backtrace
  18 +Fabricator :backtrace do
  19 + fingerprint "fingerprint"
  20 + lines(:count => 99) { Fabricate.build(:backtrace_line) }
26 21 end
27 22  
  23 +Fabricator :backtrace_line do
  24 + number { rand(999) }
  25 + file { "/path/to/file/#{SecureRandom.hex(4)}.rb" }
  26 + method(:method) { ActiveSupport.methods.shuffle.first }
  27 +end
... ...
spec/models/app_spec.rb
  Diff comments   View file @5d02ba7
... ... @@ -200,8 +200,8 @@ describe App do
200 200  
201 201 it 'captures the backtrace' do
202 202 @notice = App.report_error!(@xml)
203   - @notice.backtrace.size.should == 73
204   - @notice.backtrace.last['file'].should == '[GEM_ROOT]/bin/rake'
  203 + @notice.backtrace_lines.size.should == 73
  204 + @notice.backtrace_lines.last['file'].should == '[GEM_ROOT]/bin/rake'
205 205 end
206 206  
207 207 it 'captures the server_environment' do
... ... @@ -228,7 +228,7 @@ describe App do
228 228 it "should handle params with only a single line of backtrace" do
229 229 xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read
230 230 lambda { @notice = App.report_error!(xml) }.should_not raise_error
231   - @notice.backtrace.length.should == 1
  231 + @notice.backtrace_lines.length.should == 1
232 232 end
233 233  
234 234 it 'captures the current_user' do
... ... @@ -238,7 +238,7 @@ describe App do
238 238 @notice.current_user['email'].should == 'mr.bean@example.com'
239 239 @notice.current_user['username'].should == 'mrbean'
240 240 end
241   -
  241 +
242 242 end
243 243  
244 244  
... ...
spec/models/backtrace_line_normalizer_spec.rb 0 → 100644
  Diff comments   View file @5d02ba7
... ... @@ -0,0 +1,14 @@
  1 +require 'spec_helper'
  2 +
  3 +describe BacktraceLineNormalizer do
  4 + subject { described_class.new(raw_line).call }
  5 +
  6 + describe "sanitize file" do
  7 + let(:raw_line) { { 'number' => rand(999), 'file' => nil, 'method' => ActiveSupport.methods.shuffle.first.to_s } }
  8 +
  9 + it "should replace nil file with [unknown source]" do
  10 + subject['file'].should == "[unknown source]"
  11 + end
  12 +
  13 + end
  14 +end
... ...
spec/models/backtrace_spec.rb 0 → 100644
  Diff comments   View file @5d02ba7
... ... @@ -0,0 +1,46 @@
  1 +require 'spec_helper'
  2 +
  3 +describe Backtrace do
  4 + subject { described_class.new }
  5 +
  6 + its(:fingerprint) { should be_present }
  7 +
  8 + describe "#similar" do
  9 + context "no similar backtrace" do
  10 + its(:similar) { should be_nil }
  11 + end
  12 +
  13 + context "similar backtrace exist" do
  14 + let!(:similar_backtrace) { Fabricate(:backtrace, :fingerprint => fingerprint) }
  15 + let(:fingerprint) { "fingerprint" }
  16 +
  17 + before { subject.stub(:fingerprint => fingerprint) }
  18 +
  19 + its(:similar) { should == similar_backtrace }
  20 + end
  21 + end
  22 +
  23 + describe "find_or_create" do
  24 + subject { described_class.find_or_create(attributes) }
  25 + let(:attributes) { mock :attributes }
  26 + let(:backtrace) { mock :backtrace }
  27 +
  28 + before { described_class.stub(:new => backtrace) }
  29 +
  30 + context "no similar backtrace" do
  31 + before { backtrace.stub(:similar => nil) }
  32 + it "create new backtrace" do
  33 + described_class.should_receive(:create).with(attributes)
  34 +
  35 + described_class.find_or_create(attributes)
  36 + end
  37 + end
  38 +
  39 + context "similar backtrace exist" do
  40 + let(:similar_backtrace) { mock :similar_backtrace }
  41 + before { backtrace.stub(:similar => similar_backtrace) }
  42 +
  43 + it { should == similar_backtrace }
  44 + end
  45 + end
  46 +end
... ...
spec/models/notice_observer_spec.rb
  Diff comments   View file @5d02ba7
... ... @@ -46,6 +46,7 @@ describe NoticeObserver do
46 46 describe "should send a notification if a notification service is configured" do
47 47 let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:campfire_notification_service))}
48 48 let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) }
  49 + let(:backtrace) { Fabricate(:backtrace) }
49 50  
50 51 before do
51 52 Errbit::Config.per_app_email_at_notices = true
... ... @@ -59,13 +60,14 @@ describe NoticeObserver do
59 60 app.notification_service.should_receive(:create_notification)
60 61  
61 62 Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'},
62   - :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' })
  63 + :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' })
63 64 end
64 65 end
65 66  
66 67 describe "should not send a notification if a notification service is not configured" do
67 68 let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))}
68 69 let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) }
  70 + let(:backtrace) { Fabricate(:backtrace) }
69 71  
70 72 before do
71 73 Errbit::Config.per_app_email_at_notices = true
... ... @@ -79,7 +81,7 @@ describe NoticeObserver do
79 81 app.notification_service.should_not_receive(:create_notification)
80 82  
81 83 Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'},
82   - :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' })
  84 + :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' })
83 85 end
84 86 end
85 87  
... ...
spec/views/notices/_backtrace.html.haml_spec.rb
View file @f761cda
... ... @@ -1,18 +0,0 @@
1   -require 'spec_helper'
2   -
3   -describe "notices/_backtrace.html.haml" do
4   - describe 'missing file in backtrace' do
5   - let(:notice) do
6   - backtrace = { 'number' => rand(999), 'file' => nil, 'method' => ActiveSupport.methods.shuffle.first }
7   - Fabricate(:notice, :backtrace => [backtrace])
8   - end
9   -
10   - it "should replace nil file with [unknown source]" do
11   - assign :app, notice.err.app
12   -
13   - render "notices/backtrace", :lines => notice.backtrace
14   - rendered.should match(/\[unknown source\]/)
15   - end
16   - end
17   -end
18   -