diff --git a/app/interactors/resolved_problem_clearer.rb b/app/interactors/resolved_problem_clearer.rb index f6945ce..a78ada5 100644 --- a/app/interactors/resolved_problem_clearer.rb +++ b/app/interactors/resolved_problem_clearer.rb @@ -27,6 +27,6 @@ class ResolvedProblemClearer end def repair_database - Mongoid.default_session.command :repairDatabase => 1 + Mongoid.default_client.command :repairDatabase => 1 end end diff --git a/app/models/app.rb b/app/models/app.rb index 338b9e3..0ff1203 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -58,7 +58,11 @@ class App Err.where( :fingerprint => attrs[:fingerprint] ).first || ( - problem = problems.create!(attrs.slice(:error_class, :environment)) + problem = problems.create!( + error_class: attrs[:error_class], + environment: attrs[:environment], + app_name: name + ) problem.errs.create!(attrs.slice(:fingerprint, :problem_id)) ) end @@ -180,7 +184,9 @@ class App protected def store_cached_attributes_on_problems - problems.each(&:cache_app_attributes) + Problem.where(:app_id => id).update_all( + app_name: name + ) end def generate_api_key diff --git a/app/models/deploy.rb b/app/models/deploy.rb index 9b89160..71ff354 100644 --- a/app/models/deploy.rb +++ b/app/models/deploy.rb @@ -33,7 +33,9 @@ class Deploy end def store_cached_attributes_on_problems - Problem.where(:app_id => app.id).each(&:cache_app_attributes) + Problem.where(:app_id => app.id).update_all( + last_deploy_at: created_at + ) end def deliver_email diff --git a/app/models/error_report.rb b/app/models/error_report.rb index bbfe1f6..9cd99a1 100644 --- a/app/models/error_report.rb +++ b/app/models/error_report.rb @@ -15,8 +15,16 @@ require 'hoptoad_notifier' # * :notifier - information to identify the source of the error report # class ErrorReport - attr_reader :error_class, :message, :request, :server_environment, :api_key, - :notifier, :user_attributes, :framework, :notice + attr_reader :api_key + attr_reader :error_class + attr_reader :framework + attr_reader :message + attr_reader :notice + attr_reader :notifier + attr_reader :problem + attr_reader :request + attr_reader :server_environment + attr_reader :user_attributes cattr_accessor :fingerprint_strategy do Fingerprint::Sha1 @@ -70,42 +78,16 @@ class ErrorReport # Update problem cache with information about this notice def cache_attributes_on_problem - # increment notice count - message_digest = Digest::MD5.hexdigest(@notice.message) - host_digest = Digest::MD5.hexdigest(@notice.host) - user_agent_digest = Digest::MD5.hexdigest(@notice.user_agent_string) - - @problem = Problem.where("_id" => @error.problem_id).find_one_and_update( - '$set' => { - 'app_name' => app.name, - 'environment' => @notice.environment_name, - 'error_class' => @notice.error_class, - 'last_notice_at' => @notice.created_at, - 'message' => @notice.message, - 'resolved' => false, - 'resolved_at' => nil, - 'where' => @notice.where, - "messages.#{message_digest}.value" => @notice.message, - "hosts.#{host_digest}.value" => @notice.host, - "user_agents.#{user_agent_digest}.value" => @notice.user_agent_string, - }, - '$inc' => { - 'notices_count' => 1, - "messages.#{message_digest}.count" => 1, - "hosts.#{host_digest}.count" => 1, - "user_agents.#{user_agent_digest}.count" => 1, - } - ) - end + @problem = Problem.cache_notice(@error.problem_id, @notice) - def similar_count - @similar_count ||= @problem.notices_count + # cache_notice returns the old problem, so the count is one higher + @similar_count = @problem.notices_count + 1 end # Send email notification if needed def email_notification return false unless app.emailable? - return false unless app.email_at_notices.include?(similar_count) + return false unless app.email_at_notices.include?(@similar_count) Mailer.err_notification(@notice).deliver rescue => e HoptoadNotifier.notify(e) @@ -113,7 +95,7 @@ class ErrorReport def should_notify? app.notification_service.notify_at_notices.include?(0) || - app.notification_service.notify_at_notices.include?(similar_count) + app.notification_service.notify_at_notices.include?(@similar_count) end # Launch all notification define on the app associate to this notice diff --git a/app/models/notice.rb b/app/models/notice.rb index 78e1ace..d901331 100644 --- a/app/models/notice.rb +++ b/app/models/notice.rb @@ -21,7 +21,7 @@ class Notice index(:err_id => 1, :created_at => 1, :_id => 1) before_save :sanitize - before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem + before_destroy :problem_recache validates_presence_of :backtrace_id, :server_environment, :notifier @@ -124,12 +124,8 @@ class Notice protected - def decrease_counter_cache - problem.inc(notices_count: -1) if err - end - - def remove_cached_attributes_from_problem - problem.remove_cached_notice_attributes(self) if err + def problem_recache + problem.uncache_notice(self) end def sanitize diff --git a/app/models/notification_service.rb b/app/models/notification_service.rb index 5fba0f4..95cbbbf 100644 --- a/app/models/notification_service.rb +++ b/app/models/notification_service.rb @@ -26,7 +26,7 @@ class NotificationService else Fields = [] end - + def notify_at_notices Errbit::Config.per_app_notify_at_notices ? super : Errbit::Config.notify_at_notices end diff --git a/app/models/problem.rb b/app/models/problem.rb index 455d895..26abaef 100644 --- a/app/models/problem.rb +++ b/app/models/problem.rb @@ -6,8 +6,15 @@ class Problem include Mongoid::Document include Mongoid::Timestamps - field :last_notice_at, :type => DateTime, :default => Proc.new { Time.now } - field :first_notice_at, :type => DateTime, :default => Proc.new { Time.now } + CACHED_NOTICE_ATTRIBUTES = { + messages: :message, + hosts: :host, + user_agents: :user_agent_string + }.freeze + + + field :last_notice_at, :type => ActiveSupport::TimeWithZone, :default => Proc.new { Time.now } + field :first_notice_at, :type => ActiveSupport::TimeWithZone, :default => Proc.new { Time.now } field :last_deploy_at, :type => Time field :resolved, :type => Boolean, :default => false field :resolved_at, :type => Time @@ -63,6 +70,83 @@ class Problem env.present? ? where(:environment => env) : scoped end + def self.cache_notice(id, notice) + # increment notice count + message_digest = Digest::MD5.hexdigest(notice.message) + host_digest = Digest::MD5.hexdigest(notice.host) + user_agent_digest = Digest::MD5.hexdigest(notice.user_agent_string) + + Problem.where('_id' => id).find_one_and_update( + '$set' => { + 'environment' => notice.environment_name, + 'error_class' => notice.error_class, + 'last_notice_at' => notice.created_at.utc, + 'message' => notice.message, + 'resolved' => false, + 'resolved_at' => nil, + 'where' => notice.where, + "messages.#{message_digest}.value" => notice.message, + "hosts.#{host_digest}.value" => notice.host, + "user_agents.#{user_agent_digest}.value" => notice.user_agent_string, + }, + '$inc' => { + 'notices_count' => 1, + "messages.#{message_digest}.count" => 1, + "hosts.#{host_digest}.count" => 1, + "user_agents.#{user_agent_digest}.count" => 1, + } + ) + end + + def uncache_notice(notice) + last_notice = notices.last + + atomically do |doc| + doc.set( + 'environment' => last_notice.environment_name, + 'error_class' => last_notice.error_class, + 'last_notice_at' => last_notice.created_at, + 'message' => last_notice.message, + 'where' => last_notice.where, + 'notices_count' => notices_count.to_i > 1 ? notices_count - 1 : 0 + ) + + CACHED_NOTICE_ATTRIBUTES.each do |k,v| + digest = Digest::MD5.hexdigest(notice.send(v)) + field = "#{k}.#{digest}" + + if (doc[k].try(:[], digest).try(:[], :count)).to_i > 1 + doc.inc("#{field}.count" => -1) + else + doc.unset(field) + end + end + end + end + + def recache + CACHED_NOTICE_ATTRIBUTES.each do |k,v| + # clear all cached attributes + send("#{k}=", {}) + + # find only notices related to this problem + Notice.collection.find.aggregate([ + { "$match" => { err_id: { "$in" => err_ids } } }, + { "$group" => { _id: "$#{v}", count: {"$sum" => 1} } } + ]).each do |agg| + next if agg[:_id] == nil + + send(k)[Digest::MD5.hexdigest(agg[:_id])] = { + value: agg[:_id], + count: agg[:count] + } + end + end + + self.notices_count = Notice.where({ err_id: { "$in" => err_ids }}).count + save + end + def url Rails.application.routes.url_helpers.app_problem_url(app, self, :host => Errbit::Config.host, @@ -98,16 +182,21 @@ class Problem def unmerge! attrs = {:error_class => error_class, :environment => environment} problem_errs = errs.to_a - problem_errs.shift - [self] + problem_errs.map(&:id).map do |err_id| - err = Err.find(err_id) - app.problems.create(attrs).tap do |new_problem| - err.update_attribute(:problem_id, new_problem.id) - new_problem.reset_cached_attributes - end + + # associate and return all the problems + new_problems = [self] + + # create new problems for each err that needs one + (problem_errs[1..-1] || []).each do |err| + new_problems << app.problems.create(attrs) + err.update_attribute(:problem, new_problems.last) end - end + # recache each new problem + new_problems.each(&:recache) + + new_problems + end def self.ordered_by(sort, order) case sort @@ -124,11 +213,6 @@ class Problem where(:first_notice_at.lte => date_range.end).where("$or" => [{:resolved_at => nil}, {:resolved_at.gte => date_range.begin}]) end - - def reset_cached_attributes - ProblemUpdaterCache.new(self).update - end - def cache_app_attributes if app self.app_name = app.name @@ -140,14 +224,6 @@ class Problem self.message = self.message[0, 1000] if self.message end - def remove_cached_notice_attributes(notice) - update_attributes!( - :messages => attribute_count_descrease(:messages, notice.message), - :hosts => attribute_count_descrease(:hosts, notice.host), - :user_agents => attribute_count_descrease(:user_agents, notice.user_agent_string) - ) - end - def issue_type # Return issue_type if configured, but fall back to detecting app's issue tracker attributes['issue_type'] ||= diff --git a/config/mongo.rb b/config/mongo.rb index a8310ec..94ec888 100644 --- a/config/mongo.rb +++ b/config/mongo.rb @@ -1,4 +1,7 @@ -Mongoid.logger.level = Logger.const_get Errbit::Config.log_level.upcase +log_level = Logger.const_get Errbit::Config.log_level.upcase + +Mongoid.logger.level = log_level +Mongo::Logger.level = log_level Mongoid.configure do |config| uri = if Errbit::Config.mongo_url == 'mongodb://localhost' diff --git a/spec/fabricators/err_fabricator.rb b/spec/fabricators/err_fabricator.rb index 14ad9f3..ef85786 100644 --- a/spec/fabricators/err_fabricator.rb +++ b/spec/fabricators/err_fabricator.rb @@ -10,6 +10,11 @@ Fabricator :notice do server_environment { {'environment-name' => 'production'} } request {{ 'component' => 'foo', 'action' => 'bar' }} notifier {{ 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }} + + after_create do + Problem.cache_notice(err.problem_id, self) + problem.reload + end end Fabricator :backtrace do diff --git a/spec/interactors/resolved_problem_clearer_spec.rb b/spec/interactors/resolved_problem_clearer_spec.rb index 5738b12..0e1a4a0 100644 --- a/spec/interactors/resolved_problem_clearer_spec.rb +++ b/spec/interactors/resolved_problem_clearer_spec.rb @@ -19,16 +19,16 @@ describe ResolvedProblemClearer do } end it 'not repair database' do - allow(Mongoid.default_session).to receive(:command).and_call_original - expect(Mongoid.default_session).to_not receive(:command).with({:repairDatabase => 1}) + allow(Mongoid.default_client).to receive(:command).and_call_original + expect(Mongoid.default_client).to_not receive(:command).with({:repairDatabase => 1}) resolved_problem_clearer.execute end end context "with problem resolve" do before do - allow(Mongoid.default_session).to receive(:command).and_call_original - allow(Mongoid.default_session).to receive(:command).with({:repairDatabase => 1}) + allow(Mongoid.default_client).to receive(:command).and_call_original + allow(Mongoid.default_client).to receive(:command).with({:repairDatabase => 1}) problems.first.resolve! problems.second.resolve! end @@ -44,7 +44,7 @@ describe ResolvedProblemClearer do end it 'repair database' do - expect(Mongoid.default_session).to receive(:command).with({:repairDatabase => 1}) + expect(Mongoid.default_client).to receive(:command).with({:repairDatabase => 1}) resolved_problem_clearer.execute end end diff --git a/spec/models/error_report_spec.rb b/spec/models/error_report_spec.rb index de4d243..3316111 100644 --- a/spec/models/error_report_spec.rb +++ b/spec/models/error_report_spec.rb @@ -17,259 +17,312 @@ module Airbrake end describe ErrorReport do - context "with notice without line of backtrace" do - let(:xml){ - Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read - } - - let(:error_report) { - ErrorReport.new(xml) - } - - let!(:app) { - Fabricate( - :app, - :api_key => 'APIKEY' - ) - } - - describe "#app" do - it 'find the good app' do - expect(error_report.app).to eq app - end + let(:xml){ + Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read + } + + let(:error_report) { + ErrorReport.new(xml) + } + + let!(:app) { + Fabricate( + :app, + :api_key => 'APIKEY' + ) + } + + describe "#app" do + it 'find the good app' do + expect(error_report.app).to eq app end + end - describe "#backtrace" do - it 'should have valid backtrace' do - expect(error_report.backtrace).to be_valid - end + describe "#backtrace" do + it 'should have valid backtrace' do + expect(error_report.backtrace).to be_valid end + end - describe "#fingerprint_strategy" do - it "should be possible to change how fingerprints are generated" do - def error_report.fingerprint_strategy - Class.new do - def self.generate(*args) - 'fingerprintzzz' - end + describe "#fingerprint_strategy" do + it "should be possible to change how fingerprints are generated" do + def error_report.fingerprint_strategy + Class.new do + def self.generate(*args) + 'fingerprintzzz' end end - - expect(error_report.error.fingerprint).to eq('fingerprintzzz') end + + expect(error_report.error.fingerprint).to eq('fingerprintzzz') end + end - describe "#generate_notice!" do - it "save a notice" do + describe "#generate_notice!" do + it "save a notice" do + expect { + error_report.generate_notice! + }.to change { + app.reload.problems.count + }.by(1) + end + + context "with notice generate by Airbrake gem" do + let(:xml) { Airbrake::Notice.new( + :exception => Exception.new, + :api_key => 'APIKEY', + :project_root => Rails.root + ).to_xml } + it 'save a notice' do expect { error_report.generate_notice! }.to change { app.reload.problems.count }.by(1) end + end - context "with notice generate by Airbrake gem" do - let(:xml) { Airbrake::Notice.new( - :exception => Exception.new, - :api_key => 'APIKEY', - :project_root => Rails.root - ).to_xml } - it 'save a notice' do - expect { - error_report.generate_notice! - }.to change { - app.reload.problems.count - }.by(1) - end - end + describe "notice create" do + before { error_report.generate_notice! } + subject { error_report.notice } + its(:message) { 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' } + its(:framework) { should == 'Rails: 3.2.11' } - describe "notice create" do - before { error_report.generate_notice! } - subject { error_report.notice } - its(:message) { 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' } - its(:framework) { should == 'Rails: 3.2.11' } + it 'has complete backtrace' do + expect(subject.backtrace_lines.size).to eq 73 + expect(subject.backtrace_lines.last['file']).to eq '[GEM_ROOT]/bin/rake' + end - it 'has complete backtrace' do - expect(subject.backtrace_lines.size).to eq 73 - expect(subject.backtrace_lines.last['file']).to eq '[GEM_ROOT]/bin/rake' - end - it 'has server_environement' do - expect(subject.server_environment['environment-name']).to eq 'development' - end + it 'has server_environement' do + expect(subject.server_environment['environment-name']).to eq 'development' + end - it 'has request' do - expect(subject.request['url']).to eq 'http://example.org/verify/cupcake=fistfight&lovebird=doomsayer' - expect(subject.request['params']['controller']).to eq 'application' - end + it 'has request' do + expect(subject.request['url']).to eq 'http://example.org/verify/cupcake=fistfight&lovebird=doomsayer' + expect(subject.request['params']['controller']).to eq 'application' + end - it 'has notifier' do - expect(subject.notifier['name']).to eq 'Hoptoad Notifier' - end + it 'has notifier' do + expect(subject.notifier['name']).to eq 'Hoptoad Notifier' + end - it 'get user_attributes' do - expect(subject.user_attributes['id']).to eq '123' - expect(subject.user_attributes['name']).to eq 'Mr. Bean' - expect(subject.user_attributes['email']).to eq 'mr.bean@example.com' - expect(subject.user_attributes['username']).to eq 'mrbean' - end + it 'get user_attributes' do + expect(subject.user_attributes['id']).to eq '123' + expect(subject.user_attributes['name']).to eq 'Mr. Bean' + expect(subject.user_attributes['email']).to eq 'mr.bean@example.com' + expect(subject.user_attributes['username']).to eq 'mrbean' + end - it 'valid env_vars' do - # XML: - expect(subject.env_vars).to have_key('SCRIPT_NAME') - expect(subject.env_vars['SCRIPT_NAME']).to be_nil # blank ends up nil - - # XML representation: - # - # false - # true - # / - # - # - # - # - expected = { - 'secure' => 'false', - 'httponly' => 'true', - 'path' => '/', - 'expire_after' => nil, - 'domain' => nil, - 'id' => nil - } - expect(subject.env_vars).to have_key('rack_session_options') - expect(subject.env_vars['rack_session_options']).to eql(expected) - end + it 'valid env_vars' do + # XML: + expect(subject.env_vars).to have_key('SCRIPT_NAME') + expect(subject.env_vars['SCRIPT_NAME']).to be_nil # blank ends up nil + + # XML representation: + # + # false + # true + # / + # + # + # + # + expected = { + 'secure' => 'false', + 'httponly' => 'true', + 'path' => '/', + 'expire_after' => nil, + 'domain' => nil, + 'id' => nil + } + expect(subject.env_vars).to have_key('rack_session_options') + expect(subject.env_vars['rack_session_options']).to eql(expected) end end + end - it 'save a notice assignes to err' do + describe '#cache_attributes_on_problem' do + it 'sets the latest notice properties on the problem' do error_report.generate_notice! - expect(error_report.notice.err).to be_a(Err) + problem = error_report.problem.reload + notice = error_report.notice.reload + + expect(problem.environment).to eq('development') + expect(problem.error_class).to eq('HoptoadTestingException') + expect(problem.last_notice_at).to eq(notice.created_at) + expect(problem.message).to eq(notice.message) + expect(problem.where).to eq(notice.where) end - it 'memoize the notice' do - expect { - error_report.generate_notice! - error_report.generate_notice! - }.to change { - Notice.count - }.by(1) + it 'unresolves the problem' do + error_report.generate_notice! + problem = error_report.problem + problem.update( + resolved_at: Time.now, + resolved: true + ) + + error_report = ErrorReport.new(xml) + error_report.generate_notice! + problem.reload + + expect(problem.resolved_at).to be(nil) + expect(problem.resolved).to be(false) end - it 'find the correct err for the notice' do - err = Fabricate(:err, :problem => Fabricate(:problem, :resolved => true)) + it 'caches notice counts' do + error_report.generate_notice! + problem = error_report.problem + problem.reload + + expect(problem.notices_count).to be(1) + expect(problem.user_agents['382b0f5185773fa0f67a8ed8056c7759']['count']).to be(1) + expect(problem.messages['9449f087eee0499e2d9029ae3dacaf53']['count']).to be(1) + expect(problem.hosts['1bdf72e04d6b50c82a48c7e4dd38cc69']['count']).to be(1) + end - allow(error_report).to receive(:fingerprint).and_return(err.fingerprint) + it 'increments notice counts' do + error_report.generate_notice! + error_report = ErrorReport.new(xml) + error_report.generate_notice! + problem = error_report.problem + problem.reload - expect { - error_report.generate_notice! - }.to change { - error_report.error.resolved? - }.from(true).to(false) + expect(problem.notices_count).to be(2) + expect(problem.user_agents['382b0f5185773fa0f67a8ed8056c7759']['count']).to be(2) + expect(problem.messages['9449f087eee0499e2d9029ae3dacaf53']['count']).to be(2) + expect(problem.hosts['1bdf72e04d6b50c82a48c7e4dd38cc69']['count']).to be(2) end + end - context "with notification service configured" do - before do - app.notify_on_errs = true - app.watchers.build(:email => 'foo@example.com') - app.save - end - it 'send email' do - notice = error_report.generate_notice! - email = ActionMailer::Base.deliveries.last - expect(email.to).to include(app.watchers.first.email) - expect(email.subject).to include(notice.message.truncate(50)) - expect(email.subject).to include("[#{app.name}]") - expect(email.subject).to include("[#{notice.environment_name}]") - end + it 'save a notice assignes to err' do + error_report.generate_notice! + expect(error_report.notice.err).to be_a(Err) + end - context "with xml without request section" do - let(:xml){ - Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read - } - it "save a notice" do - expect { - error_report.generate_notice! - }.to change { - app.reload.problems.count - }.by(1) - end + it 'memoize the notice' do + expect { + error_report.generate_notice! + error_report.generate_notice! + }.to change { + Notice.count + }.by(1) + end + + it 'find the correct err for the notice' do + err = Fabricate(:err, :problem => Fabricate(:problem, :resolved => true)) + + allow(error_report).to receive(:fingerprint).and_return(err.fingerprint) + + expect { + error_report.generate_notice! + }.to change { + error_report.error.resolved? + }.from(true).to(false) + end + + context "with notification service configured" do + before do + app.notify_on_errs = true + app.watchers.build(:email => 'foo@example.com') + app.save + end + it 'send email' do + notice = error_report.generate_notice! + email = ActionMailer::Base.deliveries.last + expect(email.to).to include(app.watchers.first.email) + expect(email.subject).to include(notice.message.truncate(50)) + expect(email.subject).to include("[#{app.name}]") + expect(email.subject).to include("[#{notice.environment_name}]") + end + + context "with xml without request section" do + let(:xml){ + Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read + } + it "save a notice" do + expect { + error_report.generate_notice! + }.to change { + app.reload.problems.count + }.by(1) end + end - context "with xml with only a single line of backtrace" do - let(:xml){ - Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read - } - it "save a notice" do - expect { - error_report.generate_notice! - }.to change { - app.reload.problems.count - }.by(1) - end + context "with xml with only a single line of backtrace" do + let(:xml){ + Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read + } + it "save a notice" do + expect { + error_report.generate_notice! + }.to change { + app.reload.problems.count + }.by(1) end end + end - describe "#valid?" do - context "with valid error report" do - it "return true" do - expect(error_report.valid?).to be true - end + describe "#valid?" do + context "with valid error report" do + it "return true" do + expect(error_report.valid?).to be true end - context "with not valid api_key" do - before do - App.where(:api_key => app.api_key).delete_all - end - it "return false" do - expect(error_report.valid?).to be false - end + end + context "with not valid api_key" do + before do + App.where(:api_key => app.api_key).delete_all + end + it "return false" do + expect(error_report.valid?).to be false end end + end - describe "#notice" do - context "before generate_notice!" do - it 'return nil' do - expect(error_report.notice).to be nil - end + describe "#notice" do + context "before generate_notice!" do + it 'return nil' do + expect(error_report.notice).to be nil end + end - context "after generate_notice!" do - before do - error_report.generate_notice! - end - - it 'return the notice' do - expect(error_report.notice).to be_a Notice - end + context "after generate_notice!" do + before do + error_report.generate_notice! + end + it 'return the notice' do + expect(error_report.notice).to be_a Notice end + end + end - describe "#should_keep?" do - context "with current app version not set" do - before do - error_report.app.current_app_version = nil - error_report.server_environment['app-version'] = '1.0' - end + describe "#should_keep?" do + context "with current app version not set" do + before do + error_report.app.current_app_version = nil + error_report.server_environment['app-version'] = '1.0' + end - it "return true" do - expect(error_report.should_keep?).to be true - end + it "return true" do + expect(error_report.should_keep?).to be true end + end - context "with current app version set" do - before do - error_report.app.current_app_version = '1.0' - end + context "with current app version set" do + before do + error_report.app.current_app_version = '1.0' + end - it "return true if current or newer" do - error_report.server_environment['app-version'] = '1.0' - expect(error_report.should_keep?).to be true - end + it "return true if current or newer" do + error_report.server_environment['app-version'] = '1.0' + expect(error_report.should_keep?).to be true + end - it "return false if older" do - error_report.server_environment['app-version'] = '0.9' - expect(error_report.should_keep?).to be false - end + it "return false if older" do + error_report.server_environment['app-version'] = '0.9' + expect(error_report.should_keep?).to be false end end end diff --git a/spec/models/notice_observer_spec.rb b/spec/models/notice_observer_spec.rb index 76b6ae6..198c1b9 100644 --- a/spec/models/notice_observer_spec.rb +++ b/spec/models/notice_observer_spec.rb @@ -1,172 +1,197 @@ describe "Callback on Notice", type: 'model' do - describe "email notifications (configured individually for each app)" do + let(:notice_attrs_for) do + ->(api_key) do + { + error_class: "HoptoadTestingException", + message: "some message", + backtrace: [ + { + "number"=>"425", + "file"=>"[GEM_ROOT]/callbacks.rb", + "method"=>"__callbacks" + } + ], + request: { "component" => "application" }, + server_environment: { + "project-root" => "/path/to/sample/project", + "environment-name" => "development" + }, + api_key: api_key, + notifier: { + "name"=>"Hoptoad Notifier", + "version"=>"2.3.2", + "url"=>"http://hoptoadapp.com" + }, + framework: "Rails: 3.2.11" + } + end + end + + describe 'email notifications (configured individually for each app)' do + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } custom_thresholds = [2, 4, 8, 16, 32, 64] + let(:app) do + Fabricate(:app_with_watcher, email_at_notices: custom_thresholds) + end before do Errbit::Config.per_app_email_at_notices = true - @app = Fabricate(:app_with_watcher, :email_at_notices => custom_thresholds) - @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app)) + error_report = ErrorReport.new(notice_attrs) + error_report.generate_notice! + @problem = error_report.notice.err.problem end - after do - Errbit::Config.per_app_email_at_notices = false - end + after { Errbit::Config.per_app_email_at_notices = false } custom_thresholds.each do |threshold| it "sends an email notification after #{threshold} notice(s)" do - allow(@err.problem).to receive(:notices_count).and_return(threshold) + # set to just before the threshold + @problem.update_attributes notices_count: threshold - 1 + expect(Mailer).to receive(:err_notification). and_return(double('email', :deliver => true)) - Fabricate(:notice, :err => @err) + + error_report = ErrorReport.new(notice_attrs) + error_report.generate_notice! end end - end - describe "email notifications for a resolved issue" do - before do - Errbit::Config.per_app_email_at_notices = true - @app = Fabricate(:app_with_watcher, :email_at_notices => [1]) - @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app, :notices_count => 100)) - end + it "doesn't email after 5 notices" do + @problem.update_attributes notices_count: 5 - after do - Errbit::Config.per_app_email_at_notices = false - end + expect(Mailer).to_not receive(:err_notification) - it "should send email notification after 1 notice since an error has been resolved" do - @err.problem.resolve! - expect(Mailer).to receive(:err_notification). - and_return(double('email', :deliver => true)) - Fabricate(:notice, :err => @err) + error_report = ErrorReport.new(notice_attrs) + error_report.generate_notice! end - it 'self notify if mailer failed' do - @err.problem.resolve! - expect(Mailer).to receive(:err_notification). - and_raise(ArgumentError) + + it 'notify self if mailer fails' do + expect(Mailer).to receive(:err_notification).and_raise(ArgumentError) expect(HoptoadNotifier).to receive(:notify) - Fabricate(:notice, :err => @err) + ErrorReport.new(notice_attrs).generate_notice! end end - describe "should send a notification if a notification service is configured with defaults" 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 + describe 'email notifications for resolved issues' do + let(:notification_service) { Fabricate(:campfire_notification_service) } + let(:app) do + Fabricate( + :app_with_watcher, + notify_on_errs: true, + email_at_notices: [1,100] + ) end + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } - after do - Errbit::Config.per_app_email_at_notices = false - end + before { Errbit::Config.per_app_email_at_notices = true } + after { Errbit::Config.per_app_email_at_notices = false } - it "should create a campfire notification" do - expect(app.notification_service).to receive(:create_notification) + it 'sends email the first time after the error is resolved' do + error_report = ErrorReport.new(notice_attrs) + error_report.generate_notice! + err = error_report.notice.err - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) - end - end + err.problem.update_attributes notices_count: 99 + err.problem.resolve! - describe "send a notification if a notification service is configured with defaults but failed" do - let(:app) { Fabricate(:app_with_watcher, - :notify_on_errs => true, - :email_at_notices => [1, 100], :notification_service => Fabricate(:campfire_notification_service))} - let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 99)) } - let(:backtrace) { Fabricate(:backtrace) } + expect(Mailer).to receive(:err_notification) + .and_return(double('email', :deliver => true)) - before do - Errbit::Config.per_app_email_at_notices = true + ErrorReport.new(notice_attrs).generate_notice! end + end - after do - Errbit::Config.per_app_email_at_notices = false + describe 'send email when notification service is configured but fails' do + let(:notification_service) {Fabricate(:campfire_notification_service)} + let(:app) do + Fabricate( + :app_with_watcher, + notify_on_errs: true, + notification_service: notification_service + ) end + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } + + before { Errbit::Config.per_app_notify_at_notices = true } + after { Errbit::Config.per_app_notify_at_notices = false } + + it 'sends email' do + error_report = ErrorReport.new(notice_attrs) - it "send email" do - expect(app.notification_service).to receive(:create_notification).and_raise(ArgumentError) - expect(Mailer).to receive(:err_notification).and_return(double(:deliver => true)) + expect(error_report.app.notification_service) + .to receive(:create_notification).and_raise(ArgumentError) + expect(Mailer) + .to receive(:err_notification).and_return(double(:deliver => true)) - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) + error_report.generate_notice! 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) } + describe 'should not send a notification if a notification service is not' \ + 'configured' do - before do - Errbit::Config.per_app_email_at_notices = true - end + let(:notification_service) { Fabricate(:notification_service) } + let(:app) { Fabricate(:app, notification_service: notification_service )} + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } - after do - Errbit::Config.per_app_email_at_notices = false - end + before { Errbit::Config.per_app_notify_at_notices = true } + after { Errbit::Config.per_app_notify_at_notices = false } it "should not create a campfire notification" do - expect(app.notification_service).to_not receive(:create_notification) - - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) + error_report = ErrorReport.new(notice_attrs) + expect(error_report.app.notification_service).to_not receive(:create_notification) + error_report.generate_notice! end end describe 'hipcat notifications' do - let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:hipchat_notification_service))} - let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } + let(:notification_service) { Fabricate(:hipchat_notification_service) } + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } + let(:app) { Fabricate(:app, notification_service: notification_service) } - before do - Errbit::Config.per_app_email_at_notices = true - end - - after do - Errbit::Config.per_app_email_at_notices = false - end + before { Errbit::Config.per_app_notify_at_notices = true } + after { Errbit::Config.per_app_notify_at_notices = false } it 'creates a hipchat notification' do - expect(app.notification_service).to receive(:create_notification) - - Fabricate(:notice, :err => err) + error_report = ErrorReport.new(notice_attrs) + expect(error_report.app.notification_service) + .to receive(:create_notification) + error_report.generate_notice! end end describe "should send a notification at desired intervals" do - let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:campfire_notification_service, :notify_at_notices => [1,2]))} - let(:backtrace) { Fabricate(:backtrace) } - - before do - Errbit::Config.per_app_email_at_notices = true + let(:notification_service) do + Fabricate(:campfire_notification_service, notify_at_notices: [1,2]) end + let(:app) { Fabricate(:app, notification_service: notification_service) } + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } - after do - Errbit::Config.per_app_email_at_notices = false - end + before { Errbit::Config.per_app_notify_at_notices = true } + after { Errbit::Config.per_app_notify_at_notices = false } it "should create a campfire notification on first notice" do - err = Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 1)) - expect(app.notification_service).to receive(:create_notification) - - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) + error_report = ErrorReport.new(notice_attrs) + expect(error_report.app.notification_service) + .to receive(:create_notification) + error_report.generate_notice! # one end it "should create a campfire notification on second notice" do - err = Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 1)) - expect(app.notification_service).to receive(:create_notification) - - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) + ErrorReport.new(notice_attrs).generate_notice! # one + error_report = ErrorReport.new(notice_attrs) + expect(error_report.app.notification_service) + .to receive(:create_notification) + error_report.generate_notice! # two end it "should not create a campfire notification on third notice" do - err = Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 1)) - expect(app.notification_service).to receive(:create_notification) - - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) + ErrorReport.new(notice_attrs).generate_notice! # one + ErrorReport.new(notice_attrs).generate_notice! # two + error_report = ErrorReport.new(notice_attrs) + expect(error_report.app.notification_service) + .to_not receive(:create_notification) + error_report.generate_notice! # three end end end diff --git a/spec/models/notice_spec.rb b/spec/models/notice_spec.rb index ead86f5..2353024 100644 --- a/spec/models/notice_spec.rb +++ b/spec/models/notice_spec.rb @@ -3,7 +3,7 @@ describe Notice, type: 'model' do it 'requires a backtrace' do notice = Fabricate.build(:notice, :backtrace => nil) expect(notice).to_not be_valid - expect(notice.errors[:backtrace]).to include("can't be blank") + expect(notice.errors[:backtrace_id]).to include("can't be blank") end it 'requires the server_environment' do diff --git a/spec/models/problem_spec.rb b/spec/models/problem_spec.rb index dd092e5..b05ebaa 100644 --- a/spec/models/problem_spec.rb +++ b/spec/models/problem_spec.rb @@ -40,10 +40,10 @@ describe Problem, type: 'model' do expect(problem).to_not be_nil notice1 = Fabricate(:notice, :err => err) - expect(problem.last_notice_at).to eq notice1.created_at + expect(problem.last_notice_at).to eq notice1.reload.created_at notice2 = Fabricate(:notice, :err => err) - expect(problem.last_notice_at).to eq notice2.created_at + expect(problem.last_notice_at).to eq notice2.reload.created_at end end @@ -266,12 +266,6 @@ describe Problem, type: 'model' do expect(@problem.messages).to eq ({}) end - it "adding a notice adds a string to #messages" do - expect { - Fabricate(:notice, :err => @err, :message => 'ERR 1') - }.to change(@problem, :messages).from({}).to({Digest::MD5.hexdigest('ERR 1') => {'value' => 'ERR 1', 'count' => 1}}) - end - it "removing a notice removes string from #messages" do Fabricate(:notice, :err => @err, :message => 'ERR 1') expect { @@ -299,12 +293,6 @@ describe Problem, type: 'model' do expect(@problem.hosts).to eq ({}) end - it "adding a notice adds a string to #hosts" do - expect { - Fabricate(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) - }.to change(@problem, :hosts).from({}).to({Digest::MD5.hexdigest('example.com') => {'value' => 'example.com', 'count' => 1}}) - end - it "removing a notice removes string from #hosts" do Fabricate(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) expect { @@ -325,12 +313,6 @@ describe Problem, type: 'model' do expect(@problem.user_agents).to eq ({}) end - it "adding a notice adds a string to #user_agents" do - expect { - Fabricate(:notice, :err => @err, :request => {'cgi-data' => {'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16'}}) - }.to change(@problem, :user_agents).from({}).to({Digest::MD5.hexdigest('Chrome 10.0.648.204 (OS X 10.6.7)') => {'value' => 'Chrome 10.0.648.204 (OS X 10.6.7)', 'count' => 1}}) - end - it "removing a notice removes string from #user_agents" do Fabricate(:notice, :err => @err, :request => {'cgi-data' => {'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.204 Safari/534.16'}}) expect { diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6a3e283..520cfbf 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ # This file is copied to ~/spec when you run 'ruby script/generate rspec' # from the project root directory. -ENV["RAILS_ENV"] ||= 'test' +ENV["RAILS_ENV"] = 'test' +ENV["ERRBIT_LOG_LEVEL"] = 'error' if ENV['COVERAGE'] require 'coveralls' @@ -21,7 +22,6 @@ require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' require 'rspec/its' require 'email_spec' -require 'database_cleaner' require 'xmpp4r' require 'xmpp4r/muc' require 'mongoid-rspec' -- libgit2 0.21.2