Download as
Browse Code »

Commit 559d808d859ad84f835e6207ac4717a8c0b14959

Authored by Robert Lail
Committed by Bob Lail
1 parent e1cf6429
Exists in master and in 1 other branch production

Errs are like a hash table whose values are arrays of notices and whose keys are… 

… a composite of error_class + component + action + environment + backtrace[0..3].

These fields no longer need to be cached separately on Err because they are now cached on Problem.

This commit combines all those values into one simple key: fingerprint. This should:

 - Make err lookups faster
 - Make it easier to reason about, test, and replace the fingerprint algorithm
 - Make the relationship between Problems, Errs, and Notices clearer
Showing 6 changed files with 26 additions and 23 deletions   Show diff stats
app/models/app.rb
  Diff comments   View file @559d808
... ... @@ -42,6 +42,12 @@ class App
42 42 accepts_nested_attributes_for :notification_service, :allow_destroy => true,
43 43 :reject_if => proc { |attrs| !NotificationService.subclasses.map(&:to_s).include?(attrs[:type].to_s) }
44 44  
  45 + # Acceps a hash with the following attributes:
  46 + #
  47 + # * <tt>:error_class</tt> - the class of error (required to create a new Problem)
  48 + # * <tt>:environment</tt> - the environment the source app was running in (required to create a new Problem)
  49 + # * <tt>:fingerprint</tt> - a unique value identifying the notice
  50 + #
45 51 def find_or_create_err!(attrs)
46 52 Err.where(
47 53 :fingerprint => attrs[:fingerprint]
... ...
app/models/err.rb
  Diff comments   View file @559d808
... ... @@ -6,19 +6,16 @@ class Err
6 6 include Mongoid::Document
7 7 include Mongoid::Timestamps
8 8  
9   - field :error_class, :default => "UnknownError"
10   - field :component
11   - field :action
12   - field :environment, :default => "unknown"
13 9 field :fingerprint
14 10  
15   - belongs_to :problem
16 11 index :problem_id
17   - index :error_class
18 12 index :fingerprint
19 13  
  14 + belongs_to :problem
20 15 has_many :notices, :inverse_of => :err, :dependent => :destroy
21 16  
  17 + validates_presence_of :problem_id, :fingerprint
  18 +
22 19 delegate :app, :resolved?, :to => :problem
23 20  
24 21 end
... ...
app/models/error_report.rb
  Diff comments   View file @559d808
... ... @@ -28,14 +28,6 @@ class ErrorReport
28 28 rails_env
29 29 end
30 30  
31   - def component
32   - request['component'] || 'unknown'
33   - end
34   -
35   - def action
36   - request['action']
37   - end
38   -
39 31 def app
40 32 @app ||= App.where(:api_key => api_key).first
41 33 end
... ... @@ -71,8 +63,6 @@ class ErrorReport
71 63 def error
72 64 @error ||= app.find_or_create_err!(
73 65 :error_class => error_class,
74   - :component => component,
75   - :action => action,
76 66 :environment => rails_env,
77 67 :fingerprint => fingerprint
78 68 )
... ...
spec/fabricators/err_fabricator.rb
  Diff comments   View file @559d808
1 1 Fabricator :err do
2 2 problem!
3   - error_class! { 'FooError' }
4   - component 'foo'
5   - action 'bar'
6   - environment 'production'
  3 + fingerprint 'some-finger-print'
7 4 end
8 5  
9 6 Fabricator :notice do
... ...
spec/models/app_spec.rb
  Diff comments   View file @559d808
... ... @@ -162,9 +162,8 @@ describe App do
162 162 @app = Fabricate(:app)
163 163 @conditions = {
164 164 :error_class => 'Whoops',
165   - :component => 'Foo',
166   - :action => 'bar',
167   - :environment => 'production'
  165 + :environment => 'production',
  166 + :fingerprint => 'some-finger-print'
168 167 }
169 168 end
170 169  
... ...
spec/models/err_spec.rb
  Diff comments   View file @559d808
... ... @@ -2,4 +2,18 @@ require &#39;spec_helper&#39;
2 2  
3 3 describe Err do
4 4  
  5 + context 'validations' do
  6 + it 'requires a fingerprint' do
  7 + err = Fabricate.build(:err, :fingerprint => nil)
  8 + err.should_not be_valid
  9 + err.errors[:fingerprint].should include("can't be blank")
  10 + end
  11 +
  12 + it 'requires a problem' do
  13 + err = Fabricate.build(:err, :problem_id => nil)
  14 + err.should_not be_valid
  15 + err.errors[:problem_id].should include("can't be blank")
  16 + end
  17 + end
  18 +
5 19 end
... ...