Commit 38f58105fb191287bc252a848562662f1468981b
Exists in
master
and in
1 other branch
Merge pull request #555 from shingara/bugs/not_same_if_change_last_backtrace_line
Avoid same error if other line than first change
Showing
2 changed files
with
35 additions
and
24 deletions
Show diff stats
app/models/fingerprint.rb
1 | 1 | require 'digest/sha1' |
2 | 2 | |
3 | 3 | class Fingerprint |
4 | + | |
4 | 5 | attr_reader :notice, :api_key |
5 | - | |
6 | + | |
6 | 7 | def self.generate(notice, api_key) |
7 | 8 | self.new(notice, api_key).to_s |
8 | 9 | end |
9 | - | |
10 | + | |
10 | 11 | def initialize(notice, api_key) |
11 | 12 | @notice = notice |
12 | 13 | @api_key = api_key |
13 | 14 | end |
14 | - | |
15 | - | |
16 | - | |
15 | + | |
17 | 16 | def to_s |
18 | 17 | Digest::SHA1.hexdigest(fingerprint_source.to_s) |
19 | 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 | 22 | :file_or_message => file_or_message, |
33 | 23 | :error_class => notice.error_class, |
... | ... | @@ -37,5 +27,9 @@ class Fingerprint |
37 | 27 | :api_key => api_key |
38 | 28 | } |
39 | 29 | end |
40 | - | |
30 | + | |
31 | + def file_or_message | |
32 | + @file_or_message ||= notice.message + notice.backtrace.fingerprint | |
33 | + end | |
34 | + | |
41 | 35 | end | ... | ... |
spec/models/fingerprint_spec.rb
... | ... | @@ -3,19 +3,36 @@ require 'spec_helper' |
3 | 3 | describe Fingerprint do |
4 | 4 | |
5 | 5 | context '#generate' do |
6 | - before do | |
7 | - @backtrace = Backtrace.find_or_create(:raw => [ | |
6 | + let(:backtrace) { | |
7 | + Backtrace.create(:raw => [ | |
8 | 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 | 9 | {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"}, |
10 | 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 | 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 | 36 | end |
20 | 37 | end |
21 | 38 | ... | ... |