diff --git a/app/models/backtrace.rb b/app/models/backtrace.rb new file mode 100644 index 0000000..570aa94 --- /dev/null +++ b/app/models/backtrace.rb @@ -0,0 +1,32 @@ +class Backtrace + include Mongoid::Document + include Mongoid::Timestamps + + field :fingerprint + index :fingerprint + + has_many :notices + embeds_many :lines, :class_name => "BacktraceLine" + + after_initialize :generate_fingerprint + + def self.find_or_create(attributes = {}) + new(attributes).similar || create(attributes) + end + + def similar + Backtrace.first(:conditions => { :fingerprint => fingerprint } ) + end + + def raw=(raw) + raw.each do |raw_line| + lines << BacktraceLine.new(BacktraceLineNormalizer.new(raw_line).call) + end + end + + private + def generate_fingerprint + self.fingerprint = Digest::SHA1.hexdigest(lines.join) + end + +end diff --git a/app/models/backtrace_line.rb b/app/models/backtrace_line.rb new file mode 100644 index 0000000..f59c4e1 --- /dev/null +++ b/app/models/backtrace_line.rb @@ -0,0 +1,10 @@ +class BacktraceLine + include Mongoid::Document + + field :number, type: Integer + field :file + field :method + + embedded_in :backtrace +end + diff --git a/app/models/backtrace_line_normalizer.rb b/app/models/backtrace_line_normalizer.rb new file mode 100644 index 0000000..50b6fe6 --- /dev/null +++ b/app/models/backtrace_line_normalizer.rb @@ -0,0 +1,11 @@ +class BacktraceLineNormalizer + def initialize(raw_line) + @raw_line = raw_line + end + + def call + @raw_line.merge! 'file' => "[unknown source]" if @raw_line['file'].blank? + @raw_line.merge! 'method' => @raw_line['method'].gsub(/[0-9_]{10,}+/, "__FRAGMENT__") + end + +end diff --git a/app/models/error_report.rb b/app/models/error_report.rb index 01e9fd6..365cc3b 100644 --- a/app/models/error_report.rb +++ b/app/models/error_report.rb @@ -2,7 +2,7 @@ require 'digest/sha1' require 'hoptoad_notifier' class ErrorReport - attr_reader :error_class, :message, :backtrace, :request, :server_environment, :api_key, :notifier, :user_attributes, :current_user + attr_reader :error_class, :message, :request, :server_environment, :api_key, :notifier, :user_attributes, :current_user def initialize(xml_or_attributes) @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 @app ||= App.find_by_api_key!(api_key) end + def backtrace + @normalized_backtrace ||= Backtrace.find_or_create(:raw => @backtrace) + end + def generate_notice! notice = Notice.new( :message => message, :error_class => error_class, - :backtrace => backtrace, + :backtrace_id => backtrace.id, :request => request, :server_environment => server_environment, :notifier => notifier, @@ -55,7 +59,7 @@ class ErrorReport private def fingerprint_source { - :backtrace => normalized_backtrace.to_s, + :backtrace => backtrace.lines[0..3], :error_class => error_class, :component => component, :action => action, @@ -64,11 +68,5 @@ class ErrorReport } end - def normalized_backtrace - backtrace[0...3].map do |trace| - trace.merge 'method' => trace['method'].gsub(/[0-9_]{10,}+/, "__FRAGMENT__") - end - end - end diff --git a/app/models/notice.rb b/app/models/notice.rb index 9afd13c..f848f2f 100644 --- a/app/models/notice.rb +++ b/app/models/notice.rb @@ -6,15 +6,16 @@ class Notice include Mongoid::Timestamps field :message - field :backtrace, :type => Array field :server_environment, :type => Hash field :request, :type => Hash field :notifier, :type => Hash field :user_attributes, :type => Hash field :current_user, :type => Hash field :error_class + delegate :lines, :to => :backtrace, :prefix => true belongs_to :err + belongs_to :backtrace, :index => true index :created_at index( [ @@ -28,7 +29,7 @@ class Notice before_save :sanitize before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem - validates_presence_of :backtrace, :server_environment, :notifier + validates_presence_of :server_environment, :notifier, :backtrace scope :ordered, order_by(:created_at.asc) scope :reverse_ordered, order_by(:created_at.desc) @@ -92,15 +93,7 @@ class Notice # Backtrace containing only files from the app itself (ignore gems) def app_backtrace - backtrace.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") } - end - - def backtrace - # If gems are vendored into project, treat vendored gem dir as [GEM_ROOT] - (read_attribute(:backtrace) || []).map do |line| - # Changes "[PROJECT_ROOT]/rubygems/ruby/1.9.1/gems" to "[GEM_ROOT]/gems" - line.merge 'file' => line['file'].to_s.gsub(/\[PROJECT_ROOT\]\/.*\/ruby\/[0-9.]+\/gems/, '[GEM_ROOT]/gems') - end + backtrace_lines.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") } end protected @@ -129,8 +122,6 @@ class Notice [:server_environment, :request, :notifier].each do |h| send("#{h}=",sanitize_hash(send(h))) end - # Set unknown backtrace files - read_attribute(:backtrace).each{|line| line['file'] = "[unknown source]" if line['file'].blank? } end def sanitize_hash(h) diff --git a/app/views/issue_trackers/fogbugz_body.txt.erb b/app/views/issue_trackers/fogbugz_body.txt.erb index 56195b9..ff1a9bd 100644 --- a/app/views/issue_trackers/fogbugz_body.txt.erb +++ b/app/views/issue_trackers/fogbugz_body.txt.erb @@ -19,7 +19,7 @@ <%= pretty_hash(notice.session) %> Backtrace - <% for line in notice.backtrace %> + <% for line in notice.backtrace_lines %> <%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> <% end %> diff --git a/app/views/issue_trackers/github_issues_body.txt.erb b/app/views/issue_trackers/github_issues_body.txt.erb index b0edcd5..f3efb45 100644 --- a/app/views/issue_trackers/github_issues_body.txt.erb +++ b/app/views/issue_trackers/github_issues_body.txt.erb @@ -27,7 +27,7 @@ ## Backtrace ## ``` -<% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>** +<% for line in notice.backtrace_lines %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>** <% end %> ``` diff --git a/app/views/issue_trackers/lighthouseapp_body.txt.erb b/app/views/issue_trackers/lighthouseapp_body.txt.erb index 612047c..a5313e9 100644 --- a/app/views/issue_trackers/lighthouseapp_body.txt.erb +++ b/app/views/issue_trackers/lighthouseapp_body.txt.erb @@ -23,7 +23,7 @@ ## Backtrace ## - <% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>** + <% for line in notice.backtrace_lines %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>** <% end %> diff --git a/app/views/issue_trackers/pivotal_body.txt.erb b/app/views/issue_trackers/pivotal_body.txt.erb index b2fadc5..515a347 100644 --- a/app/views/issue_trackers/pivotal_body.txt.erb +++ b/app/views/issue_trackers/pivotal_body.txt.erb @@ -12,6 +12,6 @@ See this exception on Errbit: <%= app_problem_url problem.app, problem %> <%= pretty_hash notice.session %> Backtrace: - <%= notice.backtrace[0..4].map { |line| "#{line['number']}: #{line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '')} -> *#{line['method']}*" }.join "\n" %> + <%= notice.backtrace_lines[0..4].map { |line| "#{line['number']}: #{line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '')} -> *#{line['method']}*" }.join "\n" %> <% end %> diff --git a/app/views/issue_trackers/textile_body.txt.erb b/app/views/issue_trackers/textile_body.txt.erb index c65d886..6dd6192 100644 --- a/app/views/issue_trackers/textile_body.txt.erb +++ b/app/views/issue_trackers/textile_body.txt.erb @@ -32,7 +32,7 @@ h2. Session h2. Backtrace | Line | File | Method | -<% for line in notice.backtrace %>| <%= line['number'] %> | <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> | *<%= line['method'] %>* | +<% for line in notice.backtrace_lines %>| <%= line['number'] %> | <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> | *<%= line['method'] %>* | <% end %> h2. Environment diff --git a/app/views/mailer/err_notification.html.haml b/app/views/mailer/err_notification.html.haml index 9884876..d6fbb23 100644 --- a/app/views/mailer/err_notification.html.haml +++ b/app/views/mailer/err_notification.html.haml @@ -36,7 +36,7 @@ - if @notice.request['url'].present? = link_to @notice.request['url'], @notice.request['url'] %p.heading BACKTRACE: - - @notice.backtrace.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| + - @notice.backtrace_lines.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| %p.backtrace= line %br diff --git a/app/views/mailer/err_notification.text.erb b/app/views/mailer/err_notification.text.erb index 22acf3e..5c2151b 100644 --- a/app/views/mailer/err_notification.text.erb +++ b/app/views/mailer/err_notification.text.erb @@ -26,7 +26,7 @@ URL: BACKTRACE: -<% @notice.backtrace.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| %> +<% @notice.backtrace_lines.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| %> <%= line %> <% end %> diff --git a/app/views/notices/_atom_entry.html.haml b/app/views/notices/_atom_entry.html.haml index 942fcf7..8021c50 100644 --- a/app/views/notices/_atom_entry.html.haml +++ b/app/views/notices/_atom_entry.html.haml @@ -22,7 +22,7 @@ %h3 Backtrace %table - - for line in notice.backtrace + - for line in notice.backtrace_lines %tr %td = "#{line['number']}:" diff --git a/app/views/problems/show.html.haml b/app/views/problems/show.html.haml index 596964d..4326e61 100644 --- a/app/views/problems/show.html.haml +++ b/app/views/problems/show.html.haml @@ -63,7 +63,7 @@ #backtrace %h3 Backtrace - = render 'notices/backtrace', :lines => @notice.backtrace + = render 'notices/backtrace', :lines => @notice.backtrace_lines - if @notice.user_attributes.present? #user_attributes diff --git a/spec/fabricators/err_fabricator.rb b/spec/fabricators/err_fabricator.rb index 2b92461..f22ce5e 100644 --- a/spec/fabricators/err_fabricator.rb +++ b/spec/fabricators/err_fabricator.rb @@ -9,19 +9,19 @@ end Fabricator :notice do err! message 'FooError: Too Much Bar' - backtrace { random_backtrace } + backtrace! server_environment { {'environment-name' => 'production'} } request {{ 'component' => 'foo', 'action' => 'bar' }} notifier {{ 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }} end -def random_backtrace - backtrace = [] - 99.times {|t| backtrace << { - 'number' => rand(999), - 'file' => "/path/to/file/#{SecureRandom.hex(4)}.rb", - 'method' => ActiveSupport.methods.shuffle.first - }} - backtrace +Fabricator :backtrace do + fingerprint "fingerprint" + lines(:count => 99) { Fabricate.build(:backtrace_line) } end +Fabricator :backtrace_line do + number { rand(999) } + file { "/path/to/file/#{SecureRandom.hex(4)}.rb" } + method(:method) { ActiveSupport.methods.shuffle.first } +end diff --git a/spec/models/app_spec.rb b/spec/models/app_spec.rb index c13cde1..e55f037 100644 --- a/spec/models/app_spec.rb +++ b/spec/models/app_spec.rb @@ -200,8 +200,8 @@ describe App do it 'captures the backtrace' do @notice = App.report_error!(@xml) - @notice.backtrace.size.should == 73 - @notice.backtrace.last['file'].should == '[GEM_ROOT]/bin/rake' + @notice.backtrace_lines.size.should == 73 + @notice.backtrace_lines.last['file'].should == '[GEM_ROOT]/bin/rake' end it 'captures the server_environment' do @@ -228,7 +228,7 @@ describe App do it "should handle params with only a single line of backtrace" do xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read lambda { @notice = App.report_error!(xml) }.should_not raise_error - @notice.backtrace.length.should == 1 + @notice.backtrace_lines.length.should == 1 end it 'captures the current_user' do @@ -238,7 +238,7 @@ describe App do @notice.current_user['email'].should == 'mr.bean@example.com' @notice.current_user['username'].should == 'mrbean' end - + end diff --git a/spec/models/backtrace_line_normalizer_spec.rb b/spec/models/backtrace_line_normalizer_spec.rb new file mode 100644 index 0000000..bc2b77d --- /dev/null +++ b/spec/models/backtrace_line_normalizer_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe BacktraceLineNormalizer do + subject { described_class.new(raw_line).call } + + describe "sanitize file" do + let(:raw_line) { { 'number' => rand(999), 'file' => nil, 'method' => ActiveSupport.methods.shuffle.first.to_s } } + + it "should replace nil file with [unknown source]" do + subject['file'].should == "[unknown source]" + end + + end +end diff --git a/spec/models/backtrace_spec.rb b/spec/models/backtrace_spec.rb new file mode 100644 index 0000000..c240886 --- /dev/null +++ b/spec/models/backtrace_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Backtrace do + subject { described_class.new } + + its(:fingerprint) { should be_present } + + describe "#similar" do + context "no similar backtrace" do + its(:similar) { should be_nil } + end + + context "similar backtrace exist" do + let!(:similar_backtrace) { Fabricate(:backtrace, :fingerprint => fingerprint) } + let(:fingerprint) { "fingerprint" } + + before { subject.stub(:fingerprint => fingerprint) } + + its(:similar) { should == similar_backtrace } + end + end + + describe "find_or_create" do + subject { described_class.find_or_create(attributes) } + let(:attributes) { mock :attributes } + let(:backtrace) { mock :backtrace } + + before { described_class.stub(:new => backtrace) } + + context "no similar backtrace" do + before { backtrace.stub(:similar => nil) } + it "create new backtrace" do + described_class.should_receive(:create).with(attributes) + + described_class.find_or_create(attributes) + end + end + + context "similar backtrace exist" do + let(:similar_backtrace) { mock :similar_backtrace } + before { backtrace.stub(:similar => similar_backtrace) } + + it { should == similar_backtrace } + end + end +end diff --git a/spec/models/notice_observer_spec.rb b/spec/models/notice_observer_spec.rb index dca73fe..e7f3b49 100644 --- a/spec/models/notice_observer_spec.rb +++ b/spec/models/notice_observer_spec.rb @@ -46,6 +46,7 @@ describe NoticeObserver do describe "should send a notification if a notification service is configured" do let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:campfire_notification_service))} let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } + let(:backtrace) { Fabricate(:backtrace) } before do Errbit::Config.per_app_email_at_notices = true @@ -59,13 +60,14 @@ describe NoticeObserver do app.notification_service.should_receive(:create_notification) Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) + :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) end end describe "should not send a notification if a notification service is not configured" do let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))} let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } + let(:backtrace) { Fabricate(:backtrace) } before do Errbit::Config.per_app_email_at_notices = true @@ -79,7 +81,7 @@ describe NoticeObserver do app.notification_service.should_not_receive(:create_notification) Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) + :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) end end diff --git a/spec/views/notices/_backtrace.html.haml_spec.rb b/spec/views/notices/_backtrace.html.haml_spec.rb deleted file mode 100644 index 07dca60..0000000 --- a/spec/views/notices/_backtrace.html.haml_spec.rb +++ /dev/null @@ -1,18 +0,0 @@ -require 'spec_helper' - -describe "notices/_backtrace.html.haml" do - describe 'missing file in backtrace' do - let(:notice) do - backtrace = { 'number' => rand(999), 'file' => nil, 'method' => ActiveSupport.methods.shuffle.first } - Fabricate(:notice, :backtrace => [backtrace]) - end - - it "should replace nil file with [unknown source]" do - assign :app, notice.err.app - - render "notices/backtrace", :lines => notice.backtrace - rendered.should match(/\[unknown source\]/) - end - end -end - -- libgit2 0.21.2