Commit ed088890cffb2e093b780554225a8bcad5f69a5d
Exists in
master
and in
1 other branch
Merge branch 'master' of github.com:errbit/errbit
Showing
11 changed files
with
51 additions
and
40 deletions
Show diff stats
app/interactors/problem_updater_cache.rb
| @@ -43,8 +43,6 @@ class ProblemUpdaterCache | @@ -43,8 +43,6 @@ class ProblemUpdaterCache | ||
| 43 | attrs[:last_notice_at] = last_notice.created_at if last_notice | 43 | attrs[:last_notice_at] = last_notice.created_at if last_notice |
| 44 | attrs.merge!( | 44 | attrs.merge!( |
| 45 | :message => notice.message, | 45 | :message => notice.message, |
| 46 | - :environment => notice.environment_name, | ||
| 47 | - :error_class => notice.error_class, | ||
| 48 | :where => notice.where, | 46 | :where => notice.where, |
| 49 | :messages => attribute_count(:message, messages), | 47 | :messages => attribute_count(:message, messages), |
| 50 | :hosts => attribute_count(:host, hosts), | 48 | :hosts => attribute_count(:host, hosts), |
app/models/app.rb
| @@ -42,11 +42,17 @@ class App | @@ -42,11 +42,17 @@ class App | ||
| 42 | accepts_nested_attributes_for :notification_service, :allow_destroy => true, | 42 | accepts_nested_attributes_for :notification_service, :allow_destroy => true, |
| 43 | :reject_if => proc { |attrs| !NotificationService.subclasses.map(&:to_s).include?(attrs[:type].to_s) } | 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 | def find_or_create_err!(attrs) | 51 | def find_or_create_err!(attrs) |
| 46 | Err.where( | 52 | Err.where( |
| 47 | :fingerprint => attrs[:fingerprint] | 53 | :fingerprint => attrs[:fingerprint] |
| 48 | ).first || | 54 | ).first || |
| 49 | - problems.create!.errs.create!(attrs) | 55 | + problems.create!(attrs.slice(:error_class, :environment)).errs.create!(attrs) |
| 50 | end | 56 | end |
| 51 | 57 | ||
| 52 | # Mongoid Bug: find(id) on association proxies returns an Enumerator | 58 | # Mongoid Bug: find(id) on association proxies returns an Enumerator |
app/models/err.rb
| @@ -6,20 +6,16 @@ class Err | @@ -6,20 +6,16 @@ class Err | ||
| 6 | include Mongoid::Document | 6 | include Mongoid::Document |
| 7 | include Mongoid::Timestamps | 7 | include Mongoid::Timestamps |
| 8 | 8 | ||
| 9 | - field :error_class, :default => "UnknownError" | ||
| 10 | - field :component | ||
| 11 | - field :action | ||
| 12 | - field :environment, :default => "unknown" | ||
| 13 | field :fingerprint | 9 | field :fingerprint |
| 14 | 10 | ||
| 15 | - belongs_to :problem | ||
| 16 | index :problem_id | 11 | index :problem_id |
| 17 | - index :error_class | ||
| 18 | index :fingerprint | 12 | index :fingerprint |
| 19 | 13 | ||
| 14 | + belongs_to :problem | ||
| 20 | has_many :notices, :inverse_of => :err, :dependent => :destroy | 15 | has_many :notices, :inverse_of => :err, :dependent => :destroy |
| 21 | 16 | ||
| 17 | + validates_presence_of :problem_id, :fingerprint | ||
| 18 | + | ||
| 22 | delegate :app, :resolved?, :to => :problem | 19 | delegate :app, :resolved?, :to => :problem |
| 23 | 20 | ||
| 24 | end | 21 | end |
| 25 | - |
app/models/error_report.rb
| @@ -23,15 +23,9 @@ class ErrorReport | @@ -23,15 +23,9 @@ class ErrorReport | ||
| 23 | end | 23 | end |
| 24 | 24 | ||
| 25 | def rails_env | 25 | def rails_env |
| 26 | - server_environment['environment-name'] || 'development' | ||
| 27 | - end | ||
| 28 | - | ||
| 29 | - def component | ||
| 30 | - request['component'] || 'unknown' | ||
| 31 | - end | ||
| 32 | - | ||
| 33 | - def action | ||
| 34 | - request['action'] | 26 | + rails_env = server_environment['environment-name'] |
| 27 | + rails_env = 'development' if rails_env.blank? | ||
| 28 | + rails_env | ||
| 35 | end | 29 | end |
| 36 | 30 | ||
| 37 | def app | 31 | def app |
| @@ -69,8 +63,6 @@ class ErrorReport | @@ -69,8 +63,6 @@ class ErrorReport | ||
| 69 | def error | 63 | def error |
| 70 | @error ||= app.find_or_create_err!( | 64 | @error ||= app.find_or_create_err!( |
| 71 | :error_class => error_class, | 65 | :error_class => error_class, |
| 72 | - :component => component, | ||
| 73 | - :action => action, | ||
| 74 | :environment => rails_env, | 66 | :environment => rails_env, |
| 75 | :fingerprint => fingerprint | 67 | :fingerprint => fingerprint |
| 76 | ) | 68 | ) |
app/models/problem.rb
| @@ -39,6 +39,8 @@ class Problem | @@ -39,6 +39,8 @@ class Problem | ||
| 39 | has_many :errs, :inverse_of => :problem, :dependent => :destroy | 39 | has_many :errs, :inverse_of => :problem, :dependent => :destroy |
| 40 | has_many :comments, :inverse_of => :err, :dependent => :destroy | 40 | has_many :comments, :inverse_of => :err, :dependent => :destroy |
| 41 | 41 | ||
| 42 | + validates_presence_of :error_class, :environment | ||
| 43 | + | ||
| 42 | before_create :cache_app_attributes | 44 | before_create :cache_app_attributes |
| 43 | 45 | ||
| 44 | scope :resolved, where(:resolved => true) | 46 | scope :resolved, where(:resolved => true) |
| @@ -91,11 +93,12 @@ class Problem | @@ -91,11 +93,12 @@ class Problem | ||
| 91 | end | 93 | end |
| 92 | 94 | ||
| 93 | def unmerge! | 95 | def unmerge! |
| 96 | + attrs = {:error_class => error_class, :environment => environment} | ||
| 94 | problem_errs = errs.to_a | 97 | problem_errs = errs.to_a |
| 95 | problem_errs.shift | 98 | problem_errs.shift |
| 96 | [self] + problem_errs.map(&:id).map do |err_id| | 99 | [self] + problem_errs.map(&:id).map do |err_id| |
| 97 | err = Err.find(err_id) | 100 | err = Err.find(err_id) |
| 98 | - app.problems.create.tap do |new_problem| | 101 | + app.problems.create(attrs).tap do |new_problem| |
| 99 | err.update_attribute(:problem_id, new_problem.id) | 102 | err.update_attribute(:problem_id, new_problem.id) |
| 100 | new_problem.reset_cached_attributes | 103 | new_problem.reset_cached_attributes |
| 101 | end | 104 | end |
spec/fabricators/err_fabricator.rb
spec/fabricators/problem_fabricator.rb
| 1 | Fabricator(:problem) do | 1 | Fabricator(:problem) do |
| 2 | app! { Fabricate(:app) } | 2 | app! { Fabricate(:app) } |
| 3 | comments { [] } | 3 | comments { [] } |
| 4 | + error_class 'FooError' | ||
| 5 | + environment 'production' | ||
| 4 | end | 6 | end |
| 5 | 7 | ||
| 6 | Fabricator(:problem_with_comments, :from => :problem) do | 8 | Fabricator(:problem_with_comments, :from => :problem) do |
spec/interactors/problem_updater_cache_spec.rb
| @@ -27,8 +27,6 @@ describe ProblemUpdaterCache do | @@ -27,8 +27,6 @@ describe ProblemUpdaterCache do | ||
| 27 | 27 | ||
| 28 | it 'update information about this notice' do | 28 | it 'update information about this notice' do |
| 29 | expect(problem.message).to eq notice.message | 29 | expect(problem.message).to eq notice.message |
| 30 | - expect(problem.environment).to eq notice.environment_name | ||
| 31 | - expect(problem.error_class).to eq notice.error_class | ||
| 32 | expect(problem.where).to eq notice.where | 30 | expect(problem.where).to eq notice.where |
| 33 | end | 31 | end |
| 34 | 32 | ||
| @@ -68,8 +66,6 @@ describe ProblemUpdaterCache do | @@ -68,8 +66,6 @@ describe ProblemUpdaterCache do | ||
| 68 | end | 66 | end |
| 69 | it 'update information about this notice' do | 67 | it 'update information about this notice' do |
| 70 | expect(problem.message).to eq notice.message | 68 | expect(problem.message).to eq notice.message |
| 71 | - expect(problem.environment).to eq notice.environment_name | ||
| 72 | - expect(problem.error_class).to eq notice.error_class | ||
| 73 | expect(problem.where).to eq notice.where | 69 | expect(problem.where).to eq notice.where |
| 74 | end | 70 | end |
| 75 | 71 | ||
| @@ -118,8 +114,6 @@ describe ProblemUpdaterCache do | @@ -118,8 +114,6 @@ describe ProblemUpdaterCache do | ||
| 118 | 114 | ||
| 119 | it 'update information about this notice' do | 115 | it 'update information about this notice' do |
| 120 | expect(problem.message).to eq notice.message | 116 | expect(problem.message).to eq notice.message |
| 121 | - expect(problem.environment).to eq notice.environment_name | ||
| 122 | - expect(problem.error_class).to eq notice.error_class | ||
| 123 | expect(problem.where).to eq notice.where | 117 | expect(problem.where).to eq notice.where |
| 124 | end | 118 | end |
| 125 | 119 |
spec/models/app_spec.rb
| @@ -162,9 +162,8 @@ describe App do | @@ -162,9 +162,8 @@ describe App do | ||
| 162 | @app = Fabricate(:app) | 162 | @app = Fabricate(:app) |
| 163 | @conditions = { | 163 | @conditions = { |
| 164 | :error_class => 'Whoops', | 164 | :error_class => 'Whoops', |
| 165 | - :component => 'Foo', | ||
| 166 | - :action => 'bar', | ||
| 167 | - :environment => 'production' | 165 | + :environment => 'production', |
| 166 | + :fingerprint => 'some-finger-print' | ||
| 168 | } | 167 | } |
| 169 | end | 168 | end |
| 170 | 169 |
spec/models/err_spec.rb
| 1 | require 'spec_helper' | 1 | require 'spec_helper' |
| 2 | 2 | ||
| 3 | describe Err do | 3 | describe Err do |
| 4 | - it 'sets a default error_class and environment' do | ||
| 5 | - err = Err.new | ||
| 6 | - err.error_class.should == "UnknownError" | ||
| 7 | - err.environment.should == "unknown" | 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 | ||
| 8 | end | 17 | end |
| 9 | -end | ||
| 10 | 18 | ||
| 19 | +end |
spec/models/problem_spec.rb
| 1 | require 'spec_helper' | 1 | require 'spec_helper' |
| 2 | 2 | ||
| 3 | describe Problem do | 3 | describe Problem do |
| 4 | + | ||
| 5 | + context 'validations' do | ||
| 6 | + it 'requires a error_class' do | ||
| 7 | + err = Fabricate.build(:problem, :error_class => nil) | ||
| 8 | + err.should_not be_valid | ||
| 9 | + err.errors[:error_class].should include("can't be blank") | ||
| 10 | + end | ||
| 11 | + | ||
| 12 | + it 'requires an environment' do | ||
| 13 | + err = Fabricate.build(:problem, :environment => nil) | ||
| 14 | + err.should_not be_valid | ||
| 15 | + err.errors[:environment].should include("can't be blank") | ||
| 16 | + end | ||
| 17 | + end | ||
| 18 | + | ||
| 4 | describe "Fabrication" do | 19 | describe "Fabrication" do |
| 5 | context "Fabricate(:problem)" do | 20 | context "Fabricate(:problem)" do |
| 6 | it 'should have no comment' do | 21 | it 'should have no comment' do |