Commit 6f59092607f8cc1355bf9172f9b7e16d0f5bcd20
1 parent
4d5a1bf8
Exists in
master
and in
1 other branch
refactored notice creation process: separate Errbit
concerns (e.g. fingerprint) from notice parsing concerns. This also makes it easier to support multiple versions of the Airbrake API.
Showing
12 changed files
with
494 additions
and
181 deletions
Show diff stats
app/controllers/notices_controller.rb
| 1 | class NoticesController < ApplicationController | 1 | class NoticesController < ApplicationController |
| 2 | respond_to :xml | 2 | respond_to :xml |
| 3 | - | 3 | + |
| 4 | skip_before_filter :authenticate_user!, :only => :create | 4 | skip_before_filter :authenticate_user!, :only => :create |
| 5 | - | 5 | + |
| 6 | def create | 6 | def create |
| 7 | # params[:data] if the notice came from a GET request, raw_post if it came via POST | 7 | # params[:data] if the notice came from a GET request, raw_post if it came via POST |
| 8 | - @notice = Notice.from_xml(params[:data] || request.raw_post) | 8 | + @notice = App.report_error!(params[:data] || request.raw_post) |
| 9 | respond_with @notice | 9 | respond_with @notice |
| 10 | end | 10 | end |
| 11 | - | 11 | + |
| 12 | end | 12 | end |
app/models/app.rb
| @@ -42,6 +42,47 @@ class App | @@ -42,6 +42,47 @@ class App | ||
| 42 | 42 | ||
| 43 | 43 | ||
| 44 | 44 | ||
| 45 | + # Processes a new error report. | ||
| 46 | + # | ||
| 47 | + # Accepts either XML or a hash with the following attributes: | ||
| 48 | + # | ||
| 49 | + # * <tt>:klass</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 | + | ||
| 66 | + # Processes a new error report. | ||
| 67 | + # | ||
| 68 | + # Accepts a hash with the following attributes: | ||
| 69 | + # | ||
| 70 | + # * <tt>:klass</tt> - the class of error | ||
| 71 | + # * <tt>:message</tt> - the error message | ||
| 72 | + # * <tt>:backtrace</tt> - an array of stack trace lines | ||
| 73 | + # | ||
| 74 | + # * <tt>:request</tt> - a hash of values describing the request | ||
| 75 | + # * <tt>:server_environment</tt> - a hash of values describing the server environment | ||
| 76 | + # | ||
| 77 | + # * <tt>:notifier</tt> - information to identify the source of the error report | ||
| 78 | + # | ||
| 79 | + def report_error!(hash) | ||
| 80 | + report = ErrorReport.new(hash.merge(:api_key => api_key)) | ||
| 81 | + report.generate_notice! | ||
| 82 | + end | ||
| 83 | + | ||
| 84 | + | ||
| 85 | + | ||
| 45 | def find_or_create_err!(attrs) | 86 | def find_or_create_err!(attrs) |
| 46 | Err.where(attrs).first || problems.create!.errs.create!(attrs) | 87 | Err.where(attrs).first || problems.create!.errs.create!(attrs) |
| 47 | end | 88 | end |
| @@ -0,0 +1,91 @@ | @@ -0,0 +1,91 @@ | ||
| 1 | +require 'digest/md5' | ||
| 2 | + | ||
| 3 | + | ||
| 4 | +class ErrorReport | ||
| 5 | + | ||
| 6 | + | ||
| 7 | + | ||
| 8 | + def initialize(xml_or_attributes) | ||
| 9 | + @attributes = (xml_or_attributes.is_a?(String) ? Hoptoad.parse_xml!(xml_or_attributes) : xml_or_attributes).with_indifferent_access | ||
| 10 | + end | ||
| 11 | + | ||
| 12 | + | ||
| 13 | + | ||
| 14 | + def klass | ||
| 15 | + @attributes[:klass] | ||
| 16 | + end | ||
| 17 | + | ||
| 18 | + def message | ||
| 19 | + @attributes[:message] | ||
| 20 | + end | ||
| 21 | + | ||
| 22 | + def backtrace | ||
| 23 | + @attributes[:backtrace] | ||
| 24 | + end | ||
| 25 | + | ||
| 26 | + def request | ||
| 27 | + @attributes[:request] | ||
| 28 | + end | ||
| 29 | + | ||
| 30 | + def server_environment | ||
| 31 | + @attributes[:server_environment] | ||
| 32 | + end | ||
| 33 | + | ||
| 34 | + def api_key | ||
| 35 | + @attributes[:api_key] | ||
| 36 | + end | ||
| 37 | + | ||
| 38 | + def notifier | ||
| 39 | + @attributes[:notifier] | ||
| 40 | + end | ||
| 41 | + | ||
| 42 | + | ||
| 43 | + | ||
| 44 | + def fingerprint | ||
| 45 | + @fingerprint ||= ErrorReport.get_fingerprint(self) | ||
| 46 | + end | ||
| 47 | + | ||
| 48 | + def self.get_fingerprint(report) | ||
| 49 | + Digest::MD5.hexdigest(report.backtrace[0].to_s) | ||
| 50 | + end | ||
| 51 | + | ||
| 52 | + def rails_env | ||
| 53 | + server_environment['environment-name'] || 'development' | ||
| 54 | + end | ||
| 55 | + | ||
| 56 | + def component | ||
| 57 | + request['component'] || 'unknown' | ||
| 58 | + end | ||
| 59 | + | ||
| 60 | + def action | ||
| 61 | + request['action'] | ||
| 62 | + end | ||
| 63 | + | ||
| 64 | + def app | ||
| 65 | + @app ||= App.find_by_api_key!(api_key) | ||
| 66 | + end | ||
| 67 | + | ||
| 68 | + | ||
| 69 | + | ||
| 70 | + def generate_notice! | ||
| 71 | + notice = Notice.new( | ||
| 72 | + :message => message, | ||
| 73 | + :backtrace => backtrace, | ||
| 74 | + :request => request, | ||
| 75 | + :server_environment => server_environment, | ||
| 76 | + :notifier => notifier) | ||
| 77 | + | ||
| 78 | + err = app.find_or_create_err!( | ||
| 79 | + :klass => klass, | ||
| 80 | + :component => component, | ||
| 81 | + :action => action, | ||
| 82 | + :environment => rails_env, | ||
| 83 | + :fingerprint => fingerprint) | ||
| 84 | + | ||
| 85 | + err.notices << notice | ||
| 86 | + notice | ||
| 87 | + end | ||
| 88 | + | ||
| 89 | + | ||
| 90 | + | ||
| 91 | +end |
app/models/notice.rb
| @@ -16,8 +16,8 @@ class Notice | @@ -16,8 +16,8 @@ class Notice | ||
| 16 | index :err_id | 16 | index :err_id |
| 17 | index :created_at | 17 | index :created_at |
| 18 | 18 | ||
| 19 | + after_create :increase_counter_cache, :cache_attributes_on_problem, :unresolve_problem | ||
| 19 | after_create :deliver_notification, :if => :should_notify? | 20 | after_create :deliver_notification, :if => :should_notify? |
| 20 | - after_create :increase_counter_cache, :cache_attributes_on_problem | ||
| 21 | before_save :sanitize | 21 | before_save :sanitize |
| 22 | before_destroy :decrease_counter_cache | 22 | before_destroy :decrease_counter_cache |
| 23 | 23 | ||
| @@ -30,34 +30,6 @@ class Notice | @@ -30,34 +30,6 @@ class Notice | ||
| 30 | 30 | ||
| 31 | 31 | ||
| 32 | 32 | ||
| 33 | - def self.from_xml(hoptoad_xml) | ||
| 34 | - hoptoad_notice = Hoptoad::V2.parse_xml(hoptoad_xml) | ||
| 35 | - app = App.find_by_api_key!(hoptoad_notice['api-key']) | ||
| 36 | - | ||
| 37 | - hoptoad_notice['request'] ||= {} | ||
| 38 | - hoptoad_notice['request']['component'] = 'unknown' if hoptoad_notice['request']['component'].blank? | ||
| 39 | - hoptoad_notice['request']['action'] = nil if hoptoad_notice['request']['action'].blank? | ||
| 40 | - | ||
| 41 | - err = app.find_or_create_err!({ | ||
| 42 | - :klass => hoptoad_notice['error']['class'], | ||
| 43 | - :component => hoptoad_notice['request']['component'], | ||
| 44 | - :action => hoptoad_notice['request']['action'], | ||
| 45 | - :environment => hoptoad_notice['server-environment']['environment-name'], | ||
| 46 | - :fingerprint => hoptoad_notice['fingerprint'] | ||
| 47 | - }) | ||
| 48 | - err.problem.update_attributes(:resolved => false) if err.problem.resolved? | ||
| 49 | - err.notices.create!({ | ||
| 50 | - :klass => hoptoad_notice['error']['class'], | ||
| 51 | - :message => hoptoad_notice['error']['message'], | ||
| 52 | - :backtrace => [hoptoad_notice['error']['backtrace']['line']].flatten, | ||
| 53 | - :server_environment => hoptoad_notice['server-environment'], | ||
| 54 | - :request => hoptoad_notice['request'], | ||
| 55 | - :notifier => hoptoad_notice['notifier'] | ||
| 56 | - }) | ||
| 57 | - end | ||
| 58 | - | ||
| 59 | - | ||
| 60 | - | ||
| 61 | def user_agent | 33 | def user_agent |
| 62 | agent_string = env_vars['HTTP_USER_AGENT'] | 34 | agent_string = env_vars['HTTP_USER_AGENT'] |
| 63 | agent_string.blank? ? nil : UserAgent.parse(agent_string) | 35 | agent_string.blank? ? nil : UserAgent.parse(agent_string) |
| @@ -127,7 +99,7 @@ protected | @@ -127,7 +99,7 @@ protected | ||
| 127 | 99 | ||
| 128 | 100 | ||
| 129 | def should_notify? | 101 | def should_notify? |
| 130 | - app.notify_on_errs? && (Errbit::Config.per_app_email_at_notices && app.email_at_notices || Errbit::Config.email_at_notices).include?(err.notices.count) && app.watchers.any? | 102 | + app.notify_on_errs? && (Errbit::Config.per_app_email_at_notices && app.email_at_notices || Errbit::Config.email_at_notices).include?(problem.notices_count) && app.watchers.any? |
| 131 | end | 103 | end |
| 132 | 104 | ||
| 133 | 105 | ||
| @@ -141,6 +113,11 @@ protected | @@ -141,6 +113,11 @@ protected | ||
| 141 | end | 113 | end |
| 142 | 114 | ||
| 143 | 115 | ||
| 116 | + def unresolve_problem | ||
| 117 | + problem.update_attribute(:resolved, false) if problem.resolved? | ||
| 118 | + end | ||
| 119 | + | ||
| 120 | + | ||
| 144 | def cache_attributes_on_problem | 121 | def cache_attributes_on_problem |
| 145 | problem.cache_notice_attributes(self) if problem.notices_count == 1 | 122 | problem.cache_notice_attributes(self) if problem.notices_count == 1 |
| 146 | end | 123 | end |
| @@ -0,0 +1 @@ | @@ -0,0 +1 @@ | ||
| 1 | +require 'core_ext/hash' |
| @@ -0,0 +1,61 @@ | @@ -0,0 +1,61 @@ | ||
| 1 | +module CoreExt | ||
| 2 | + module Hash | ||
| 3 | + | ||
| 4 | + | ||
| 5 | + | ||
| 6 | + def pick(*picks) | ||
| 7 | + picks = picks.flatten | ||
| 8 | + picks.inject({}) {|result, key| self.key?(key) ? result.merge(key => self[key]) : result} | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + | ||
| 12 | + | ||
| 13 | + def pick!(*picks) | ||
| 14 | + picks = picks.flatten | ||
| 15 | + keys.each {|key| self.delete(key) unless picks.member?(key) } | ||
| 16 | + end | ||
| 17 | + | ||
| 18 | + | ||
| 19 | + | ||
| 20 | + def except(*picks) | ||
| 21 | + result = self.dup | ||
| 22 | + result.except!(*picks) | ||
| 23 | + result | ||
| 24 | + end | ||
| 25 | + | ||
| 26 | + | ||
| 27 | + | ||
| 28 | + def except!(*picks) | ||
| 29 | + picks = picks.flatten | ||
| 30 | + keys.each {|key| self.delete(key) if picks.member?(key) } | ||
| 31 | + end | ||
| 32 | + | ||
| 33 | + | ||
| 34 | + | ||
| 35 | + def inspect!(depth=0) | ||
| 36 | + s = "" | ||
| 37 | + self.each do |k,v| | ||
| 38 | + s << (" " * depth) | ||
| 39 | + s << k | ||
| 40 | + s << ": " | ||
| 41 | + if v.is_a?(Hash) | ||
| 42 | + s << "{\n" | ||
| 43 | + s << v.inspect!(depth + 2) | ||
| 44 | + s << (" " * depth) | ||
| 45 | + s << "}" | ||
| 46 | + elsif v.is_a?(Array) | ||
| 47 | + s << v.inspect | ||
| 48 | + else | ||
| 49 | + s << v.to_s | ||
| 50 | + end | ||
| 51 | + s << "\n" | ||
| 52 | + end | ||
| 53 | + s | ||
| 54 | + end | ||
| 55 | + | ||
| 56 | + | ||
| 57 | + | ||
| 58 | + end | ||
| 59 | +end | ||
| 60 | + | ||
| 61 | +Hash.send :include, CoreExt::Hash | ||
| 0 | \ No newline at end of file | 62 | \ No newline at end of file |
lib/hoptoad.rb
| 1 | -module Hoptoad | ||
| 2 | - module V2 | ||
| 3 | - require 'digest/md5' | 1 | +require 'hoptoad/v2' |
| 4 | 2 | ||
| 5 | - class ApiVersionError < StandardError | ||
| 6 | - def initialize(version) | ||
| 7 | - super "Wrong API Version: Expecting v2.0, got version: #{version}" | ||
| 8 | - end | ||
| 9 | - end | ||
| 10 | 3 | ||
| 11 | - def self.parse_xml(xml) | ||
| 12 | - xml = xml.unpack('C*').pack('U*') # Repack string into Unicode to fix invalid UTF-8 chars | ||
| 13 | - parsed = ActiveSupport::XmlMini.backend.parse(xml)['notice'] | ||
| 14 | - raise ApiVersionError.new(parsed['version']) unless parsed && parsed['version'].to_s == '2.0' | ||
| 15 | - rekeyed = rekey(parsed) | ||
| 16 | - rekeyed['fingerprint'] = Digest::MD5.hexdigest(rekeyed['error']['backtrace'].to_s) | ||
| 17 | - rekeyed | 4 | +module Hoptoad |
| 5 | + | ||
| 6 | + | ||
| 7 | + def self.parse_xml!(xml) | ||
| 8 | + xml = xml.unpack('C*').pack('U*') # Repack string into Unicode to fix invalid UTF-8 chars | ||
| 9 | + parsed = ActiveSupport::XmlMini.backend.parse(xml)['notice'] || raise(ApiVersionError) | ||
| 10 | + processor = get_version_processor(parsed['version']) | ||
| 11 | + processor.process_notice(parsed) | ||
| 12 | + end | ||
| 13 | + | ||
| 14 | + | ||
| 15 | +private | ||
| 16 | + | ||
| 17 | + | ||
| 18 | + def self.get_version_processor(version) | ||
| 19 | + case version | ||
| 20 | + when '2.0'; Hoptoad::V2 | ||
| 21 | + else; raise ApiVersionError | ||
| 22 | + end | ||
| 23 | + end | ||
| 24 | + | ||
| 25 | + | ||
| 26 | +public | ||
| 27 | + | ||
| 28 | + | ||
| 29 | + class ApiVersionError < StandardError | ||
| 30 | + def initialize | ||
| 31 | + super "Wrong API Version: Expecting v2.0" | ||
| 18 | end | 32 | end |
| 19 | - | ||
| 20 | - private | ||
| 21 | - | ||
| 22 | - def self.rekey(node) | ||
| 23 | - if node.is_a?(Hash) && node.has_key?('var') && node.has_key?('key') | ||
| 24 | - {node['key'] => rekey(node['var'])} | ||
| 25 | - elsif node.is_a?(Hash) && node.has_key?('var') | ||
| 26 | - rekey(node['var']) | ||
| 27 | - elsif node.is_a?(Hash) && node.has_key?('__content__') && node.has_key?('key') | ||
| 28 | - {node['key'] => node['__content__']} | ||
| 29 | - elsif node.is_a?(Hash) && node.has_key?('__content__') | ||
| 30 | - node['__content__'] | ||
| 31 | - elsif node.is_a?(Hash) | ||
| 32 | - node.inject({}) {|rekeyed, (key,val)| | ||
| 33 | - rekeyed.merge(key => rekey(val)) | ||
| 34 | - } | ||
| 35 | - elsif node.is_a?(Array) && node.first.has_key?('key') | ||
| 36 | - node.inject({}) {|rekeyed,keypair| | ||
| 37 | - rekeyed.merge(rekey(keypair)) | ||
| 38 | - } | ||
| 39 | - elsif node.is_a?(Array) | ||
| 40 | - node.map {|n| rekey(n)} | ||
| 41 | - else | ||
| 42 | - node | ||
| 43 | - end | ||
| 44 | - end | ||
| 45 | end | 33 | end |
| 34 | + | ||
| 35 | + | ||
| 46 | end | 36 | end |
| 47 | - |
| @@ -0,0 +1,78 @@ | @@ -0,0 +1,78 @@ | ||
| 1 | +module Hoptoad | ||
| 2 | + module V2 | ||
| 3 | + | ||
| 4 | + | ||
| 5 | + def self.process_notice(parsed) | ||
| 6 | + for_errbit_api( | ||
| 7 | + normalize( | ||
| 8 | + rekey(parsed))) | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + | ||
| 12 | + private | ||
| 13 | + | ||
| 14 | + | ||
| 15 | + def self.rekey(node) | ||
| 16 | + case node | ||
| 17 | + when Hash | ||
| 18 | + if node.has_key?('var') && node.has_key?('key') | ||
| 19 | + {normalize_key(node['key']) => rekey(node['var'])} | ||
| 20 | + elsif node.has_key?('var') | ||
| 21 | + rekey(node['var']) | ||
| 22 | + elsif node.has_key?('__content__') && node.has_key?('key') | ||
| 23 | + {normalize_key(node['key']) => rekey(node['__content__'])} | ||
| 24 | + elsif node.has_key?('__content__') | ||
| 25 | + rekey(node['__content__']) | ||
| 26 | + else | ||
| 27 | + node.inject({}) {|rekeyed, (key, val)| rekeyed.merge(normalize_key(key) => rekey(val))} | ||
| 28 | + end | ||
| 29 | + | ||
| 30 | + when Array | ||
| 31 | + if node.first.has_key?('key') | ||
| 32 | + node.inject({}) {|rekeyed, keypair| rekeyed.merge(rekey(keypair))} | ||
| 33 | + else | ||
| 34 | + node.map {|n| rekey(n)} | ||
| 35 | + end | ||
| 36 | + | ||
| 37 | + else | ||
| 38 | + node | ||
| 39 | + | ||
| 40 | + end | ||
| 41 | + end | ||
| 42 | + | ||
| 43 | + | ||
| 44 | + def self.normalize_key(key) | ||
| 45 | + key.gsub('.', '_') | ||
| 46 | + end | ||
| 47 | + | ||
| 48 | + | ||
| 49 | + def self.normalize(notice) | ||
| 50 | + error = notice['error'] | ||
| 51 | + backtrace = error['backtrace'] | ||
| 52 | + backtrace['line'] = [backtrace['line']] unless backtrace['line'].is_a?(Array) | ||
| 53 | + | ||
| 54 | + notice['request'] ||= {} | ||
| 55 | + notice['request']['component'] = 'unknown' if notice['request']['component'].blank? | ||
| 56 | + notice['request']['action'] = nil if notice['request']['action'].blank? | ||
| 57 | + | ||
| 58 | + notice | ||
| 59 | + end | ||
| 60 | + | ||
| 61 | + | ||
| 62 | + def self.for_errbit_api(notice) | ||
| 63 | + { | ||
| 64 | + :klass => notice['error']['class'], | ||
| 65 | + :message => notice['error']['message'], | ||
| 66 | + :backtrace => notice['error']['backtrace']['line'], | ||
| 67 | + | ||
| 68 | + :request => notice['request'], | ||
| 69 | + :server_environment => notice['server-environment'], | ||
| 70 | + | ||
| 71 | + :api_key => notice['api-key'], | ||
| 72 | + :notifier => notice['notifier'] | ||
| 73 | + } | ||
| 74 | + end | ||
| 75 | + | ||
| 76 | + | ||
| 77 | + end | ||
| 78 | +end | ||
| 0 | \ No newline at end of file | 79 | \ No newline at end of file |
spec/controllers/notices_controller_spec.rb
| 1 | require 'spec_helper' | 1 | require 'spec_helper' |
| 2 | 2 | ||
| 3 | describe NoticesController do | 3 | describe NoticesController do |
| 4 | - | 4 | + |
| 5 | context 'notices API' do | 5 | context 'notices API' do |
| 6 | before do | 6 | before do |
| 7 | @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | 7 | @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read |
| 8 | @app = Factory(:app_with_watcher) | 8 | @app = Factory(:app_with_watcher) |
| 9 | App.stub(:find_by_api_key!).and_return(@app) | 9 | App.stub(:find_by_api_key!).and_return(@app) |
| 10 | - @notice = Notice.from_xml(@xml) | ||
| 11 | - | 10 | + @notice = App.report_error!(@xml) |
| 11 | + | ||
| 12 | request.env['Content-type'] = 'text/xml' | 12 | request.env['Content-type'] = 'text/xml' |
| 13 | request.env['Accept'] = 'text/xml, application/xml' | 13 | request.env['Accept'] = 'text/xml, application/xml' |
| 14 | end | 14 | end |
| 15 | - | 15 | + |
| 16 | it "generates a notice from xml [POST]" do | 16 | it "generates a notice from xml [POST]" do |
| 17 | - Notice.should_receive(:from_xml).with(@xml).and_return(@notice) | 17 | + App.should_receive(:report_error!).with(@xml).and_return(@notice) |
| 18 | request.should_receive(:raw_post).and_return(@xml) | 18 | request.should_receive(:raw_post).and_return(@xml) |
| 19 | post :create | 19 | post :create |
| 20 | end | 20 | end |
| 21 | - | 21 | + |
| 22 | it "generates a notice from xml [GET]" do | 22 | it "generates a notice from xml [GET]" do |
| 23 | - Notice.should_receive(:from_xml).with(@xml).and_return(@notice) | 23 | + App.should_receive(:report_error!).with(@xml).and_return(@notice) |
| 24 | get :create, {:data => @xml} | 24 | get :create, {:data => @xml} |
| 25 | end | 25 | end |
| 26 | - | 26 | + |
| 27 | it "sends a notification email" do | 27 | it "sends a notification email" do |
| 28 | request.should_receive(:raw_post).and_return(@xml) | 28 | request.should_receive(:raw_post).and_return(@xml) |
| 29 | post :create | 29 | post :create |
| @@ -34,5 +34,5 @@ describe NoticesController do | @@ -34,5 +34,5 @@ describe NoticesController do | ||
| 34 | email.subject.should include("[#{@notice.environment_name}]") | 34 | email.subject.should include("[#{@notice.environment_name}]") |
| 35 | end | 35 | end |
| 36 | end | 36 | end |
| 37 | - | 37 | + |
| 38 | end | 38 | end |
spec/fixtures/hoptoad_test_notice_with_one_line_of_backtrace.xml
0 → 100644
| @@ -0,0 +1,75 @@ | @@ -0,0 +1,75 @@ | ||
| 1 | +<?xml version="1.0" encoding="UTF-8"?> | ||
| 2 | +<notice version="2.0"> | ||
| 3 | + <api-key>APIKEY</api-key> | ||
| 4 | + <notifier> | ||
| 5 | + <name>Hoptoad Notifier</name> | ||
| 6 | + <version>2.3.2</version> | ||
| 7 | + <url>http://hoptoadapp.com</url> | ||
| 8 | + </notifier> | ||
| 9 | + <error> | ||
| 10 | + <class>HoptoadTestingException</class> | ||
| 11 | + <message>HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.</message> | ||
| 12 | + <backtrace> | ||
| 13 | + <line number="425" file="[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb" method="_run__2115867319__process_action__262109504__callbacks"/> | ||
| 14 | + </backtrace> | ||
| 15 | + </error> | ||
| 16 | + <request> | ||
| 17 | + <url>http://example.org/verify</url> | ||
| 18 | + <component>application</component> | ||
| 19 | + <action>verify</action> | ||
| 20 | + <params> | ||
| 21 | + <var key="action">verify</var> | ||
| 22 | + <var key="controller">application</var> | ||
| 23 | + </params> | ||
| 24 | + <cgi-data> | ||
| 25 | + <var key="rack.session"/> | ||
| 26 | + <var key="action_dispatch.request.formats">text/html</var> | ||
| 27 | + <var key="action_dispatch.request.parameters"> | ||
| 28 | + <var key="action">verify</var> | ||
| 29 | + <var key="controller">application</var> | ||
| 30 | + </var> | ||
| 31 | + <var key="SERVER_NAME">example.org</var> | ||
| 32 | + <var key="rack.url_scheme">http</var> | ||
| 33 | + <var key="action_dispatch.remote_ip"/> | ||
| 34 | + <var key="CONTENT_LENGTH">0</var> | ||
| 35 | + <var key="rack.errors">#<StringIO:0x103d9dec0></var> | ||
| 36 | + <var key="action_dispatch.request.unsigned_session_cookie"/> | ||
| 37 | + <var key="action_dispatch.request.query_parameters"/> | ||
| 38 | + <var key="HTTPS">off</var> | ||
| 39 | + <var key="rack.run_once">false</var> | ||
| 40 | + <var key="PATH_INFO">/verify</var> | ||
| 41 | + <var key="action_dispatch.secret_token">994f235e3372684bc736dd11842b754d2ddcffc8c2958d33a29527c3217becd6655fa4653a318bc7c34131f9baf2acc0c424ed07e48e0e5e87c6cd34d711e985</var> | ||
| 42 | + <var key="rack.version">11</var> | ||
| 43 | + <var key="SCRIPT_NAME"/> | ||
| 44 | + <var key="action_dispatch.request.path_parameters"> | ||
| 45 | + <var key="action">verify</var> | ||
| 46 | + <var key="controller">application</var> | ||
| 47 | + </var> | ||
| 48 | + <var key="rack.multithread">false</var> | ||
| 49 | + <var key="action_dispatch.parameter_filter">password</var> | ||
| 50 | + <var key="action_dispatch.cookies"/> | ||
| 51 | + <var key="action_dispatch.request.request_parameters"/> | ||
| 52 | + <var key="rack.multiprocess">true</var> | ||
| 53 | + <var key="rack.request.query_hash"/> | ||
| 54 | + <var key="SERVER_PORT">80</var> | ||
| 55 | + <var key="REQUEST_METHOD">GET</var> | ||
| 56 | + <var key="action_controller.instance">#<ApplicationController:0x103d2f560></var> | ||
| 57 | + <var key="rack.session.options"> | ||
| 58 | + <var key="secure">false</var> | ||
| 59 | + <var key="httponly">true</var> | ||
| 60 | + <var key="path">/</var> | ||
| 61 | + <var key="expire_after"/> | ||
| 62 | + <var key="domain"/> | ||
| 63 | + <var key="id"/> | ||
| 64 | + </var> | ||
| 65 | + <var key="rack.input">#<StringIO:0x103d9dc90></var> | ||
| 66 | + <var key="action_dispatch.request.content_type"/> | ||
| 67 | + <var key="rack.request.query_string"/> | ||
| 68 | + <var key="QUERY_STRING"/> | ||
| 69 | + </cgi-data> | ||
| 70 | + </request> | ||
| 71 | + <server-environment> | ||
| 72 | + <project-root>/path/to/sample/project</project-root> | ||
| 73 | + <environment-name>development</environment-name> | ||
| 74 | + </server-environment> | ||
| 75 | +</notice> | ||
| 0 | \ No newline at end of file | 76 | \ No newline at end of file |
spec/models/app_spec.rb
| @@ -145,4 +145,94 @@ describe App do | @@ -145,4 +145,94 @@ describe App do | ||
| 145 | end | 145 | end |
| 146 | 146 | ||
| 147 | 147 | ||
| 148 | + context '#report_error!' do | ||
| 149 | + before do | ||
| 150 | + @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
| 151 | + @app = Factory(:app, :api_key => 'APIKEY') | ||
| 152 | + ErrorReport.stub(:get_fingerprint).and_return('fingerprintdigest') | ||
| 153 | + end | ||
| 154 | + | ||
| 155 | + it 'finds the correct app' do | ||
| 156 | + @notice = App.report_error!(@xml) | ||
| 157 | + @notice.err.app.should == @app | ||
| 158 | + end | ||
| 159 | + | ||
| 160 | + it 'finds the correct err for the notice' do | ||
| 161 | + App.should_receive(:find_by_api_key!).and_return(@app) | ||
| 162 | + @app.should_receive(:find_or_create_err!).with({ | ||
| 163 | + :klass => 'HoptoadTestingException', | ||
| 164 | + :component => 'application', | ||
| 165 | + :action => 'verify', | ||
| 166 | + :environment => 'development', | ||
| 167 | + :fingerprint => 'fingerprintdigest' | ||
| 168 | + }).and_return(err = Factory(:err)) | ||
| 169 | + err.notices.stub(:create!) | ||
| 170 | + @notice = App.report_error!(@xml) | ||
| 171 | + end | ||
| 172 | + | ||
| 173 | + it 'marks the err as unresolved if it was previously resolved' do | ||
| 174 | + App.should_receive(:find_by_api_key!).and_return(@app) | ||
| 175 | + @app.should_receive(:find_or_create_err!).with({ | ||
| 176 | + :klass => 'HoptoadTestingException', | ||
| 177 | + :component => 'application', | ||
| 178 | + :action => 'verify', | ||
| 179 | + :environment => 'development', | ||
| 180 | + :fingerprint => 'fingerprintdigest' | ||
| 181 | + }).and_return(err = Factory(:err, :problem => Factory(:problem, :resolved => true))) | ||
| 182 | + err.should be_resolved | ||
| 183 | + @notice = App.report_error!(@xml) | ||
| 184 | + @notice.err.should == err | ||
| 185 | + @notice.err.should_not be_resolved | ||
| 186 | + end | ||
| 187 | + | ||
| 188 | + it 'should create a new notice' do | ||
| 189 | + @notice = App.report_error!(@xml) | ||
| 190 | + @notice.should be_persisted | ||
| 191 | + end | ||
| 192 | + | ||
| 193 | + it 'assigns an err to the notice' do | ||
| 194 | + @notice = App.report_error!(@xml) | ||
| 195 | + @notice.err.should be_a(Err) | ||
| 196 | + end | ||
| 197 | + | ||
| 198 | + it 'captures the err message' do | ||
| 199 | + @notice = App.report_error!(@xml) | ||
| 200 | + @notice.message.should == 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' | ||
| 201 | + end | ||
| 202 | + | ||
| 203 | + it 'captures the backtrace' do | ||
| 204 | + @notice = App.report_error!(@xml) | ||
| 205 | + @notice.backtrace.size.should == 73 | ||
| 206 | + @notice.backtrace.last['file'].should == '[GEM_ROOT]/bin/rake' | ||
| 207 | + end | ||
| 208 | + | ||
| 209 | + it 'captures the server_environment' do | ||
| 210 | + @notice = App.report_error!(@xml) | ||
| 211 | + @notice.server_environment['environment-name'].should == 'development' | ||
| 212 | + end | ||
| 213 | + | ||
| 214 | + it 'captures the request' do | ||
| 215 | + @notice = App.report_error!(@xml) | ||
| 216 | + @notice.request['url'].should == 'http://example.org/verify' | ||
| 217 | + @notice.request['params']['controller'].should == 'application' | ||
| 218 | + end | ||
| 219 | + | ||
| 220 | + it 'captures the notifier' do | ||
| 221 | + @notice = App.report_error!(@xml) | ||
| 222 | + @notice.notifier['name'].should == 'Hoptoad Notifier' | ||
| 223 | + end | ||
| 224 | + | ||
| 225 | + it "should handle params without 'request' section" do | ||
| 226 | + xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | ||
| 227 | + lambda { App.report_error!(xml) }.should_not raise_error | ||
| 228 | + end | ||
| 229 | + | ||
| 230 | + it "should handle params with only a single line of backtrace" do | ||
| 231 | + xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read | ||
| 232 | + lambda { @notice = App.report_error!(xml) }.should_not raise_error | ||
| 233 | + @notice.backtrace.length.should == 1 | ||
| 234 | + end | ||
| 235 | + end | ||
| 236 | + | ||
| 237 | + | ||
| 148 | end | 238 | end |
spec/models/notice_spec.rb
| @@ -39,111 +39,21 @@ describe Notice do | @@ -39,111 +39,21 @@ describe Notice do | ||
| 39 | 'method' => ActiveSupport.methods.shuffle.first | 39 | 'method' => ActiveSupport.methods.shuffle.first |
| 40 | }] | 40 | }] |
| 41 | end | 41 | end |
| 42 | - | 42 | + |
| 43 | it "should be false for line not starting with PROJECT_ROOT" do | 43 | it "should be false for line not starting with PROJECT_ROOT" do |
| 44 | Notice.in_app_backtrace_line?(backtrace[0]).should == false | 44 | Notice.in_app_backtrace_line?(backtrace[0]).should == false |
| 45 | end | 45 | end |
| 46 | - | 46 | + |
| 47 | it "should be false for file in vendor dir" do | 47 | it "should be false for file in vendor dir" do |
| 48 | Notice.in_app_backtrace_line?(backtrace[1]).should == false | 48 | Notice.in_app_backtrace_line?(backtrace[1]).should == false |
| 49 | end | 49 | end |
| 50 | - | 50 | + |
| 51 | it "should be true for application file" do | 51 | it "should be true for application file" do |
| 52 | Notice.in_app_backtrace_line?(backtrace[2]).should == true | 52 | Notice.in_app_backtrace_line?(backtrace[2]).should == true |
| 53 | end | 53 | end |
| 54 | end | 54 | end |
| 55 | 55 | ||
| 56 | 56 | ||
| 57 | - context '#from_xml' do | ||
| 58 | - before do | ||
| 59 | - @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
| 60 | - @app = Factory(:app, :api_key => 'APIKEY') | ||
| 61 | - Digest::MD5.stub(:hexdigest).and_return('fingerprintdigest') | ||
| 62 | - end | ||
| 63 | - | ||
| 64 | - it 'finds the correct app' do | ||
| 65 | - @notice = Notice.from_xml(@xml) | ||
| 66 | - @notice.err.app.should == @app | ||
| 67 | - end | ||
| 68 | - | ||
| 69 | - it 'finds the correct err for the notice' do | ||
| 70 | - App.should_receive(:find_by_api_key!).and_return(@app) | ||
| 71 | - @app.should_receive(:find_or_create_err!).with({ | ||
| 72 | - :klass => 'HoptoadTestingException', | ||
| 73 | - :component => 'application', | ||
| 74 | - :action => 'verify', | ||
| 75 | - :environment => 'development', | ||
| 76 | - :fingerprint => 'fingerprintdigest' | ||
| 77 | - }).and_return(err = Factory(:err)) | ||
| 78 | - err.notices.stub(:create!) | ||
| 79 | - @notice = Notice.from_xml(@xml) | ||
| 80 | - end | ||
| 81 | - | ||
| 82 | - | ||
| 83 | - it 'marks the err as unresolved if it was previously resolved' do | ||
| 84 | - App.should_receive(:find_by_api_key!).and_return(@app) | ||
| 85 | - @app.should_receive(:find_or_create_err!).with({ | ||
| 86 | - :klass => 'HoptoadTestingException', | ||
| 87 | - :component => 'application', | ||
| 88 | - :action => 'verify', | ||
| 89 | - :environment => 'development', | ||
| 90 | - :fingerprint => 'fingerprintdigest' | ||
| 91 | - }).and_return(err = Factory(:err, :problem => Factory(:problem, :resolved => true))) | ||
| 92 | - err.should be_resolved | ||
| 93 | - @notice = Notice.from_xml(@xml) | ||
| 94 | - @notice.err.should == err | ||
| 95 | - @notice.err.should_not be_resolved | ||
| 96 | - end | ||
| 97 | - | ||
| 98 | - it 'should create a new notice' do | ||
| 99 | - @notice = Notice.from_xml(@xml) | ||
| 100 | - @notice.should be_persisted | ||
| 101 | - end | ||
| 102 | - | ||
| 103 | - it 'assigns an err to the notice' do | ||
| 104 | - @notice = Notice.from_xml(@xml) | ||
| 105 | - @notice.err.should be_a(Err) | ||
| 106 | - end | ||
| 107 | - | ||
| 108 | - it 'captures the err message' do | ||
| 109 | - @notice = Notice.from_xml(@xml) | ||
| 110 | - @notice.message.should == 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' | ||
| 111 | - end | ||
| 112 | - | ||
| 113 | - it 'captures the backtrace' do | ||
| 114 | - @notice = Notice.from_xml(@xml) | ||
| 115 | - @notice.backtrace.size.should == 73 | ||
| 116 | - @notice.backtrace.last['file'].should == '[GEM_ROOT]/bin/rake' | ||
| 117 | - end | ||
| 118 | - | ||
| 119 | - it 'captures the server_environment' do | ||
| 120 | - @notice = Notice.from_xml(@xml) | ||
| 121 | - @notice.server_environment['environment-name'].should == 'development' | ||
| 122 | - end | ||
| 123 | - | ||
| 124 | - it 'captures the request' do | ||
| 125 | - @notice = Notice.from_xml(@xml) | ||
| 126 | - @notice.request['url'].should == 'http://example.org/verify' | ||
| 127 | - @notice.request['params']['controller'].should == 'application' | ||
| 128 | - end | ||
| 129 | - | ||
| 130 | - it 'captures the notifier' do | ||
| 131 | - @notice = Notice.from_xml(@xml) | ||
| 132 | - @notice.notifier['name'].should == 'Hoptoad Notifier' | ||
| 133 | - end | ||
| 134 | - | ||
| 135 | - it "should handle params withour 'request' section" do | ||
| 136 | - @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | ||
| 137 | - lambda { Notice.from_xml(@xml) }.should_not raise_error | ||
| 138 | - end | ||
| 139 | - | ||
| 140 | - it "should raise ApiVersionError" do | ||
| 141 | - @xml = Rails.root.join('spec', 'fixtures', 'hoptoad_test_notice_with_wrong_version.xml').read | ||
| 142 | - expect { Notice.from_xml(@xml) }.to raise_error(Hoptoad::V2::ApiVersionError) | ||
| 143 | - end | ||
| 144 | - end | ||
| 145 | - | ||
| 146 | - | ||
| 147 | describe "key sanitization" do | 57 | describe "key sanitization" do |
| 148 | before do | 58 | before do |
| 149 | @hash = { "some.key" => { "$nested.key" => {"$Path" => "/", "some$key" => "key"}}} | 59 | @hash = { "some.key" => { "$nested.key" => {"$Path" => "/", "some$key" => "key"}}} |
| @@ -179,7 +89,7 @@ describe Notice do | @@ -179,7 +89,7 @@ describe Notice do | ||
| 179 | before do | 89 | before do |
| 180 | Errbit::Config.per_app_email_at_notices = true | 90 | Errbit::Config.per_app_email_at_notices = true |
| 181 | @app = Factory(:app_with_watcher, :email_at_notices => custom_thresholds) | 91 | @app = Factory(:app_with_watcher, :email_at_notices => custom_thresholds) |
| 182 | - @problem = Factory(:err, :problem => @app.problems.create!) | 92 | + @err = Factory(:err, :problem => Factory(:problem, :app => @app)) |
| 183 | end | 93 | end |
| 184 | 94 | ||
| 185 | after do | 95 | after do |
| @@ -188,10 +98,10 @@ describe Notice do | @@ -188,10 +98,10 @@ describe Notice do | ||
| 188 | 98 | ||
| 189 | custom_thresholds.each do |threshold| | 99 | custom_thresholds.each do |threshold| |
| 190 | it "sends an email notification after #{threshold} notice(s)" do | 100 | it "sends an email notification after #{threshold} notice(s)" do |
| 191 | - @problem.notices.stub(:count).and_return(threshold) | 101 | + @err.problem.stub(:notices_count).and_return(threshold) |
| 192 | Mailer.should_receive(:err_notification). | 102 | Mailer.should_receive(:err_notification). |
| 193 | and_return(mock('email', :deliver => true)) | 103 | and_return(mock('email', :deliver => true)) |
| 194 | - Factory(:notice, :err => @problem) | 104 | + Factory(:notice, :err => @err) |
| 195 | end | 105 | end |
| 196 | end | 106 | end |
| 197 | end | 107 | end |