Commit 28a7ff23aa71ff1b9874dede1d236cdc1efb2145
1 parent
0e90eba3
Exists in
master
and in
1 other branch
Avoid same error if other line than first change
Only the first line of backtrace was use to mark error like same. Avoid this behavior because some language ( like python ) change only last line of backtrace
Showing
2 changed files
with
35 additions
and
24 deletions
Show diff stats
app/models/fingerprint.rb
1 | require 'digest/sha1' | 1 | require 'digest/sha1' |
2 | 2 | ||
3 | class Fingerprint | 3 | class Fingerprint |
4 | + | ||
4 | attr_reader :notice, :api_key | 5 | attr_reader :notice, :api_key |
5 | - | 6 | + |
6 | def self.generate(notice, api_key) | 7 | def self.generate(notice, api_key) |
7 | self.new(notice, api_key).to_s | 8 | self.new(notice, api_key).to_s |
8 | end | 9 | end |
9 | - | 10 | + |
10 | def initialize(notice, api_key) | 11 | def initialize(notice, api_key) |
11 | @notice = notice | 12 | @notice = notice |
12 | @api_key = api_key | 13 | @api_key = api_key |
13 | end | 14 | end |
14 | - | ||
15 | - | ||
16 | - | 15 | + |
17 | def to_s | 16 | def to_s |
18 | Digest::SHA1.hexdigest(fingerprint_source.to_s) | 17 | Digest::SHA1.hexdigest(fingerprint_source.to_s) |
19 | end | 18 | end |
20 | - | ||
21 | - def fingerprint_source | ||
22 | - # Find the first backtrace line with a file and line number. | ||
23 | - if line = notice.backtrace.lines.detect {|l| l.number.present? && l.file.present? } | ||
24 | - # If line exists, only use file and number. | ||
25 | - file_or_message = "#{line.file}:#{line.number}" | ||
26 | - else | ||
27 | - # If no backtrace, use error message | ||
28 | - file_or_message = notice.message | ||
29 | - end | ||
30 | 19 | ||
20 | + def fingerprint_source | ||
31 | { | 21 | { |
32 | :file_or_message => file_or_message, | 22 | :file_or_message => file_or_message, |
33 | :error_class => notice.error_class, | 23 | :error_class => notice.error_class, |
@@ -37,5 +27,9 @@ class Fingerprint | @@ -37,5 +27,9 @@ class Fingerprint | ||
37 | :api_key => api_key | 27 | :api_key => api_key |
38 | } | 28 | } |
39 | end | 29 | end |
40 | - | 30 | + |
31 | + def file_or_message | ||
32 | + @file_or_message ||= notice.message + notice.backtrace.fingerprint | ||
33 | + end | ||
34 | + | ||
41 | end | 35 | end |
spec/models/fingerprint_spec.rb
@@ -3,19 +3,36 @@ require 'spec_helper' | @@ -3,19 +3,36 @@ require 'spec_helper' | ||
3 | describe Fingerprint do | 3 | describe Fingerprint do |
4 | 4 | ||
5 | context '#generate' do | 5 | context '#generate' do |
6 | - before do | ||
7 | - @backtrace = Backtrace.find_or_create(:raw => [ | 6 | + let(:backtrace) { |
7 | + Backtrace.create(:raw => [ | ||
8 | {"number"=>"425", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run__2115867319__process_action__262109504__callbacks"}, | 8 | {"number"=>"425", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run__2115867319__process_action__262109504__callbacks"}, |
9 | {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"}, | 9 | {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"}, |
10 | {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run_process_action_callbacks"} | 10 | {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run_process_action_callbacks"} |
11 | ]) | 11 | ]) |
12 | + } | ||
13 | + let(:notice1) { Fabricate.build(:notice, :backtrace => backtrace) } | ||
14 | + let(:notice2) { Fabricate.build(:notice, :backtrace => backtrace_2) } | ||
15 | + | ||
16 | + context "with same backtrace" do | ||
17 | + let(:backtrace_2) { backtrace } | ||
18 | + it 'should create the same fingerprint for two notices' do | ||
19 | + expect(Fingerprint.generate(notice1, "api key")).to eq Fingerprint.generate(notice2, "api key") | ||
20 | + end | ||
12 | end | 21 | end |
13 | - | ||
14 | - it 'should create the same fingerprint for two notices with the same backtrace' do | ||
15 | - notice1 = Fabricate.build(:notice, :backtrace => @backtrace) | ||
16 | - notice2 = Fabricate.build(:notice, :backtrace => @backtrace) | ||
17 | - | ||
18 | - Fingerprint.generate(notice1, "api key").should == Fingerprint.generate(notice2, "api key") | 22 | + |
23 | + context "with different backtrace with only last line change" do | ||
24 | + let(:backtrace_2) { | ||
25 | + backtrace | ||
26 | + backtrace.lines.last.number = 401 | ||
27 | + backtrace.send(:generate_fingerprint) | ||
28 | + backtrace.save | ||
29 | + backtrace | ||
30 | + } | ||
31 | + it 'should not same fingerprint' do | ||
32 | + expect( | ||
33 | + Fingerprint.generate(notice1, "api key") | ||
34 | + ).not_to eql Fingerprint.generate(notice2, "api key") | ||
35 | + end | ||
19 | end | 36 | end |
20 | end | 37 | end |
21 | 38 |