Commit 8a7183a37003b246f6aaf331b5a021662f85a9e5
1 parent
cf166826
Exists in
master
and in
1 other branch
Extract completly the ErrorReport class
Extract all concern about this call to his own class and move test to this class
Showing
6 changed files
with
36 additions
and
84 deletions
Show diff stats
app/models/app.rb
| @@ -42,44 +42,6 @@ class App | @@ -42,44 +42,6 @@ class App | ||
| 42 | accepts_nested_attributes_for :notification_service, :allow_destroy => true, | 42 | accepts_nested_attributes_for :notification_service, :allow_destroy => true, |
| 43 | :reject_if => proc { |attrs| !NotificationService.subclasses.map(&:to_s).include?(attrs[:type].to_s) } | 43 | :reject_if => proc { |attrs| !NotificationService.subclasses.map(&:to_s).include?(attrs[:type].to_s) } |
| 44 | 44 | ||
| 45 | - # Processes a new error report. | ||
| 46 | - # | ||
| 47 | - # Accepts either XML or a hash with the following attributes: | ||
| 48 | - # | ||
| 49 | - # * <tt>:error_class</tt> - the class of error | ||
| 50 | - # * <tt>:message</tt> - the error message | ||
| 51 | - # * <tt>:backtrace</tt> - an array of stack trace lines | ||
| 52 | - # | ||
| 53 | - # * <tt>:request</tt> - a hash of values describing the request | ||
| 54 | - # * <tt>:server_environment</tt> - a hash of values describing the server environment | ||
| 55 | - # | ||
| 56 | - # * <tt>:api_key</tt> - the API key with which the error was reported | ||
| 57 | - # * <tt>:notifier</tt> - information to identify the source of the error report | ||
| 58 | - # | ||
| 59 | - def self.report_error!(*args) | ||
| 60 | - report = ErrorReport.new(*args) | ||
| 61 | - report.generate_notice! | ||
| 62 | - end | ||
| 63 | - | ||
| 64 | - | ||
| 65 | - # Processes a new error report. | ||
| 66 | - # | ||
| 67 | - # Accepts a hash with the following attributes: | ||
| 68 | - # | ||
| 69 | - # * <tt>:error_class</tt> - the class of error | ||
| 70 | - # * <tt>:message</tt> - the error message | ||
| 71 | - # * <tt>:backtrace</tt> - an array of stack trace lines | ||
| 72 | - # | ||
| 73 | - # * <tt>:request</tt> - a hash of values describing the request | ||
| 74 | - # * <tt>:server_environment</tt> - a hash of values describing the server environment | ||
| 75 | - # | ||
| 76 | - # * <tt>:notifier</tt> - information to identify the source of the error report | ||
| 77 | - # | ||
| 78 | - def report_error!(hash) | ||
| 79 | - report = ErrorReport.new(hash.merge(:api_key => api_key)) | ||
| 80 | - report.generate_notice! | ||
| 81 | - end | ||
| 82 | - | ||
| 83 | def find_or_create_err!(attrs) | 45 | def find_or_create_err!(attrs) |
| 84 | Err.where( | 46 | Err.where( |
| 85 | :fingerprint => attrs[:fingerprint] | 47 | :fingerprint => attrs[:fingerprint] |
app/models/error_report.rb
| 1 | require 'digest/sha1' | 1 | require 'digest/sha1' |
| 2 | require 'hoptoad_notifier' | 2 | require 'hoptoad_notifier' |
| 3 | 3 | ||
| 4 | +## | ||
| 5 | +# Processes a new error report. | ||
| 6 | +# | ||
| 7 | +# Accepts a hash with the following attributes: | ||
| 8 | +# | ||
| 9 | +# * <tt>:error_class</tt> - the class of error | ||
| 10 | +# * <tt>:message</tt> - the error message | ||
| 11 | +# * <tt>:backtrace</tt> - an array of stack trace lines | ||
| 12 | +# | ||
| 13 | +# * <tt>:request</tt> - a hash of values describing the request | ||
| 14 | +# * <tt>:server_environment</tt> - a hash of values describing the server environment | ||
| 15 | +# | ||
| 16 | +# * <tt>:notifier</tt> - information to identify the source of the error report | ||
| 17 | +# | ||
| 4 | class ErrorReport | 18 | class ErrorReport |
| 5 | attr_reader :error_class, :message, :request, :server_environment, :api_key, :notifier, :user_attributes, :framework | 19 | attr_reader :error_class, :message, :request, :server_environment, :api_key, :notifier, :user_attributes, :framework |
| 6 | 20 |
lib/tasks/errbit/demo.rake
| @@ -42,15 +42,15 @@ namespace :errbit do | @@ -42,15 +42,15 @@ namespace :errbit do | ||
| 42 | 42 | ||
| 43 | errors.each do |error_template| | 43 | errors.each do |error_template| |
| 44 | rand(34).times do | 44 | rand(34).times do |
| 45 | - | ||
| 46 | - error_report = error_template.reverse_merge({ | 45 | + ErrorReport.new(error_template.reverse_merge({ |
| 46 | + :api_key => app.api_key, | ||
| 47 | :error_class => "StandardError", | 47 | :error_class => "StandardError", |
| 48 | :message => "Oops. Something went wrong!", | 48 | :message => "Oops. Something went wrong!", |
| 49 | :backtrace => random_backtrace, | 49 | :backtrace => random_backtrace, |
| 50 | :request => { | 50 | :request => { |
| 51 | - 'component' => 'main', | ||
| 52 | - 'action' => 'error' | ||
| 53 | - }, | 51 | + 'component' => 'main', |
| 52 | + 'action' => 'error' | ||
| 53 | + }, | ||
| 54 | :server_environment => {'environment-name' => Rails.env.to_s}, | 54 | :server_environment => {'environment-name' => Rails.env.to_s}, |
| 55 | :notifier => {:name => "seeds.rb"}, | 55 | :notifier => {:name => "seeds.rb"}, |
| 56 | :app_user => { | 56 | :app_user => { |
| @@ -59,9 +59,7 @@ namespace :errbit do | @@ -59,9 +59,7 @@ namespace :errbit do | ||
| 59 | :name => "John Smith", | 59 | :name => "John Smith", |
| 60 | :url => "http://www.example.com/users/jsmith" | 60 | :url => "http://www.example.com/users/jsmith" |
| 61 | } | 61 | } |
| 62 | - }) | ||
| 63 | - | ||
| 64 | - app.report_error!(error_report) | 62 | + })).generate_notice! |
| 65 | end | 63 | end |
| 66 | end | 64 | end |
| 67 | 65 |
spec/controllers/notices_controller_spec.rb
| @@ -77,21 +77,6 @@ describe NoticesController do | @@ -77,21 +77,6 @@ describe NoticesController do | ||
| 77 | expect(response.status).to eq 422 | 77 | expect(response.status).to eq 422 |
| 78 | end | 78 | end |
| 79 | end | 79 | end |
| 80 | - | ||
| 81 | - # Not relevant here, Nee to be test on App.report_error! test | ||
| 82 | - it "sends a notification email", :pending => true do | ||
| 83 | - App.should_receive(:report_error!).with(xml).and_return(notice) | ||
| 84 | - request.should_receive(:raw_post).and_return(xml) | ||
| 85 | - post :create, :format => :xml | ||
| 86 | - response.should be_success | ||
| 87 | - response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) | ||
| 88 | - response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 89 | - email = ActionMailer::Base.deliveries.last | ||
| 90 | - email.to.should include(app.watchers.first.email) | ||
| 91 | - email.subject.should include(notice.message.truncate(50)) | ||
| 92 | - email.subject.should include("[#{app.name}]") | ||
| 93 | - email.subject.should include("[#{notice.environment_name}]") | ||
| 94 | - end | ||
| 95 | end | 80 | end |
| 96 | 81 | ||
| 97 | describe "GET /locate/:id" do | 82 | describe "GET /locate/:id" do |
spec/models/app_spec.rb
| @@ -187,28 +187,6 @@ describe App do | @@ -187,28 +187,6 @@ describe App do | ||
| 187 | end | 187 | end |
| 188 | 188 | ||
| 189 | 189 | ||
| 190 | - context '#report_error!', :pending => true do | ||
| 191 | - # method delete. test need to be on spec/models/error_report | ||
| 192 | - before do | ||
| 193 | - @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
| 194 | - @app = Fabricate(:app, :api_key => 'APIKEY') | ||
| 195 | - ErrorReport.any_instance.stub(:fingerprint).and_return('fingerprintdigest') | ||
| 196 | - end | ||
| 197 | - | ||
| 198 | - it "should handle params without 'request' section" do | ||
| 199 | - xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | ||
| 200 | - lambda { App.report_error!(xml) }.should_not raise_error | ||
| 201 | - end | ||
| 202 | - | ||
| 203 | - it "should handle params with only a single line of backtrace" do | ||
| 204 | - xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read | ||
| 205 | - lambda { @notice = App.report_error!(xml) }.should_not raise_error | ||
| 206 | - @notice.backtrace_lines.length.should == 1 | ||
| 207 | - end | ||
| 208 | - | ||
| 209 | - | ||
| 210 | - end | ||
| 211 | - | ||
| 212 | describe ".find_by_api_key!" do | 190 | describe ".find_by_api_key!" do |
| 213 | it 'return the app with api_key' do | 191 | it 'return the app with api_key' do |
| 214 | app = Fabricate(:app) | 192 | app = Fabricate(:app) |
spec/models/error_report_spec.rb
| @@ -97,6 +97,22 @@ describe ErrorReport do | @@ -97,6 +97,22 @@ describe ErrorReport do | ||
| 97 | }.from(true).to(false) | 97 | }.from(true).to(false) |
| 98 | end | 98 | end |
| 99 | 99 | ||
| 100 | + context "with notification service configured" do | ||
| 101 | + before do | ||
| 102 | + app.notify_on_errs = true | ||
| 103 | + app.watchers.build(:email => 'foo@example.com') | ||
| 104 | + app.save | ||
| 105 | + end | ||
| 106 | + it 'send email' do | ||
| 107 | + notice = error_report.generate_notice! | ||
| 108 | + email = ActionMailer::Base.deliveries.last | ||
| 109 | + email.to.should include(app.watchers.first.email) | ||
| 110 | + email.subject.should include(notice.message.truncate(50)) | ||
| 111 | + email.subject.should include("[#{app.name}]") | ||
| 112 | + email.subject.should include("[#{notice.environment_name}]") | ||
| 113 | + end | ||
| 114 | + end | ||
| 115 | + | ||
| 100 | context "with xml without request section" do | 116 | context "with xml without request section" do |
| 101 | let(:xml){ | 117 | let(:xml){ |
| 102 | Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | 118 | Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read |
| @@ -159,6 +175,5 @@ describe ErrorReport do | @@ -159,6 +175,5 @@ describe ErrorReport do | ||
| 159 | end | 175 | end |
| 160 | end | 176 | end |
| 161 | 177 | ||
| 162 | - | ||
| 163 | end | 178 | end |
| 164 | end | 179 | end |