Commit 8ef1dc2be3b538c8870aeef0fe18347d76d527d2
1 parent
912600bb
Exists in
master
and in
1 other branch
refactor and reorganize after mongoid5 upgrade
Showing
15 changed files
with
533 additions
and
403 deletions
Show diff stats
app/interactors/resolved_problem_clearer.rb
@@ -27,6 +27,6 @@ class ResolvedProblemClearer | @@ -27,6 +27,6 @@ class ResolvedProblemClearer | ||
27 | end | 27 | end |
28 | 28 | ||
29 | def repair_database | 29 | def repair_database |
30 | - Mongoid.default_session.command :repairDatabase => 1 | 30 | + Mongoid.default_client.command :repairDatabase => 1 |
31 | end | 31 | end |
32 | end | 32 | end |
app/models/app.rb
@@ -58,7 +58,11 @@ class App | @@ -58,7 +58,11 @@ class App | ||
58 | Err.where( | 58 | Err.where( |
59 | :fingerprint => attrs[:fingerprint] | 59 | :fingerprint => attrs[:fingerprint] |
60 | ).first || ( | 60 | ).first || ( |
61 | - problem = problems.create!(attrs.slice(:error_class, :environment)) | 61 | + problem = problems.create!( |
62 | + error_class: attrs[:error_class], | ||
63 | + environment: attrs[:environment], | ||
64 | + app_name: name | ||
65 | + ) | ||
62 | problem.errs.create!(attrs.slice(:fingerprint, :problem_id)) | 66 | problem.errs.create!(attrs.slice(:fingerprint, :problem_id)) |
63 | ) | 67 | ) |
64 | end | 68 | end |
@@ -180,7 +184,9 @@ class App | @@ -180,7 +184,9 @@ class App | ||
180 | protected | 184 | protected |
181 | 185 | ||
182 | def store_cached_attributes_on_problems | 186 | def store_cached_attributes_on_problems |
183 | - problems.each(&:cache_app_attributes) | 187 | + Problem.where(:app_id => id).update_all( |
188 | + app_name: name | ||
189 | + ) | ||
184 | end | 190 | end |
185 | 191 | ||
186 | def generate_api_key | 192 | def generate_api_key |
app/models/deploy.rb
@@ -33,7 +33,9 @@ class Deploy | @@ -33,7 +33,9 @@ class Deploy | ||
33 | end | 33 | end |
34 | 34 | ||
35 | def store_cached_attributes_on_problems | 35 | def store_cached_attributes_on_problems |
36 | - Problem.where(:app_id => app.id).each(&:cache_app_attributes) | 36 | + Problem.where(:app_id => app.id).update_all( |
37 | + last_deploy_at: created_at | ||
38 | + ) | ||
37 | end | 39 | end |
38 | 40 | ||
39 | def deliver_email | 41 | def deliver_email |
app/models/error_report.rb
@@ -15,8 +15,16 @@ require 'hoptoad_notifier' | @@ -15,8 +15,16 @@ require 'hoptoad_notifier' | ||
15 | # * <tt>:notifier</tt> - information to identify the source of the error report | 15 | # * <tt>:notifier</tt> - information to identify the source of the error report |
16 | # | 16 | # |
17 | class ErrorReport | 17 | class ErrorReport |
18 | - attr_reader :error_class, :message, :request, :server_environment, :api_key, | ||
19 | - :notifier, :user_attributes, :framework, :notice | 18 | + attr_reader :api_key |
19 | + attr_reader :error_class | ||
20 | + attr_reader :framework | ||
21 | + attr_reader :message | ||
22 | + attr_reader :notice | ||
23 | + attr_reader :notifier | ||
24 | + attr_reader :problem | ||
25 | + attr_reader :request | ||
26 | + attr_reader :server_environment | ||
27 | + attr_reader :user_attributes | ||
20 | 28 | ||
21 | cattr_accessor :fingerprint_strategy do | 29 | cattr_accessor :fingerprint_strategy do |
22 | Fingerprint::Sha1 | 30 | Fingerprint::Sha1 |
@@ -70,42 +78,16 @@ class ErrorReport | @@ -70,42 +78,16 @@ class ErrorReport | ||
70 | 78 | ||
71 | # Update problem cache with information about this notice | 79 | # Update problem cache with information about this notice |
72 | def cache_attributes_on_problem | 80 | def cache_attributes_on_problem |
73 | - # increment notice count | ||
74 | - message_digest = Digest::MD5.hexdigest(@notice.message) | ||
75 | - host_digest = Digest::MD5.hexdigest(@notice.host) | ||
76 | - user_agent_digest = Digest::MD5.hexdigest(@notice.user_agent_string) | ||
77 | - | ||
78 | - @problem = Problem.where("_id" => @error.problem_id).find_one_and_update( | ||
79 | - '$set' => { | ||
80 | - 'app_name' => app.name, | ||
81 | - 'environment' => @notice.environment_name, | ||
82 | - 'error_class' => @notice.error_class, | ||
83 | - 'last_notice_at' => @notice.created_at, | ||
84 | - 'message' => @notice.message, | ||
85 | - 'resolved' => false, | ||
86 | - 'resolved_at' => nil, | ||
87 | - 'where' => @notice.where, | ||
88 | - "messages.#{message_digest}.value" => @notice.message, | ||
89 | - "hosts.#{host_digest}.value" => @notice.host, | ||
90 | - "user_agents.#{user_agent_digest}.value" => @notice.user_agent_string, | ||
91 | - }, | ||
92 | - '$inc' => { | ||
93 | - 'notices_count' => 1, | ||
94 | - "messages.#{message_digest}.count" => 1, | ||
95 | - "hosts.#{host_digest}.count" => 1, | ||
96 | - "user_agents.#{user_agent_digest}.count" => 1, | ||
97 | - } | ||
98 | - ) | ||
99 | - end | 81 | + @problem = Problem.cache_notice(@error.problem_id, @notice) |
100 | 82 | ||
101 | - def similar_count | ||
102 | - @similar_count ||= @problem.notices_count | 83 | + # cache_notice returns the old problem, so the count is one higher |
84 | + @similar_count = @problem.notices_count + 1 | ||
103 | end | 85 | end |
104 | 86 | ||
105 | # Send email notification if needed | 87 | # Send email notification if needed |
106 | def email_notification | 88 | def email_notification |
107 | return false unless app.emailable? | 89 | return false unless app.emailable? |
108 | - return false unless app.email_at_notices.include?(similar_count) | 90 | + return false unless app.email_at_notices.include?(@similar_count) |
109 | Mailer.err_notification(@notice).deliver | 91 | Mailer.err_notification(@notice).deliver |
110 | rescue => e | 92 | rescue => e |
111 | HoptoadNotifier.notify(e) | 93 | HoptoadNotifier.notify(e) |
@@ -113,7 +95,7 @@ class ErrorReport | @@ -113,7 +95,7 @@ class ErrorReport | ||
113 | 95 | ||
114 | def should_notify? | 96 | def should_notify? |
115 | app.notification_service.notify_at_notices.include?(0) || | 97 | app.notification_service.notify_at_notices.include?(0) || |
116 | - app.notification_service.notify_at_notices.include?(similar_count) | 98 | + app.notification_service.notify_at_notices.include?(@similar_count) |
117 | end | 99 | end |
118 | 100 | ||
119 | # Launch all notification define on the app associate to this notice | 101 | # Launch all notification define on the app associate to this notice |
app/models/notice.rb
@@ -21,7 +21,7 @@ class Notice | @@ -21,7 +21,7 @@ class Notice | ||
21 | index(:err_id => 1, :created_at => 1, :_id => 1) | 21 | index(:err_id => 1, :created_at => 1, :_id => 1) |
22 | 22 | ||
23 | before_save :sanitize | 23 | before_save :sanitize |
24 | - before_destroy :decrease_counter_cache, :remove_cached_attributes_from_problem | 24 | + before_destroy :problem_recache |
25 | 25 | ||
26 | validates_presence_of :backtrace_id, :server_environment, :notifier | 26 | validates_presence_of :backtrace_id, :server_environment, :notifier |
27 | 27 | ||
@@ -124,12 +124,8 @@ class Notice | @@ -124,12 +124,8 @@ class Notice | ||
124 | 124 | ||
125 | protected | 125 | protected |
126 | 126 | ||
127 | - def decrease_counter_cache | ||
128 | - problem.inc(notices_count: -1) if err | ||
129 | - end | ||
130 | - | ||
131 | - def remove_cached_attributes_from_problem | ||
132 | - problem.remove_cached_notice_attributes(self) if err | 127 | + def problem_recache |
128 | + problem.uncache_notice(self) | ||
133 | end | 129 | end |
134 | 130 | ||
135 | def sanitize | 131 | def sanitize |
app/models/notification_service.rb
@@ -26,7 +26,7 @@ class NotificationService | @@ -26,7 +26,7 @@ class NotificationService | ||
26 | else | 26 | else |
27 | Fields = [] | 27 | Fields = [] |
28 | end | 28 | end |
29 | - | 29 | + |
30 | def notify_at_notices | 30 | def notify_at_notices |
31 | Errbit::Config.per_app_notify_at_notices ? super : Errbit::Config.notify_at_notices | 31 | Errbit::Config.per_app_notify_at_notices ? super : Errbit::Config.notify_at_notices |
32 | end | 32 | end |
app/models/problem.rb
@@ -6,8 +6,15 @@ class Problem | @@ -6,8 +6,15 @@ class Problem | ||
6 | include Mongoid::Document | 6 | include Mongoid::Document |
7 | include Mongoid::Timestamps | 7 | include Mongoid::Timestamps |
8 | 8 | ||
9 | - field :last_notice_at, :type => DateTime, :default => Proc.new { Time.now } | ||
10 | - field :first_notice_at, :type => DateTime, :default => Proc.new { Time.now } | 9 | + CACHED_NOTICE_ATTRIBUTES = { |
10 | + messages: :message, | ||
11 | + hosts: :host, | ||
12 | + user_agents: :user_agent_string | ||
13 | + }.freeze | ||
14 | + | ||
15 | + | ||
16 | + field :last_notice_at, :type => ActiveSupport::TimeWithZone, :default => Proc.new { Time.now } | ||
17 | + field :first_notice_at, :type => ActiveSupport::TimeWithZone, :default => Proc.new { Time.now } | ||
11 | field :last_deploy_at, :type => Time | 18 | field :last_deploy_at, :type => Time |
12 | field :resolved, :type => Boolean, :default => false | 19 | field :resolved, :type => Boolean, :default => false |
13 | field :resolved_at, :type => Time | 20 | field :resolved_at, :type => Time |
@@ -63,6 +70,83 @@ class Problem | @@ -63,6 +70,83 @@ class Problem | ||
63 | env.present? ? where(:environment => env) : scoped | 70 | env.present? ? where(:environment => env) : scoped |
64 | end | 71 | end |
65 | 72 | ||
73 | + def self.cache_notice(id, notice) | ||
74 | + # increment notice count | ||
75 | + message_digest = Digest::MD5.hexdigest(notice.message) | ||
76 | + host_digest = Digest::MD5.hexdigest(notice.host) | ||
77 | + user_agent_digest = Digest::MD5.hexdigest(notice.user_agent_string) | ||
78 | + | ||
79 | + Problem.where('_id' => id).find_one_and_update( | ||
80 | + '$set' => { | ||
81 | + 'environment' => notice.environment_name, | ||
82 | + 'error_class' => notice.error_class, | ||
83 | + 'last_notice_at' => notice.created_at.utc, | ||
84 | + 'message' => notice.message, | ||
85 | + 'resolved' => false, | ||
86 | + 'resolved_at' => nil, | ||
87 | + 'where' => notice.where, | ||
88 | + "messages.#{message_digest}.value" => notice.message, | ||
89 | + "hosts.#{host_digest}.value" => notice.host, | ||
90 | + "user_agents.#{user_agent_digest}.value" => notice.user_agent_string, | ||
91 | + }, | ||
92 | + '$inc' => { | ||
93 | + 'notices_count' => 1, | ||
94 | + "messages.#{message_digest}.count" => 1, | ||
95 | + "hosts.#{host_digest}.count" => 1, | ||
96 | + "user_agents.#{user_agent_digest}.count" => 1, | ||
97 | + } | ||
98 | + ) | ||
99 | + end | ||
100 | + | ||
101 | + def uncache_notice(notice) | ||
102 | + last_notice = notices.last | ||
103 | + | ||
104 | + atomically do |doc| | ||
105 | + doc.set( | ||
106 | + 'environment' => last_notice.environment_name, | ||
107 | + 'error_class' => last_notice.error_class, | ||
108 | + 'last_notice_at' => last_notice.created_at, | ||
109 | + 'message' => last_notice.message, | ||
110 | + 'where' => last_notice.where, | ||
111 | + 'notices_count' => notices_count.to_i > 1 ? notices_count - 1 : 0 | ||
112 | + ) | ||
113 | + | ||
114 | + CACHED_NOTICE_ATTRIBUTES.each do |k,v| | ||
115 | + digest = Digest::MD5.hexdigest(notice.send(v)) | ||
116 | + field = "#{k}.#{digest}" | ||
117 | + | ||
118 | + if (doc[k].try(:[], digest).try(:[], :count)).to_i > 1 | ||
119 | + doc.inc("#{field}.count" => -1) | ||
120 | + else | ||
121 | + doc.unset(field) | ||
122 | + end | ||
123 | + end | ||
124 | + end | ||
125 | + end | ||
126 | + | ||
127 | + def recache | ||
128 | + CACHED_NOTICE_ATTRIBUTES.each do |k,v| | ||
129 | + # clear all cached attributes | ||
130 | + send("#{k}=", {}) | ||
131 | + | ||
132 | + # find only notices related to this problem | ||
133 | + Notice.collection.find.aggregate([ | ||
134 | + { "$match" => { err_id: { "$in" => err_ids } } }, | ||
135 | + { "$group" => { _id: "$#{v}", count: {"$sum" => 1} } } | ||
136 | + ]).each do |agg| | ||
137 | + next if agg[:_id] == nil | ||
138 | + | ||
139 | + send(k)[Digest::MD5.hexdigest(agg[:_id])] = { | ||
140 | + value: agg[:_id], | ||
141 | + count: agg[:count] | ||
142 | + } | ||
143 | + end | ||
144 | + end | ||
145 | + | ||
146 | + self.notices_count = Notice.where({ err_id: { "$in" => err_ids }}).count | ||
147 | + save | ||
148 | + end | ||
149 | + | ||
66 | def url | 150 | def url |
67 | Rails.application.routes.url_helpers.app_problem_url(app, self, | 151 | Rails.application.routes.url_helpers.app_problem_url(app, self, |
68 | :host => Errbit::Config.host, | 152 | :host => Errbit::Config.host, |
@@ -98,16 +182,21 @@ class Problem | @@ -98,16 +182,21 @@ class Problem | ||
98 | def unmerge! | 182 | def unmerge! |
99 | attrs = {:error_class => error_class, :environment => environment} | 183 | attrs = {:error_class => error_class, :environment => environment} |
100 | problem_errs = errs.to_a | 184 | problem_errs = errs.to_a |
101 | - problem_errs.shift | ||
102 | - [self] + problem_errs.map(&:id).map do |err_id| | ||
103 | - err = Err.find(err_id) | ||
104 | - app.problems.create(attrs).tap do |new_problem| | ||
105 | - err.update_attribute(:problem_id, new_problem.id) | ||
106 | - new_problem.reset_cached_attributes | ||
107 | - end | 185 | + |
186 | + # associate and return all the problems | ||
187 | + new_problems = [self] | ||
188 | + | ||
189 | + # create new problems for each err that needs one | ||
190 | + (problem_errs[1..-1] || []).each do |err| | ||
191 | + new_problems << app.problems.create(attrs) | ||
192 | + err.update_attribute(:problem, new_problems.last) | ||
108 | end | 193 | end |
109 | - end | ||
110 | 194 | ||
195 | + # recache each new problem | ||
196 | + new_problems.each(&:recache) | ||
197 | + | ||
198 | + new_problems | ||
199 | + end | ||
111 | 200 | ||
112 | def self.ordered_by(sort, order) | 201 | def self.ordered_by(sort, order) |
113 | case sort | 202 | case sort |
@@ -124,11 +213,6 @@ class Problem | @@ -124,11 +213,6 @@ class Problem | ||
124 | where(:first_notice_at.lte => date_range.end).where("$or" => [{:resolved_at => nil}, {:resolved_at.gte => date_range.begin}]) | 213 | where(:first_notice_at.lte => date_range.end).where("$or" => [{:resolved_at => nil}, {:resolved_at.gte => date_range.begin}]) |
125 | end | 214 | end |
126 | 215 | ||
127 | - | ||
128 | - def reset_cached_attributes | ||
129 | - ProblemUpdaterCache.new(self).update | ||
130 | - end | ||
131 | - | ||
132 | def cache_app_attributes | 216 | def cache_app_attributes |
133 | if app | 217 | if app |
134 | self.app_name = app.name | 218 | self.app_name = app.name |
@@ -140,14 +224,6 @@ class Problem | @@ -140,14 +224,6 @@ class Problem | ||
140 | self.message = self.message[0, 1000] if self.message | 224 | self.message = self.message[0, 1000] if self.message |
141 | end | 225 | end |
142 | 226 | ||
143 | - def remove_cached_notice_attributes(notice) | ||
144 | - update_attributes!( | ||
145 | - :messages => attribute_count_descrease(:messages, notice.message), | ||
146 | - :hosts => attribute_count_descrease(:hosts, notice.host), | ||
147 | - :user_agents => attribute_count_descrease(:user_agents, notice.user_agent_string) | ||
148 | - ) | ||
149 | - end | ||
150 | - | ||
151 | def issue_type | 227 | def issue_type |
152 | # Return issue_type if configured, but fall back to detecting app's issue tracker | 228 | # Return issue_type if configured, but fall back to detecting app's issue tracker |
153 | attributes['issue_type'] ||= | 229 | attributes['issue_type'] ||= |
config/mongo.rb
1 | -Mongoid.logger.level = Logger.const_get Errbit::Config.log_level.upcase | 1 | +log_level = Logger.const_get Errbit::Config.log_level.upcase |
2 | + | ||
3 | +Mongoid.logger.level = log_level | ||
4 | +Mongo::Logger.level = log_level | ||
2 | 5 | ||
3 | Mongoid.configure do |config| | 6 | Mongoid.configure do |config| |
4 | uri = if Errbit::Config.mongo_url == 'mongodb://localhost' | 7 | uri = if Errbit::Config.mongo_url == 'mongodb://localhost' |
spec/fabricators/err_fabricator.rb
@@ -10,6 +10,11 @@ Fabricator :notice do | @@ -10,6 +10,11 @@ Fabricator :notice do | ||
10 | server_environment { {'environment-name' => 'production'} } | 10 | server_environment { {'environment-name' => 'production'} } |
11 | request {{ 'component' => 'foo', 'action' => 'bar' }} | 11 | request {{ 'component' => 'foo', 'action' => 'bar' }} |
12 | notifier {{ 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }} | 12 | notifier {{ 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }} |
13 | + | ||
14 | + after_create do | ||
15 | + Problem.cache_notice(err.problem_id, self) | ||
16 | + problem.reload | ||
17 | + end | ||
13 | end | 18 | end |
14 | 19 | ||
15 | Fabricator :backtrace do | 20 | Fabricator :backtrace do |
spec/interactors/resolved_problem_clearer_spec.rb
@@ -19,16 +19,16 @@ describe ResolvedProblemClearer do | @@ -19,16 +19,16 @@ describe ResolvedProblemClearer do | ||
19 | } | 19 | } |
20 | end | 20 | end |
21 | it 'not repair database' do | 21 | it 'not repair database' do |
22 | - allow(Mongoid.default_session).to receive(:command).and_call_original | ||
23 | - expect(Mongoid.default_session).to_not receive(:command).with({:repairDatabase => 1}) | 22 | + allow(Mongoid.default_client).to receive(:command).and_call_original |
23 | + expect(Mongoid.default_client).to_not receive(:command).with({:repairDatabase => 1}) | ||
24 | resolved_problem_clearer.execute | 24 | resolved_problem_clearer.execute |
25 | end | 25 | end |
26 | end | 26 | end |
27 | 27 | ||
28 | context "with problem resolve" do | 28 | context "with problem resolve" do |
29 | before do | 29 | before do |
30 | - allow(Mongoid.default_session).to receive(:command).and_call_original | ||
31 | - allow(Mongoid.default_session).to receive(:command).with({:repairDatabase => 1}) | 30 | + allow(Mongoid.default_client).to receive(:command).and_call_original |
31 | + allow(Mongoid.default_client).to receive(:command).with({:repairDatabase => 1}) | ||
32 | problems.first.resolve! | 32 | problems.first.resolve! |
33 | problems.second.resolve! | 33 | problems.second.resolve! |
34 | end | 34 | end |
@@ -44,7 +44,7 @@ describe ResolvedProblemClearer do | @@ -44,7 +44,7 @@ describe ResolvedProblemClearer do | ||
44 | end | 44 | end |
45 | 45 | ||
46 | it 'repair database' do | 46 | it 'repair database' do |
47 | - expect(Mongoid.default_session).to receive(:command).with({:repairDatabase => 1}) | 47 | + expect(Mongoid.default_client).to receive(:command).with({:repairDatabase => 1}) |
48 | resolved_problem_clearer.execute | 48 | resolved_problem_clearer.execute |
49 | end | 49 | end |
50 | end | 50 | end |
spec/models/error_report_spec.rb
@@ -17,259 +17,312 @@ module Airbrake | @@ -17,259 +17,312 @@ module Airbrake | ||
17 | end | 17 | end |
18 | 18 | ||
19 | describe ErrorReport do | 19 | describe ErrorReport do |
20 | - context "with notice without line of backtrace" do | ||
21 | - let(:xml){ | ||
22 | - Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
23 | - } | ||
24 | - | ||
25 | - let(:error_report) { | ||
26 | - ErrorReport.new(xml) | ||
27 | - } | ||
28 | - | ||
29 | - let!(:app) { | ||
30 | - Fabricate( | ||
31 | - :app, | ||
32 | - :api_key => 'APIKEY' | ||
33 | - ) | ||
34 | - } | ||
35 | - | ||
36 | - describe "#app" do | ||
37 | - it 'find the good app' do | ||
38 | - expect(error_report.app).to eq app | ||
39 | - end | 20 | + let(:xml){ |
21 | + Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
22 | + } | ||
23 | + | ||
24 | + let(:error_report) { | ||
25 | + ErrorReport.new(xml) | ||
26 | + } | ||
27 | + | ||
28 | + let!(:app) { | ||
29 | + Fabricate( | ||
30 | + :app, | ||
31 | + :api_key => 'APIKEY' | ||
32 | + ) | ||
33 | + } | ||
34 | + | ||
35 | + describe "#app" do | ||
36 | + it 'find the good app' do | ||
37 | + expect(error_report.app).to eq app | ||
40 | end | 38 | end |
39 | + end | ||
41 | 40 | ||
42 | - describe "#backtrace" do | ||
43 | - it 'should have valid backtrace' do | ||
44 | - expect(error_report.backtrace).to be_valid | ||
45 | - end | 41 | + describe "#backtrace" do |
42 | + it 'should have valid backtrace' do | ||
43 | + expect(error_report.backtrace).to be_valid | ||
46 | end | 44 | end |
45 | + end | ||
47 | 46 | ||
48 | - describe "#fingerprint_strategy" do | ||
49 | - it "should be possible to change how fingerprints are generated" do | ||
50 | - def error_report.fingerprint_strategy | ||
51 | - Class.new do | ||
52 | - def self.generate(*args) | ||
53 | - 'fingerprintzzz' | ||
54 | - end | 47 | + describe "#fingerprint_strategy" do |
48 | + it "should be possible to change how fingerprints are generated" do | ||
49 | + def error_report.fingerprint_strategy | ||
50 | + Class.new do | ||
51 | + def self.generate(*args) | ||
52 | + 'fingerprintzzz' | ||
55 | end | 53 | end |
56 | end | 54 | end |
57 | - | ||
58 | - expect(error_report.error.fingerprint).to eq('fingerprintzzz') | ||
59 | end | 55 | end |
56 | + | ||
57 | + expect(error_report.error.fingerprint).to eq('fingerprintzzz') | ||
60 | end | 58 | end |
59 | + end | ||
61 | 60 | ||
62 | - describe "#generate_notice!" do | ||
63 | - it "save a notice" do | 61 | + describe "#generate_notice!" do |
62 | + it "save a notice" do | ||
63 | + expect { | ||
64 | + error_report.generate_notice! | ||
65 | + }.to change { | ||
66 | + app.reload.problems.count | ||
67 | + }.by(1) | ||
68 | + end | ||
69 | + | ||
70 | + context "with notice generate by Airbrake gem" do | ||
71 | + let(:xml) { Airbrake::Notice.new( | ||
72 | + :exception => Exception.new, | ||
73 | + :api_key => 'APIKEY', | ||
74 | + :project_root => Rails.root | ||
75 | + ).to_xml } | ||
76 | + it 'save a notice' do | ||
64 | expect { | 77 | expect { |
65 | error_report.generate_notice! | 78 | error_report.generate_notice! |
66 | }.to change { | 79 | }.to change { |
67 | app.reload.problems.count | 80 | app.reload.problems.count |
68 | }.by(1) | 81 | }.by(1) |
69 | end | 82 | end |
83 | + end | ||
70 | 84 | ||
71 | - context "with notice generate by Airbrake gem" do | ||
72 | - let(:xml) { Airbrake::Notice.new( | ||
73 | - :exception => Exception.new, | ||
74 | - :api_key => 'APIKEY', | ||
75 | - :project_root => Rails.root | ||
76 | - ).to_xml } | ||
77 | - it 'save a notice' do | ||
78 | - expect { | ||
79 | - error_report.generate_notice! | ||
80 | - }.to change { | ||
81 | - app.reload.problems.count | ||
82 | - }.by(1) | ||
83 | - end | ||
84 | - end | 85 | + describe "notice create" do |
86 | + before { error_report.generate_notice! } | ||
87 | + subject { error_report.notice } | ||
88 | + its(:message) { 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' } | ||
89 | + its(:framework) { should == 'Rails: 3.2.11' } | ||
85 | 90 | ||
86 | - describe "notice create" do | ||
87 | - before { error_report.generate_notice! } | ||
88 | - subject { error_report.notice } | ||
89 | - its(:message) { 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' } | ||
90 | - its(:framework) { should == 'Rails: 3.2.11' } | 91 | + it 'has complete backtrace' do |
92 | + expect(subject.backtrace_lines.size).to eq 73 | ||
93 | + expect(subject.backtrace_lines.last['file']).to eq '[GEM_ROOT]/bin/rake' | ||
94 | + end | ||
91 | 95 | ||
92 | - it 'has complete backtrace' do | ||
93 | - expect(subject.backtrace_lines.size).to eq 73 | ||
94 | - expect(subject.backtrace_lines.last['file']).to eq '[GEM_ROOT]/bin/rake' | ||
95 | - end | ||
96 | - it 'has server_environement' do | ||
97 | - expect(subject.server_environment['environment-name']).to eq 'development' | ||
98 | - end | 96 | + it 'has server_environement' do |
97 | + expect(subject.server_environment['environment-name']).to eq 'development' | ||
98 | + end | ||
99 | 99 | ||
100 | - it 'has request' do | ||
101 | - expect(subject.request['url']).to eq 'http://example.org/verify/cupcake=fistfight&lovebird=doomsayer' | ||
102 | - expect(subject.request['params']['controller']).to eq 'application' | ||
103 | - end | 100 | + it 'has request' do |
101 | + expect(subject.request['url']).to eq 'http://example.org/verify/cupcake=fistfight&lovebird=doomsayer' | ||
102 | + expect(subject.request['params']['controller']).to eq 'application' | ||
103 | + end | ||
104 | 104 | ||
105 | - it 'has notifier' do | ||
106 | - expect(subject.notifier['name']).to eq 'Hoptoad Notifier' | ||
107 | - end | 105 | + it 'has notifier' do |
106 | + expect(subject.notifier['name']).to eq 'Hoptoad Notifier' | ||
107 | + end | ||
108 | 108 | ||
109 | - it 'get user_attributes' do | ||
110 | - expect(subject.user_attributes['id']).to eq '123' | ||
111 | - expect(subject.user_attributes['name']).to eq 'Mr. Bean' | ||
112 | - expect(subject.user_attributes['email']).to eq 'mr.bean@example.com' | ||
113 | - expect(subject.user_attributes['username']).to eq 'mrbean' | ||
114 | - end | 109 | + it 'get user_attributes' do |
110 | + expect(subject.user_attributes['id']).to eq '123' | ||
111 | + expect(subject.user_attributes['name']).to eq 'Mr. Bean' | ||
112 | + expect(subject.user_attributes['email']).to eq 'mr.bean@example.com' | ||
113 | + expect(subject.user_attributes['username']).to eq 'mrbean' | ||
114 | + end | ||
115 | 115 | ||
116 | - it 'valid env_vars' do | ||
117 | - # XML: <var key="SCRIPT_NAME"/> | ||
118 | - expect(subject.env_vars).to have_key('SCRIPT_NAME') | ||
119 | - expect(subject.env_vars['SCRIPT_NAME']).to be_nil # blank ends up nil | ||
120 | - | ||
121 | - # XML representation: | ||
122 | - # <var key="rack.session.options"> | ||
123 | - # <var key="secure">false</var> | ||
124 | - # <var key="httponly">true</var> | ||
125 | - # <var key="path">/</var> | ||
126 | - # <var key="expire_after"/> | ||
127 | - # <var key="domain"/> | ||
128 | - # <var key="id"/> | ||
129 | - # </var> | ||
130 | - expected = { | ||
131 | - 'secure' => 'false', | ||
132 | - 'httponly' => 'true', | ||
133 | - 'path' => '/', | ||
134 | - 'expire_after' => nil, | ||
135 | - 'domain' => nil, | ||
136 | - 'id' => nil | ||
137 | - } | ||
138 | - expect(subject.env_vars).to have_key('rack_session_options') | ||
139 | - expect(subject.env_vars['rack_session_options']).to eql(expected) | ||
140 | - end | 116 | + it 'valid env_vars' do |
117 | + # XML: <var key="SCRIPT_NAME"/> | ||
118 | + expect(subject.env_vars).to have_key('SCRIPT_NAME') | ||
119 | + expect(subject.env_vars['SCRIPT_NAME']).to be_nil # blank ends up nil | ||
120 | + | ||
121 | + # XML representation: | ||
122 | + # <var key="rack.session.options"> | ||
123 | + # <var key="secure">false</var> | ||
124 | + # <var key="httponly">true</var> | ||
125 | + # <var key="path">/</var> | ||
126 | + # <var key="expire_after"/> | ||
127 | + # <var key="domain"/> | ||
128 | + # <var key="id"/> | ||
129 | + # </var> | ||
130 | + expected = { | ||
131 | + 'secure' => 'false', | ||
132 | + 'httponly' => 'true', | ||
133 | + 'path' => '/', | ||
134 | + 'expire_after' => nil, | ||
135 | + 'domain' => nil, | ||
136 | + 'id' => nil | ||
137 | + } | ||
138 | + expect(subject.env_vars).to have_key('rack_session_options') | ||
139 | + expect(subject.env_vars['rack_session_options']).to eql(expected) | ||
141 | end | 140 | end |
142 | end | 141 | end |
142 | + end | ||
143 | 143 | ||
144 | - it 'save a notice assignes to err' do | 144 | + describe '#cache_attributes_on_problem' do |
145 | + it 'sets the latest notice properties on the problem' do | ||
145 | error_report.generate_notice! | 146 | error_report.generate_notice! |
146 | - expect(error_report.notice.err).to be_a(Err) | 147 | + problem = error_report.problem.reload |
148 | + notice = error_report.notice.reload | ||
149 | + | ||
150 | + expect(problem.environment).to eq('development') | ||
151 | + expect(problem.error_class).to eq('HoptoadTestingException') | ||
152 | + expect(problem.last_notice_at).to eq(notice.created_at) | ||
153 | + expect(problem.message).to eq(notice.message) | ||
154 | + expect(problem.where).to eq(notice.where) | ||
147 | end | 155 | end |
148 | 156 | ||
149 | - it 'memoize the notice' do | ||
150 | - expect { | ||
151 | - error_report.generate_notice! | ||
152 | - error_report.generate_notice! | ||
153 | - }.to change { | ||
154 | - Notice.count | ||
155 | - }.by(1) | 157 | + it 'unresolves the problem' do |
158 | + error_report.generate_notice! | ||
159 | + problem = error_report.problem | ||
160 | + problem.update( | ||
161 | + resolved_at: Time.now, | ||
162 | + resolved: true | ||
163 | + ) | ||
164 | + | ||
165 | + error_report = ErrorReport.new(xml) | ||
166 | + error_report.generate_notice! | ||
167 | + problem.reload | ||
168 | + | ||
169 | + expect(problem.resolved_at).to be(nil) | ||
170 | + expect(problem.resolved).to be(false) | ||
156 | end | 171 | end |
157 | 172 | ||
158 | - it 'find the correct err for the notice' do | ||
159 | - err = Fabricate(:err, :problem => Fabricate(:problem, :resolved => true)) | 173 | + it 'caches notice counts' do |
174 | + error_report.generate_notice! | ||
175 | + problem = error_report.problem | ||
176 | + problem.reload | ||
177 | + | ||
178 | + expect(problem.notices_count).to be(1) | ||
179 | + expect(problem.user_agents['382b0f5185773fa0f67a8ed8056c7759']['count']).to be(1) | ||
180 | + expect(problem.messages['9449f087eee0499e2d9029ae3dacaf53']['count']).to be(1) | ||
181 | + expect(problem.hosts['1bdf72e04d6b50c82a48c7e4dd38cc69']['count']).to be(1) | ||
182 | + end | ||
160 | 183 | ||
161 | - allow(error_report).to receive(:fingerprint).and_return(err.fingerprint) | 184 | + it 'increments notice counts' do |
185 | + error_report.generate_notice! | ||
186 | + error_report = ErrorReport.new(xml) | ||
187 | + error_report.generate_notice! | ||
188 | + problem = error_report.problem | ||
189 | + problem.reload | ||
162 | 190 | ||
163 | - expect { | ||
164 | - error_report.generate_notice! | ||
165 | - }.to change { | ||
166 | - error_report.error.resolved? | ||
167 | - }.from(true).to(false) | 191 | + expect(problem.notices_count).to be(2) |
192 | + expect(problem.user_agents['382b0f5185773fa0f67a8ed8056c7759']['count']).to be(2) | ||
193 | + expect(problem.messages['9449f087eee0499e2d9029ae3dacaf53']['count']).to be(2) | ||
194 | + expect(problem.hosts['1bdf72e04d6b50c82a48c7e4dd38cc69']['count']).to be(2) | ||
168 | end | 195 | end |
196 | + end | ||
169 | 197 | ||
170 | - context "with notification service configured" do | ||
171 | - before do | ||
172 | - app.notify_on_errs = true | ||
173 | - app.watchers.build(:email => 'foo@example.com') | ||
174 | - app.save | ||
175 | - end | ||
176 | - it 'send email' do | ||
177 | - notice = error_report.generate_notice! | ||
178 | - email = ActionMailer::Base.deliveries.last | ||
179 | - expect(email.to).to include(app.watchers.first.email) | ||
180 | - expect(email.subject).to include(notice.message.truncate(50)) | ||
181 | - expect(email.subject).to include("[#{app.name}]") | ||
182 | - expect(email.subject).to include("[#{notice.environment_name}]") | ||
183 | - end | 198 | + it 'save a notice assignes to err' do |
199 | + error_report.generate_notice! | ||
200 | + expect(error_report.notice.err).to be_a(Err) | ||
201 | + end | ||
184 | 202 | ||
185 | - context "with xml without request section" do | ||
186 | - let(:xml){ | ||
187 | - Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | ||
188 | - } | ||
189 | - it "save a notice" do | ||
190 | - expect { | ||
191 | - error_report.generate_notice! | ||
192 | - }.to change { | ||
193 | - app.reload.problems.count | ||
194 | - }.by(1) | ||
195 | - end | 203 | + it 'memoize the notice' do |
204 | + expect { | ||
205 | + error_report.generate_notice! | ||
206 | + error_report.generate_notice! | ||
207 | + }.to change { | ||
208 | + Notice.count | ||
209 | + }.by(1) | ||
210 | + end | ||
211 | + | ||
212 | + it 'find the correct err for the notice' do | ||
213 | + err = Fabricate(:err, :problem => Fabricate(:problem, :resolved => true)) | ||
214 | + | ||
215 | + allow(error_report).to receive(:fingerprint).and_return(err.fingerprint) | ||
216 | + | ||
217 | + expect { | ||
218 | + error_report.generate_notice! | ||
219 | + }.to change { | ||
220 | + error_report.error.resolved? | ||
221 | + }.from(true).to(false) | ||
222 | + end | ||
223 | + | ||
224 | + context "with notification service configured" do | ||
225 | + before do | ||
226 | + app.notify_on_errs = true | ||
227 | + app.watchers.build(:email => 'foo@example.com') | ||
228 | + app.save | ||
229 | + end | ||
230 | + it 'send email' do | ||
231 | + notice = error_report.generate_notice! | ||
232 | + email = ActionMailer::Base.deliveries.last | ||
233 | + expect(email.to).to include(app.watchers.first.email) | ||
234 | + expect(email.subject).to include(notice.message.truncate(50)) | ||
235 | + expect(email.subject).to include("[#{app.name}]") | ||
236 | + expect(email.subject).to include("[#{notice.environment_name}]") | ||
237 | + end | ||
238 | + | ||
239 | + context "with xml without request section" do | ||
240 | + let(:xml){ | ||
241 | + Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | ||
242 | + } | ||
243 | + it "save a notice" do | ||
244 | + expect { | ||
245 | + error_report.generate_notice! | ||
246 | + }.to change { | ||
247 | + app.reload.problems.count | ||
248 | + }.by(1) | ||
196 | end | 249 | end |
250 | + end | ||
197 | 251 | ||
198 | - context "with xml with only a single line of backtrace" do | ||
199 | - let(:xml){ | ||
200 | - Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read | ||
201 | - } | ||
202 | - it "save a notice" do | ||
203 | - expect { | ||
204 | - error_report.generate_notice! | ||
205 | - }.to change { | ||
206 | - app.reload.problems.count | ||
207 | - }.by(1) | ||
208 | - end | 252 | + context "with xml with only a single line of backtrace" do |
253 | + let(:xml){ | ||
254 | + Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read | ||
255 | + } | ||
256 | + it "save a notice" do | ||
257 | + expect { | ||
258 | + error_report.generate_notice! | ||
259 | + }.to change { | ||
260 | + app.reload.problems.count | ||
261 | + }.by(1) | ||
209 | end | 262 | end |
210 | end | 263 | end |
264 | + end | ||
211 | 265 | ||
212 | - describe "#valid?" do | ||
213 | - context "with valid error report" do | ||
214 | - it "return true" do | ||
215 | - expect(error_report.valid?).to be true | ||
216 | - end | 266 | + describe "#valid?" do |
267 | + context "with valid error report" do | ||
268 | + it "return true" do | ||
269 | + expect(error_report.valid?).to be true | ||
217 | end | 270 | end |
218 | - context "with not valid api_key" do | ||
219 | - before do | ||
220 | - App.where(:api_key => app.api_key).delete_all | ||
221 | - end | ||
222 | - it "return false" do | ||
223 | - expect(error_report.valid?).to be false | ||
224 | - end | 271 | + end |
272 | + context "with not valid api_key" do | ||
273 | + before do | ||
274 | + App.where(:api_key => app.api_key).delete_all | ||
275 | + end | ||
276 | + it "return false" do | ||
277 | + expect(error_report.valid?).to be false | ||
225 | end | 278 | end |
226 | end | 279 | end |
280 | + end | ||
227 | 281 | ||
228 | - describe "#notice" do | ||
229 | - context "before generate_notice!" do | ||
230 | - it 'return nil' do | ||
231 | - expect(error_report.notice).to be nil | ||
232 | - end | 282 | + describe "#notice" do |
283 | + context "before generate_notice!" do | ||
284 | + it 'return nil' do | ||
285 | + expect(error_report.notice).to be nil | ||
233 | end | 286 | end |
287 | + end | ||
234 | 288 | ||
235 | - context "after generate_notice!" do | ||
236 | - before do | ||
237 | - error_report.generate_notice! | ||
238 | - end | ||
239 | - | ||
240 | - it 'return the notice' do | ||
241 | - expect(error_report.notice).to be_a Notice | ||
242 | - end | 289 | + context "after generate_notice!" do |
290 | + before do | ||
291 | + error_report.generate_notice! | ||
292 | + end | ||
243 | 293 | ||
294 | + it 'return the notice' do | ||
295 | + expect(error_report.notice).to be_a Notice | ||
244 | end | 296 | end |
297 | + | ||
245 | end | 298 | end |
299 | + end | ||
246 | 300 | ||
247 | - describe "#should_keep?" do | ||
248 | - context "with current app version not set" do | ||
249 | - before do | ||
250 | - error_report.app.current_app_version = nil | ||
251 | - error_report.server_environment['app-version'] = '1.0' | ||
252 | - end | 301 | + describe "#should_keep?" do |
302 | + context "with current app version not set" do | ||
303 | + before do | ||
304 | + error_report.app.current_app_version = nil | ||
305 | + error_report.server_environment['app-version'] = '1.0' | ||
306 | + end | ||
253 | 307 | ||
254 | - it "return true" do | ||
255 | - expect(error_report.should_keep?).to be true | ||
256 | - end | 308 | + it "return true" do |
309 | + expect(error_report.should_keep?).to be true | ||
257 | end | 310 | end |
311 | + end | ||
258 | 312 | ||
259 | - context "with current app version set" do | ||
260 | - before do | ||
261 | - error_report.app.current_app_version = '1.0' | ||
262 | - end | 313 | + context "with current app version set" do |
314 | + before do | ||
315 | + error_report.app.current_app_version = '1.0' | ||
316 | + end | ||
263 | 317 | ||
264 | - it "return true if current or newer" do | ||
265 | - error_report.server_environment['app-version'] = '1.0' | ||
266 | - expect(error_report.should_keep?).to be true | ||
267 | - end | 318 | + it "return true if current or newer" do |
319 | + error_report.server_environment['app-version'] = '1.0' | ||
320 | + expect(error_report.should_keep?).to be true | ||
321 | + end | ||
268 | 322 | ||
269 | - it "return false if older" do | ||
270 | - error_report.server_environment['app-version'] = '0.9' | ||
271 | - expect(error_report.should_keep?).to be false | ||
272 | - end | 323 | + it "return false if older" do |
324 | + error_report.server_environment['app-version'] = '0.9' | ||
325 | + expect(error_report.should_keep?).to be false | ||
273 | end | 326 | end |
274 | end | 327 | end |
275 | end | 328 | end |
spec/models/notice_observer_spec.rb
1 | describe "Callback on Notice", type: 'model' do | 1 | describe "Callback on Notice", type: 'model' do |
2 | - describe "email notifications (configured individually for each app)" do | 2 | + let(:notice_attrs_for) do |
3 | + ->(api_key) do | ||
4 | + { | ||
5 | + error_class: "HoptoadTestingException", | ||
6 | + message: "some message", | ||
7 | + backtrace: [ | ||
8 | + { | ||
9 | + "number"=>"425", | ||
10 | + "file"=>"[GEM_ROOT]/callbacks.rb", | ||
11 | + "method"=>"__callbacks" | ||
12 | + } | ||
13 | + ], | ||
14 | + request: { "component" => "application" }, | ||
15 | + server_environment: { | ||
16 | + "project-root" => "/path/to/sample/project", | ||
17 | + "environment-name" => "development" | ||
18 | + }, | ||
19 | + api_key: api_key, | ||
20 | + notifier: { | ||
21 | + "name"=>"Hoptoad Notifier", | ||
22 | + "version"=>"2.3.2", | ||
23 | + "url"=>"http://hoptoadapp.com" | ||
24 | + }, | ||
25 | + framework: "Rails: 3.2.11" | ||
26 | + } | ||
27 | + end | ||
28 | + end | ||
29 | + | ||
30 | + describe 'email notifications (configured individually for each app)' do | ||
31 | + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } | ||
3 | custom_thresholds = [2, 4, 8, 16, 32, 64] | 32 | custom_thresholds = [2, 4, 8, 16, 32, 64] |
33 | + let(:app) do | ||
34 | + Fabricate(:app_with_watcher, email_at_notices: custom_thresholds) | ||
35 | + end | ||
4 | 36 | ||
5 | before do | 37 | before do |
6 | Errbit::Config.per_app_email_at_notices = true | 38 | Errbit::Config.per_app_email_at_notices = true |
7 | - @app = Fabricate(:app_with_watcher, :email_at_notices => custom_thresholds) | ||
8 | - @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app)) | 39 | + error_report = ErrorReport.new(notice_attrs) |
40 | + error_report.generate_notice! | ||
41 | + @problem = error_report.notice.err.problem | ||
9 | end | 42 | end |
10 | 43 | ||
11 | - after do | ||
12 | - Errbit::Config.per_app_email_at_notices = false | ||
13 | - end | 44 | + after { Errbit::Config.per_app_email_at_notices = false } |
14 | 45 | ||
15 | custom_thresholds.each do |threshold| | 46 | custom_thresholds.each do |threshold| |
16 | it "sends an email notification after #{threshold} notice(s)" do | 47 | it "sends an email notification after #{threshold} notice(s)" do |
17 | - allow(@err.problem).to receive(:notices_count).and_return(threshold) | 48 | + # set to just before the threshold |
49 | + @problem.update_attributes notices_count: threshold - 1 | ||
50 | + | ||
18 | expect(Mailer).to receive(:err_notification). | 51 | expect(Mailer).to receive(:err_notification). |
19 | and_return(double('email', :deliver => true)) | 52 | and_return(double('email', :deliver => true)) |
20 | - Fabricate(:notice, :err => @err) | 53 | + |
54 | + error_report = ErrorReport.new(notice_attrs) | ||
55 | + error_report.generate_notice! | ||
21 | end | 56 | end |
22 | end | 57 | end |
23 | - end | ||
24 | 58 | ||
25 | - describe "email notifications for a resolved issue" do | ||
26 | - before do | ||
27 | - Errbit::Config.per_app_email_at_notices = true | ||
28 | - @app = Fabricate(:app_with_watcher, :email_at_notices => [1]) | ||
29 | - @err = Fabricate(:err, :problem => Fabricate(:problem, :app => @app, :notices_count => 100)) | ||
30 | - end | 59 | + it "doesn't email after 5 notices" do |
60 | + @problem.update_attributes notices_count: 5 | ||
31 | 61 | ||
32 | - after do | ||
33 | - Errbit::Config.per_app_email_at_notices = false | ||
34 | - end | 62 | + expect(Mailer).to_not receive(:err_notification) |
35 | 63 | ||
36 | - it "should send email notification after 1 notice since an error has been resolved" do | ||
37 | - @err.problem.resolve! | ||
38 | - expect(Mailer).to receive(:err_notification). | ||
39 | - and_return(double('email', :deliver => true)) | ||
40 | - Fabricate(:notice, :err => @err) | 64 | + error_report = ErrorReport.new(notice_attrs) |
65 | + error_report.generate_notice! | ||
41 | end | 66 | end |
42 | - it 'self notify if mailer failed' do | ||
43 | - @err.problem.resolve! | ||
44 | - expect(Mailer).to receive(:err_notification). | ||
45 | - and_raise(ArgumentError) | 67 | + |
68 | + it 'notify self if mailer fails' do | ||
69 | + expect(Mailer).to receive(:err_notification).and_raise(ArgumentError) | ||
46 | expect(HoptoadNotifier).to receive(:notify) | 70 | expect(HoptoadNotifier).to receive(:notify) |
47 | - Fabricate(:notice, :err => @err) | 71 | + ErrorReport.new(notice_attrs).generate_notice! |
48 | end | 72 | end |
49 | end | 73 | end |
50 | 74 | ||
51 | - describe "should send a notification if a notification service is configured with defaults" do | ||
52 | - let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:campfire_notification_service))} | ||
53 | - let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } | ||
54 | - let(:backtrace) { Fabricate(:backtrace) } | ||
55 | - | ||
56 | - before do | ||
57 | - Errbit::Config.per_app_email_at_notices = true | 75 | + describe 'email notifications for resolved issues' do |
76 | + let(:notification_service) { Fabricate(:campfire_notification_service) } | ||
77 | + let(:app) do | ||
78 | + Fabricate( | ||
79 | + :app_with_watcher, | ||
80 | + notify_on_errs: true, | ||
81 | + email_at_notices: [1,100] | ||
82 | + ) | ||
58 | end | 83 | end |
84 | + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } | ||
59 | 85 | ||
60 | - after do | ||
61 | - Errbit::Config.per_app_email_at_notices = false | ||
62 | - end | 86 | + before { Errbit::Config.per_app_email_at_notices = true } |
87 | + after { Errbit::Config.per_app_email_at_notices = false } | ||
63 | 88 | ||
64 | - it "should create a campfire notification" do | ||
65 | - expect(app.notification_service).to receive(:create_notification) | 89 | + it 'sends email the first time after the error is resolved' do |
90 | + error_report = ErrorReport.new(notice_attrs) | ||
91 | + error_report.generate_notice! | ||
92 | + err = error_report.notice.err | ||
66 | 93 | ||
67 | - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | ||
68 | - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | ||
69 | - end | ||
70 | - end | 94 | + err.problem.update_attributes notices_count: 99 |
95 | + err.problem.resolve! | ||
71 | 96 | ||
72 | - describe "send a notification if a notification service is configured with defaults but failed" do | ||
73 | - let(:app) { Fabricate(:app_with_watcher, | ||
74 | - :notify_on_errs => true, | ||
75 | - :email_at_notices => [1, 100], :notification_service => Fabricate(:campfire_notification_service))} | ||
76 | - let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 99)) } | ||
77 | - let(:backtrace) { Fabricate(:backtrace) } | 97 | + expect(Mailer).to receive(:err_notification) |
98 | + .and_return(double('email', :deliver => true)) | ||
78 | 99 | ||
79 | - before do | ||
80 | - Errbit::Config.per_app_email_at_notices = true | 100 | + ErrorReport.new(notice_attrs).generate_notice! |
81 | end | 101 | end |
102 | + end | ||
82 | 103 | ||
83 | - after do | ||
84 | - Errbit::Config.per_app_email_at_notices = false | 104 | + describe 'send email when notification service is configured but fails' do |
105 | + let(:notification_service) {Fabricate(:campfire_notification_service)} | ||
106 | + let(:app) do | ||
107 | + Fabricate( | ||
108 | + :app_with_watcher, | ||
109 | + notify_on_errs: true, | ||
110 | + notification_service: notification_service | ||
111 | + ) | ||
85 | end | 112 | end |
113 | + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } | ||
114 | + | ||
115 | + before { Errbit::Config.per_app_notify_at_notices = true } | ||
116 | + after { Errbit::Config.per_app_notify_at_notices = false } | ||
117 | + | ||
118 | + it 'sends email' do | ||
119 | + error_report = ErrorReport.new(notice_attrs) | ||
86 | 120 | ||
87 | - it "send email" do | ||
88 | - expect(app.notification_service).to receive(:create_notification).and_raise(ArgumentError) | ||
89 | - expect(Mailer).to receive(:err_notification).and_return(double(:deliver => true)) | 121 | + expect(error_report.app.notification_service) |
122 | + .to receive(:create_notification).and_raise(ArgumentError) | ||
123 | + expect(Mailer) | ||
124 | + .to receive(:err_notification).and_return(double(:deliver => true)) | ||
90 | 125 | ||
91 | - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | ||
92 | - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | 126 | + error_report.generate_notice! |
93 | end | 127 | end |
94 | end | 128 | end |
95 | 129 | ||
96 | - describe "should not send a notification if a notification service is not configured" do | ||
97 | - let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))} | ||
98 | - let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } | ||
99 | - let(:backtrace) { Fabricate(:backtrace) } | 130 | + describe 'should not send a notification if a notification service is not' \ |
131 | + 'configured' do | ||
100 | 132 | ||
101 | - before do | ||
102 | - Errbit::Config.per_app_email_at_notices = true | ||
103 | - end | 133 | + let(:notification_service) { Fabricate(:notification_service) } |
134 | + let(:app) { Fabricate(:app, notification_service: notification_service )} | ||
135 | + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } | ||
104 | 136 | ||
105 | - after do | ||
106 | - Errbit::Config.per_app_email_at_notices = false | ||
107 | - end | 137 | + before { Errbit::Config.per_app_notify_at_notices = true } |
138 | + after { Errbit::Config.per_app_notify_at_notices = false } | ||
108 | 139 | ||
109 | it "should not create a campfire notification" do | 140 | it "should not create a campfire notification" do |
110 | - expect(app.notification_service).to_not receive(:create_notification) | ||
111 | - | ||
112 | - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | ||
113 | - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | 141 | + error_report = ErrorReport.new(notice_attrs) |
142 | + expect(error_report.app.notification_service).to_not receive(:create_notification) | ||
143 | + error_report.generate_notice! | ||
114 | end | 144 | end |
115 | end | 145 | end |
116 | 146 | ||
117 | describe 'hipcat notifications' do | 147 | describe 'hipcat notifications' do |
118 | - let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:hipchat_notification_service))} | ||
119 | - let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } | 148 | + let(:notification_service) { Fabricate(:hipchat_notification_service) } |
149 | + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } | ||
150 | + let(:app) { Fabricate(:app, notification_service: notification_service) } | ||
120 | 151 | ||
121 | - before do | ||
122 | - Errbit::Config.per_app_email_at_notices = true | ||
123 | - end | ||
124 | - | ||
125 | - after do | ||
126 | - Errbit::Config.per_app_email_at_notices = false | ||
127 | - end | 152 | + before { Errbit::Config.per_app_notify_at_notices = true } |
153 | + after { Errbit::Config.per_app_notify_at_notices = false } | ||
128 | 154 | ||
129 | it 'creates a hipchat notification' do | 155 | it 'creates a hipchat notification' do |
130 | - expect(app.notification_service).to receive(:create_notification) | ||
131 | - | ||
132 | - Fabricate(:notice, :err => err) | 156 | + error_report = ErrorReport.new(notice_attrs) |
157 | + expect(error_report.app.notification_service) | ||
158 | + .to receive(:create_notification) | ||
159 | + error_report.generate_notice! | ||
133 | end | 160 | end |
134 | end | 161 | end |
135 | 162 | ||
136 | describe "should send a notification at desired intervals" do | 163 | describe "should send a notification at desired intervals" do |
137 | - let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:campfire_notification_service, :notify_at_notices => [1,2]))} | ||
138 | - let(:backtrace) { Fabricate(:backtrace) } | ||
139 | - | ||
140 | - before do | ||
141 | - Errbit::Config.per_app_email_at_notices = true | 164 | + let(:notification_service) do |
165 | + Fabricate(:campfire_notification_service, notify_at_notices: [1,2]) | ||
142 | end | 166 | end |
167 | + let(:app) { Fabricate(:app, notification_service: notification_service) } | ||
168 | + let(:notice_attrs) { notice_attrs_for.call(app.api_key) } | ||
143 | 169 | ||
144 | - after do | ||
145 | - Errbit::Config.per_app_email_at_notices = false | ||
146 | - end | 170 | + before { Errbit::Config.per_app_notify_at_notices = true } |
171 | + after { Errbit::Config.per_app_notify_at_notices = false } | ||
147 | 172 | ||
148 | it "should create a campfire notification on first notice" do | 173 | it "should create a campfire notification on first notice" do |
149 | - err = Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 1)) | ||
150 | - expect(app.notification_service).to receive(:create_notification) | ||
151 | - | ||
152 | - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | ||
153 | - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | 174 | + error_report = ErrorReport.new(notice_attrs) |
175 | + expect(error_report.app.notification_service) | ||
176 | + .to receive(:create_notification) | ||
177 | + error_report.generate_notice! # one | ||
154 | end | 178 | end |
155 | 179 | ||
156 | it "should create a campfire notification on second notice" do | 180 | it "should create a campfire notification on second notice" do |
157 | - err = Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 1)) | ||
158 | - expect(app.notification_service).to receive(:create_notification) | ||
159 | - | ||
160 | - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | ||
161 | - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | 181 | + ErrorReport.new(notice_attrs).generate_notice! # one |
182 | + error_report = ErrorReport.new(notice_attrs) | ||
183 | + expect(error_report.app.notification_service) | ||
184 | + .to receive(:create_notification) | ||
185 | + error_report.generate_notice! # two | ||
162 | end | 186 | end |
163 | 187 | ||
164 | it "should not create a campfire notification on third notice" do | 188 | it "should not create a campfire notification on third notice" do |
165 | - err = Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 1)) | ||
166 | - expect(app.notification_service).to receive(:create_notification) | ||
167 | - | ||
168 | - Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | ||
169 | - :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | 189 | + ErrorReport.new(notice_attrs).generate_notice! # one |
190 | + ErrorReport.new(notice_attrs).generate_notice! # two | ||
191 | + error_report = ErrorReport.new(notice_attrs) | ||
192 | + expect(error_report.app.notification_service) | ||
193 | + .to_not receive(:create_notification) | ||
194 | + error_report.generate_notice! # three | ||
170 | end | 195 | end |
171 | end | 196 | end |
172 | end | 197 | end |
spec/models/notice_spec.rb
@@ -3,7 +3,7 @@ describe Notice, type: 'model' do | @@ -3,7 +3,7 @@ describe Notice, type: 'model' do | ||
3 | it 'requires a backtrace' do | 3 | it 'requires a backtrace' do |
4 | notice = Fabricate.build(:notice, :backtrace => nil) | 4 | notice = Fabricate.build(:notice, :backtrace => nil) |
5 | expect(notice).to_not be_valid | 5 | expect(notice).to_not be_valid |
6 | - expect(notice.errors[:backtrace]).to include("can't be blank") | 6 | + expect(notice.errors[:backtrace_id]).to include("can't be blank") |
7 | end | 7 | end |
8 | 8 | ||
9 | it 'requires the server_environment' do | 9 | it 'requires the server_environment' do |
spec/models/problem_spec.rb
@@ -40,10 +40,10 @@ describe Problem, type: 'model' do | @@ -40,10 +40,10 @@ describe Problem, type: 'model' do | ||
40 | expect(problem).to_not be_nil | 40 | expect(problem).to_not be_nil |
41 | 41 | ||
42 | notice1 = Fabricate(:notice, :err => err) | 42 | notice1 = Fabricate(:notice, :err => err) |
43 | - expect(problem.last_notice_at).to eq notice1.created_at | 43 | + expect(problem.last_notice_at).to eq notice1.reload.created_at |
44 | 44 | ||
45 | notice2 = Fabricate(:notice, :err => err) | 45 | notice2 = Fabricate(:notice, :err => err) |
46 | - expect(problem.last_notice_at).to eq notice2.created_at | 46 | + expect(problem.last_notice_at).to eq notice2.reload.created_at |
47 | end | 47 | end |
48 | end | 48 | end |
49 | 49 | ||
@@ -266,12 +266,6 @@ describe Problem, type: 'model' do | @@ -266,12 +266,6 @@ describe Problem, type: 'model' do | ||
266 | expect(@problem.messages).to eq ({}) | 266 | expect(@problem.messages).to eq ({}) |
267 | end | 267 | end |
268 | 268 | ||
269 | - it "adding a notice adds a string to #messages" do | ||
270 | - expect { | ||
271 | - Fabricate(:notice, :err => @err, :message => 'ERR 1') | ||
272 | - }.to change(@problem, :messages).from({}).to({Digest::MD5.hexdigest('ERR 1') => {'value' => 'ERR 1', 'count' => 1}}) | ||
273 | - end | ||
274 | - | ||
275 | it "removing a notice removes string from #messages" do | 269 | it "removing a notice removes string from #messages" do |
276 | Fabricate(:notice, :err => @err, :message => 'ERR 1') | 270 | Fabricate(:notice, :err => @err, :message => 'ERR 1') |
277 | expect { | 271 | expect { |
@@ -299,12 +293,6 @@ describe Problem, type: 'model' do | @@ -299,12 +293,6 @@ describe Problem, type: 'model' do | ||
299 | expect(@problem.hosts).to eq ({}) | 293 | expect(@problem.hosts).to eq ({}) |
300 | end | 294 | end |
301 | 295 | ||
302 | - it "adding a notice adds a string to #hosts" do | ||
303 | - expect { | ||
304 | - Fabricate(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) | ||
305 | - }.to change(@problem, :hosts).from({}).to({Digest::MD5.hexdigest('example.com') => {'value' => 'example.com', 'count' => 1}}) | ||
306 | - end | ||
307 | - | ||
308 | it "removing a notice removes string from #hosts" do | 296 | it "removing a notice removes string from #hosts" do |
309 | Fabricate(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) | 297 | Fabricate(:notice, :err => @err, :request => {'url' => "http://example.com/resource/12"}) |
310 | expect { | 298 | expect { |
@@ -325,12 +313,6 @@ describe Problem, type: 'model' do | @@ -325,12 +313,6 @@ describe Problem, type: 'model' do | ||
325 | expect(@problem.user_agents).to eq ({}) | 313 | expect(@problem.user_agents).to eq ({}) |
326 | end | 314 | end |
327 | 315 | ||
328 | - it "adding a notice adds a string to #user_agents" do | ||
329 | - expect { | ||
330 | - 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'}}) | ||
331 | - }.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}}) | ||
332 | - end | ||
333 | - | ||
334 | it "removing a notice removes string from #user_agents" do | 316 | it "removing a notice removes string from #user_agents" do |
335 | 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'}}) | 317 | 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'}}) |
336 | expect { | 318 | expect { |
spec/spec_helper.rb
1 | # This file is copied to ~/spec when you run 'ruby script/generate rspec' | 1 | # This file is copied to ~/spec when you run 'ruby script/generate rspec' |
2 | # from the project root directory. | 2 | # from the project root directory. |
3 | -ENV["RAILS_ENV"] ||= 'test' | 3 | +ENV["RAILS_ENV"] = 'test' |
4 | +ENV["ERRBIT_LOG_LEVEL"] = 'error' | ||
4 | 5 | ||
5 | if ENV['COVERAGE'] | 6 | if ENV['COVERAGE'] |
6 | require 'coveralls' | 7 | require 'coveralls' |
@@ -21,7 +22,6 @@ require File.expand_path("../../config/environment", __FILE__) | @@ -21,7 +22,6 @@ require File.expand_path("../../config/environment", __FILE__) | ||
21 | require 'rspec/rails' | 22 | require 'rspec/rails' |
22 | require 'rspec/its' | 23 | require 'rspec/its' |
23 | require 'email_spec' | 24 | require 'email_spec' |
24 | -require 'database_cleaner' | ||
25 | require 'xmpp4r' | 25 | require 'xmpp4r' |
26 | require 'xmpp4r/muc' | 26 | require 'xmpp4r/muc' |
27 | require 'mongoid-rspec' | 27 | require 'mongoid-rspec' |