Commit 7f72028937f84decf962f57f014d1d2a06a8ec4b
Exists in
master
and in
1 other branch
Merge pull request #475 from shingara/features/issue_472
Return 422 HTTP status code when invalid API key
Showing
9 changed files
with
420 additions
and
219 deletions
Show diff stats
app/controllers/notices_controller.rb
| @@ -5,11 +5,17 @@ class NoticesController < ApplicationController | @@ -5,11 +5,17 @@ class NoticesController < ApplicationController | ||
| 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 = App.report_error!(params[:data] || request.raw_post) | ||
| 9 | - api_xml = notice.to_xml(:only => false, :methods => [:id]) do |xml| | ||
| 10 | - xml.url locate_url(notice.id, :host => Errbit::Config.host) | 8 | + report = ErrorReport.new(params[:data] || request.raw_post) |
| 9 | + | ||
| 10 | + if report.valid? | ||
| 11 | + report.generate_notice! | ||
| 12 | + api_xml = report.notice.to_xml(:only => false, :methods => [:id]) do |xml| | ||
| 13 | + xml.url locate_url(report.notice.id, :host => Errbit::Config.host) | ||
| 14 | + end | ||
| 15 | + render :xml => api_xml | ||
| 16 | + else | ||
| 17 | + render :text => "Your API key is unknown", :status => 422 | ||
| 11 | end | 18 | end |
| 12 | - render :xml => api_xml | ||
| 13 | end | 19 | end |
| 14 | 20 | ||
| 15 | # Redirects a notice to the problem page. Useful when using User Information at Airbrake gem. | 21 | # Redirects a notice to the problem page. Useful when using User Information at Airbrake gem. |
| @@ -17,4 +23,5 @@ class NoticesController < ApplicationController | @@ -17,4 +23,5 @@ class NoticesController < ApplicationController | ||
| 17 | problem = Notice.find(params[:id]).problem | 23 | problem = Notice.find(params[:id]).problem |
| 18 | redirect_to app_problem_path(problem.app, problem) | 24 | redirect_to app_problem_path(problem.app, problem) |
| 19 | end | 25 | end |
| 26 | + | ||
| 20 | end | 27 | end |
app/models/app.rb
| @@ -42,46 +42,11 @@ class App | @@ -42,46 +42,11 @@ 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(:fingerprint => attrs[:fingerprint]).first || problems.create!.errs.create!(attrs) | 46 | + Err.where( |
| 47 | + :fingerprint => attrs[:fingerprint] | ||
| 48 | + ).first || | ||
| 49 | + problems.create!.errs.create!(attrs) | ||
| 85 | end | 50 | end |
| 86 | 51 | ||
| 87 | # Mongoid Bug: find(id) on association proxies returns an Enumerator | 52 | # Mongoid Bug: find(id) on association proxies returns an Enumerator |
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 | ||
| @@ -26,7 +40,7 @@ class ErrorReport | @@ -26,7 +40,7 @@ class ErrorReport | ||
| 26 | end | 40 | end |
| 27 | 41 | ||
| 28 | def app | 42 | def app |
| 29 | - @app ||= App.find_by_api_key!(api_key) | 43 | + @app ||= App.where(:api_key => api_key).first |
| 30 | end | 44 | end |
| 31 | 45 | ||
| 32 | def backtrace | 46 | def backtrace |
| @@ -34,7 +48,9 @@ class ErrorReport | @@ -34,7 +48,9 @@ class ErrorReport | ||
| 34 | end | 48 | end |
| 35 | 49 | ||
| 36 | def generate_notice! | 50 | def generate_notice! |
| 37 | - notice = Notice.new( | 51 | + return unless valid? |
| 52 | + return @notice if @notice | ||
| 53 | + @notice = Notice.new( | ||
| 38 | :message => message, | 54 | :message => message, |
| 39 | :error_class => error_class, | 55 | :error_class => error_class, |
| 40 | :backtrace_id => backtrace.id, | 56 | :backtrace_id => backtrace.id, |
| @@ -44,19 +60,33 @@ class ErrorReport | @@ -44,19 +60,33 @@ class ErrorReport | ||
| 44 | :user_attributes => user_attributes, | 60 | :user_attributes => user_attributes, |
| 45 | :framework => framework | 61 | :framework => framework |
| 46 | ) | 62 | ) |
| 63 | + error.notices << @notice | ||
| 64 | + @notice | ||
| 65 | + end | ||
| 66 | + attr_reader :notice | ||
| 47 | 67 | ||
| 48 | - err = app.find_or_create_err!( | 68 | + ## |
| 69 | + # Error associate to this error_report | ||
| 70 | + # | ||
| 71 | + # Can already exist or not | ||
| 72 | + # | ||
| 73 | + # @return [ Error ] | ||
| 74 | + def error | ||
| 75 | + @error ||= app.find_or_create_err!( | ||
| 49 | :error_class => error_class, | 76 | :error_class => error_class, |
| 50 | :component => component, | 77 | :component => component, |
| 51 | :action => action, | 78 | :action => action, |
| 52 | :environment => rails_env, | 79 | :environment => rails_env, |
| 53 | - :fingerprint => fingerprint) | 80 | + :fingerprint => fingerprint |
| 81 | + ) | ||
| 82 | + end | ||
| 54 | 83 | ||
| 55 | - err.notices << notice | ||
| 56 | - notice | 84 | + def valid? |
| 85 | + !!app | ||
| 57 | end | 86 | end |
| 58 | 87 | ||
| 59 | private | 88 | private |
| 89 | + | ||
| 60 | def fingerprint_source | 90 | def fingerprint_source |
| 61 | # Find the first backtrace line with a file and line number. | 91 | # Find the first backtrace line with a file and line number. |
| 62 | if line = backtrace.lines.detect {|l| l.number.present? && l.file.present? } | 92 | if line = backtrace.lines.detect {|l| l.number.present? && l.file.present? } |
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
| @@ -3,87 +3,53 @@ require 'spec_helper' | @@ -3,87 +3,53 @@ require 'spec_helper' | ||
| 3 | describe NoticesController do | 3 | describe NoticesController do |
| 4 | it_requires_authentication :for => { :locate => :get } | 4 | it_requires_authentication :for => { :locate => :get } |
| 5 | 5 | ||
| 6 | + let(:notice) { Fabricate(:notice) } | ||
| 7 | + let(:xml) { Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read } | ||
| 6 | let(:app) { Fabricate(:app) } | 8 | let(:app) { Fabricate(:app) } |
| 9 | + let(:error_report) { mock(:valid? => true, :generate_notice! => true, :notice => notice) } | ||
| 7 | 10 | ||
| 8 | context 'notices API' do | 11 | context 'notices API' do |
| 9 | before do | 12 | before do |
| 10 | - @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
| 11 | - @app = Fabricate(:app_with_watcher) | ||
| 12 | - App.stub(:find_by_api_key!).and_return(@app) | ||
| 13 | - @notice = App.report_error!(@xml) | 13 | + ErrorReport.should_receive(:new).with(xml).and_return(error_report) |
| 14 | end | 14 | end |
| 15 | 15 | ||
| 16 | - it "generates a notice from raw xml [POST]" do | ||
| 17 | - App.should_receive(:report_error!).with(@xml).and_return(@notice) | ||
| 18 | - request.should_receive(:raw_post).and_return(@xml) | ||
| 19 | - post :create, :format => :xml | ||
| 20 | - response.should be_success | ||
| 21 | - # Same RegExp from Airbrake::Sender#send_to_airbrake (https://github.com/airbrake/airbrake/blob/master/lib/airbrake/sender.rb#L53) | ||
| 22 | - # Inspired by https://github.com/airbrake/airbrake/blob/master/test/sender_test.rb | ||
| 23 | - response.body.should match(%r{<id[^>]*>#{@notice.id}</id>}) | ||
| 24 | - response.body.should match(%r{<url[^>]*>(.+)#{locate_path(@notice.id)}</url>}) | ||
| 25 | - end | ||
| 26 | - | ||
| 27 | - it "should trnasform xml <va> tags to hashes correctly" do | ||
| 28 | - App.should_receive(:report_error!).with(@xml).and_return(@notice) | ||
| 29 | - request.should_receive(:raw_post).and_return(@xml) | ||
| 30 | - post :create, :format => :xml | 16 | + context "with xml pass in raw_port" do |
| 17 | + before do | ||
| 18 | + request.should_receive(:raw_post).and_return(xml) | ||
| 19 | + post :create, :format => :xml | ||
| 20 | + end | ||
| 31 | 21 | ||
| 32 | - # XML: <var key="SCRIPT_NAME"/> | ||
| 33 | - @notice.env_vars.should have_key('SCRIPT_NAME') | ||
| 34 | - @notice.env_vars['SCRIPT_NAME'].should be_nil # blank ends up nil | 22 | + it "generates a notice from raw xml [POST]" do |
| 23 | + response.should be_success | ||
| 24 | + # Same RegExp from Airbrake::Sender#send_to_airbrake (https://github.com/airbrake/airbrake/blob/master/lib/airbrake/sender.rb#L53) | ||
| 25 | + # Inspired by https://github.com/airbrake/airbrake/blob/master/test/sender_test.rb | ||
| 26 | + response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) | ||
| 27 | + response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 28 | + end | ||
| 35 | 29 | ||
| 36 | - # XML representation: | ||
| 37 | - # <var key="rack.session.options"> | ||
| 38 | - # <var key="secure">false</var> | ||
| 39 | - # <var key="httponly">true</var> | ||
| 40 | - # <var key="path">/</var> | ||
| 41 | - # <var key="expire_after"/> | ||
| 42 | - # <var key="domain"/> | ||
| 43 | - # <var key="id"/> | ||
| 44 | - # </var> | ||
| 45 | - expected = { | ||
| 46 | - 'secure' => 'false', | ||
| 47 | - 'httponly' => 'true', | ||
| 48 | - 'path' => '/', | ||
| 49 | - 'expire_after' => nil, | ||
| 50 | - 'domain' => nil, | ||
| 51 | - 'id' => nil | ||
| 52 | - } | ||
| 53 | - @notice.env_vars.should have_key('rack_session_options') | ||
| 54 | - @notice.env_vars['rack_session_options'].should eql(expected) | ||
| 55 | end | 30 | end |
| 56 | 31 | ||
| 57 | it "generates a notice from xml in a data param [POST]" do | 32 | it "generates a notice from xml in a data param [POST]" do |
| 58 | - App.should_receive(:report_error!).with(@xml).and_return(@notice) | ||
| 59 | - post :create, :data => @xml, :format => :xml | 33 | + post :create, :data => xml, :format => :xml |
| 60 | response.should be_success | 34 | response.should be_success |
| 61 | # Same RegExp from Airbrake::Sender#send_to_airbrake (https://github.com/airbrake/airbrake/blob/master/lib/airbrake/sender.rb#L53) | 35 | # Same RegExp from Airbrake::Sender#send_to_airbrake (https://github.com/airbrake/airbrake/blob/master/lib/airbrake/sender.rb#L53) |
| 62 | # Inspired by https://github.com/airbrake/airbrake/blob/master/test/sender_test.rb | 36 | # Inspired by https://github.com/airbrake/airbrake/blob/master/test/sender_test.rb |
| 63 | - response.body.should match(%r{<id[^>]*>#{@notice.id}</id>}) | ||
| 64 | - response.body.should match(%r{<url[^>]*>(.+)#{locate_path(@notice.id)}</url>}) | 37 | + response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) |
| 38 | + response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 65 | end | 39 | end |
| 66 | 40 | ||
| 67 | it "generates a notice from xml [GET]" do | 41 | it "generates a notice from xml [GET]" do |
| 68 | - App.should_receive(:report_error!).with(@xml).and_return(@notice) | ||
| 69 | - get :create, :data => @xml, :format => :xml | 42 | + get :create, :data => xml, :format => :xml |
| 70 | response.should be_success | 43 | response.should be_success |
| 71 | - response.body.should match(%r{<id[^>]*>#{@notice.id}</id>}) | ||
| 72 | - response.body.should match(%r{<url[^>]*>(.+)#{locate_path(@notice.id)}</url>}) | 44 | + response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) |
| 45 | + response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 73 | end | 46 | end |
| 74 | - | ||
| 75 | - it "sends a notification email" do | ||
| 76 | - App.should_receive(:report_error!).with(@xml).and_return(@notice) | ||
| 77 | - request.should_receive(:raw_post).and_return(@xml) | ||
| 78 | - post :create, :format => :xml | ||
| 79 | - response.should be_success | ||
| 80 | - response.body.should match(%r{<id[^>]*>#{@notice.id}</id>}) | ||
| 81 | - response.body.should match(%r{<url[^>]*>(.+)#{locate_path(@notice.id)}</url>}) | ||
| 82 | - email = ActionMailer::Base.deliveries.last | ||
| 83 | - email.to.should include(@app.watchers.first.email) | ||
| 84 | - email.subject.should include(@notice.message.truncate(50)) | ||
| 85 | - email.subject.should include("[#{@app.name}]") | ||
| 86 | - email.subject.should include("[#{@notice.environment_name}]") | 47 | + context "with an invalid API_KEY" do |
| 48 | + let(:error_report) { mock(:valid? => false) } | ||
| 49 | + it 'return 422' do | ||
| 50 | + post :create, :format => :xml, :data => xml | ||
| 51 | + expect(response.status).to eq 422 | ||
| 52 | + end | ||
| 87 | end | 53 | end |
| 88 | end | 54 | end |
| 89 | 55 |
spec/fixtures/hoptoad_test_notice_without_line_of_backtrace.xml
0 → 100644
| @@ -0,0 +1,73 @@ | @@ -0,0 +1,73 @@ | ||
| 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></backtrace> | ||
| 13 | + </error> | ||
| 14 | + <request> | ||
| 15 | + <url>http://example.org/verify</url> | ||
| 16 | + <component>application</component> | ||
| 17 | + <action>verify</action> | ||
| 18 | + <params> | ||
| 19 | + <var key="action">verify</var> | ||
| 20 | + <var key="controller">application</var> | ||
| 21 | + </params> | ||
| 22 | + <cgi-data> | ||
| 23 | + <var key="rack.session"/> | ||
| 24 | + <var key="action_dispatch.request.formats">text/html</var> | ||
| 25 | + <var key="action_dispatch.request.parameters"> | ||
| 26 | + <var key="action">verify</var> | ||
| 27 | + <var key="controller">application</var> | ||
| 28 | + </var> | ||
| 29 | + <var key="SERVER_NAME">example.org</var> | ||
| 30 | + <var key="rack.url_scheme">http</var> | ||
| 31 | + <var key="action_dispatch.remote_ip"/> | ||
| 32 | + <var key="CONTENT_LENGTH">0</var> | ||
| 33 | + <var key="rack.errors">#<StringIO:0x103d9dec0></var> | ||
| 34 | + <var key="action_dispatch.request.unsigned_session_cookie"/> | ||
| 35 | + <var key="action_dispatch.request.query_parameters"/> | ||
| 36 | + <var key="HTTPS">off</var> | ||
| 37 | + <var key="rack.run_once">false</var> | ||
| 38 | + <var key="PATH_INFO">/verify</var> | ||
| 39 | + <var key="action_dispatch.secret_token">994f235e3372684bc736dd11842b754d2ddcffc8c2958d33a29527c3217becd6655fa4653a318bc7c34131f9baf2acc0c424ed07e48e0e5e87c6cd34d711e985</var> | ||
| 40 | + <var key="rack.version">11</var> | ||
| 41 | + <var key="SCRIPT_NAME"/> | ||
| 42 | + <var key="action_dispatch.request.path_parameters"> | ||
| 43 | + <var key="action">verify</var> | ||
| 44 | + <var key="controller">application</var> | ||
| 45 | + </var> | ||
| 46 | + <var key="rack.multithread">false</var> | ||
| 47 | + <var key="action_dispatch.parameter_filter">password</var> | ||
| 48 | + <var key="action_dispatch.cookies"/> | ||
| 49 | + <var key="action_dispatch.request.request_parameters"/> | ||
| 50 | + <var key="rack.multiprocess">true</var> | ||
| 51 | + <var key="rack.request.query_hash"/> | ||
| 52 | + <var key="SERVER_PORT">80</var> | ||
| 53 | + <var key="REQUEST_METHOD">GET</var> | ||
| 54 | + <var key="action_controller.instance">#<ApplicationController:0x103d2f560></var> | ||
| 55 | + <var key="rack.session.options"> | ||
| 56 | + <var key="secure">false</var> | ||
| 57 | + <var key="httponly">true</var> | ||
| 58 | + <var key="path">/</var> | ||
| 59 | + <var key="expire_after"/> | ||
| 60 | + <var key="domain"/> | ||
| 61 | + <var key="id"/> | ||
| 62 | + </var> | ||
| 63 | + <var key="rack.input">#<StringIO:0x103d9dc90></var> | ||
| 64 | + <var key="action_dispatch.request.content_type"/> | ||
| 65 | + <var key="rack.request.query_string"/> | ||
| 66 | + <var key="QUERY_STRING"/> | ||
| 67 | + </cgi-data> | ||
| 68 | + </request> | ||
| 69 | + <server-environment> | ||
| 70 | + <project-root>/path/to/sample/project</project-root> | ||
| 71 | + <environment-name>development</environment-name> | ||
| 72 | + </server-environment> | ||
| 73 | +</notice> |
spec/models/app_spec.rb
| @@ -187,109 +187,17 @@ describe App do | @@ -187,109 +187,17 @@ describe App do | ||
| 187 | end | 187 | end |
| 188 | 188 | ||
| 189 | 189 | ||
| 190 | - context '#report_error!' do | ||
| 191 | - before do | ||
| 192 | - @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
| 193 | - @app = Fabricate(:app, :api_key => 'APIKEY') | ||
| 194 | - ErrorReport.any_instance.stub(:fingerprint).and_return('fingerprintdigest') | ||
| 195 | - end | ||
| 196 | - | ||
| 197 | - it 'finds the correct app' do | ||
| 198 | - @notice = App.report_error!(@xml) | ||
| 199 | - @notice.err.app.should == @app | ||
| 200 | - end | ||
| 201 | - | ||
| 202 | - it 'finds the correct err for the notice' do | ||
| 203 | - App.should_receive(:find_by_api_key!).and_return(@app) | ||
| 204 | - @app.should_receive(:find_or_create_err!).with({ | ||
| 205 | - :error_class => 'HoptoadTestingException', | ||
| 206 | - :component => 'application', | ||
| 207 | - :action => 'verify', | ||
| 208 | - :environment => 'development', | ||
| 209 | - :fingerprint => 'fingerprintdigest' | ||
| 210 | - }).and_return(err = Fabricate(:err)) | ||
| 211 | - err.notices.stub(:create!) | ||
| 212 | - @notice = App.report_error!(@xml) | ||
| 213 | - end | ||
| 214 | - | ||
| 215 | - it 'marks the err as unresolved if it was previously resolved' do | ||
| 216 | - App.should_receive(:find_by_api_key!).and_return(@app) | ||
| 217 | - @app.should_receive(:find_or_create_err!).with({ | ||
| 218 | - :error_class => 'HoptoadTestingException', | ||
| 219 | - :component => 'application', | ||
| 220 | - :action => 'verify', | ||
| 221 | - :environment => 'development', | ||
| 222 | - :fingerprint => 'fingerprintdigest' | ||
| 223 | - }).and_return(err = Fabricate(:err, :problem => Fabricate(:problem, :resolved => true))) | ||
| 224 | - err.should be_resolved | ||
| 225 | - @notice = App.report_error!(@xml) | ||
| 226 | - @notice.err.should == err | ||
| 227 | - @notice.err.should_not be_resolved | ||
| 228 | - end | ||
| 229 | - | ||
| 230 | - it 'should create a new notice' do | ||
| 231 | - @notice = App.report_error!(@xml) | ||
| 232 | - @notice.should be_persisted | ||
| 233 | - end | ||
| 234 | - | ||
| 235 | - it 'assigns an err to the notice' do | ||
| 236 | - @notice = App.report_error!(@xml) | ||
| 237 | - @notice.err.should be_a(Err) | ||
| 238 | - end | ||
| 239 | - | ||
| 240 | - it 'captures the err message' do | ||
| 241 | - @notice = App.report_error!(@xml) | ||
| 242 | - @notice.message.should == 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' | ||
| 243 | - end | ||
| 244 | - | ||
| 245 | - it 'captures the backtrace' do | ||
| 246 | - @notice = App.report_error!(@xml) | ||
| 247 | - @notice.backtrace_lines.size.should == 73 | ||
| 248 | - @notice.backtrace_lines.last['file'].should == '[GEM_ROOT]/bin/rake' | ||
| 249 | - end | ||
| 250 | - | ||
| 251 | - it 'captures the server_environment' do | ||
| 252 | - @notice = App.report_error!(@xml) | ||
| 253 | - @notice.server_environment['environment-name'].should == 'development' | ||
| 254 | - end | ||
| 255 | - | ||
| 256 | - it 'captures the request' do | ||
| 257 | - @notice = App.report_error!(@xml) | ||
| 258 | - @notice.request['url'].should == 'http://example.org/verify' | ||
| 259 | - @notice.request['params']['controller'].should == 'application' | ||
| 260 | - end | ||
| 261 | - | ||
| 262 | - it 'captures the notifier' do | ||
| 263 | - @notice = App.report_error!(@xml) | ||
| 264 | - @notice.notifier['name'].should == 'Hoptoad Notifier' | ||
| 265 | - end | ||
| 266 | - | ||
| 267 | - it "should handle params without 'request' section" do | ||
| 268 | - xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | ||
| 269 | - lambda { App.report_error!(xml) }.should_not raise_error | ||
| 270 | - end | ||
| 271 | - | ||
| 272 | - it "should handle params with only a single line of backtrace" do | ||
| 273 | - xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read | ||
| 274 | - lambda { @notice = App.report_error!(xml) }.should_not raise_error | ||
| 275 | - @notice.backtrace_lines.length.should == 1 | 190 | + describe ".find_by_api_key!" do |
| 191 | + it 'return the app with api_key' do | ||
| 192 | + app = Fabricate(:app) | ||
| 193 | + expect(App.find_by_api_key!(app.api_key)).to eq app | ||
| 276 | end | 194 | end |
| 277 | - | ||
| 278 | - it 'captures the current_user' do | ||
| 279 | - @notice = App.report_error!(@xml) | ||
| 280 | - @notice.user_attributes['id'].should == '123' | ||
| 281 | - @notice.user_attributes['name'].should == 'Mr. Bean' | ||
| 282 | - @notice.user_attributes['email'].should == 'mr.bean@example.com' | ||
| 283 | - @notice.user_attributes['username'].should == 'mrbean' | 195 | + it 'raise Mongoid::Errors::DocumentNotFound if not found' do |
| 196 | + expect { | ||
| 197 | + App.find_by_api_key!('foo') | ||
| 198 | + }.to raise_error(Mongoid::Errors::DocumentNotFound) | ||
| 284 | end | 199 | end |
| 285 | - | ||
| 286 | - it 'captures the framework' do | ||
| 287 | - @notice = App.report_error!(@xml) | ||
| 288 | - @notice.framework.should == 'Rails: 3.2.11' | ||
| 289 | - end | ||
| 290 | - | ||
| 291 | end | 200 | end |
| 292 | 201 | ||
| 293 | - | ||
| 294 | end | 202 | end |
| 295 | 203 |
| @@ -0,0 +1,204 @@ | @@ -0,0 +1,204 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe ErrorReport do | ||
| 4 | + context "with notice without line of backtrace" do | ||
| 5 | + let(:xml){ | ||
| 6 | + Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | ||
| 7 | + } | ||
| 8 | + | ||
| 9 | + let(:error_report) { | ||
| 10 | + ErrorReport.new(xml) | ||
| 11 | + } | ||
| 12 | + | ||
| 13 | + let!(:app) { | ||
| 14 | + Fabricate( | ||
| 15 | + :app, | ||
| 16 | + :api_key => 'APIKEY' | ||
| 17 | + ) | ||
| 18 | + } | ||
| 19 | + | ||
| 20 | + describe "#app" do | ||
| 21 | + it 'find the good app' do | ||
| 22 | + expect(error_report.app).to eq app | ||
| 23 | + end | ||
| 24 | + end | ||
| 25 | + | ||
| 26 | + describe "#backtrace" do | ||
| 27 | + | ||
| 28 | + it 'should have valid backtrace' do | ||
| 29 | + error_report.backtrace.should be_valid | ||
| 30 | + end | ||
| 31 | + end | ||
| 32 | + | ||
| 33 | + describe "#generate_notice!" do | ||
| 34 | + it "save a notice" do | ||
| 35 | + expect { | ||
| 36 | + error_report.generate_notice! | ||
| 37 | + }.to change { | ||
| 38 | + app.reload.problems.count | ||
| 39 | + }.by(1) | ||
| 40 | + end | ||
| 41 | + describe "notice create" do | ||
| 42 | + before { error_report.generate_notice! } | ||
| 43 | + subject { error_report.notice } | ||
| 44 | + its(:message) { 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.' } | ||
| 45 | + its(:framework) { should == 'Rails: 3.2.11' } | ||
| 46 | + | ||
| 47 | + it 'has complete backtrace' do | ||
| 48 | + subject.backtrace_lines.size.should == 73 | ||
| 49 | + subject.backtrace_lines.last['file'].should == '[GEM_ROOT]/bin/rake' | ||
| 50 | + end | ||
| 51 | + it 'has server_environement' do | ||
| 52 | + subject.server_environment['environment-name'].should == 'development' | ||
| 53 | + end | ||
| 54 | + | ||
| 55 | + it 'has request' do | ||
| 56 | + subject.request['url'].should == 'http://example.org/verify' | ||
| 57 | + subject.request['params']['controller'].should == 'application' | ||
| 58 | + end | ||
| 59 | + | ||
| 60 | + it 'has notifier' do | ||
| 61 | + subject.notifier['name'].should == 'Hoptoad Notifier' | ||
| 62 | + end | ||
| 63 | + | ||
| 64 | + it 'get user_attributes' do | ||
| 65 | + subject.user_attributes['id'].should == '123' | ||
| 66 | + subject.user_attributes['name'].should == 'Mr. Bean' | ||
| 67 | + subject.user_attributes['email'].should == 'mr.bean@example.com' | ||
| 68 | + subject.user_attributes['username'].should == 'mrbean' | ||
| 69 | + end | ||
| 70 | + it 'valid env_vars' do | ||
| 71 | + # XML: <var key="SCRIPT_NAME"/> | ||
| 72 | + subject.env_vars.should have_key('SCRIPT_NAME') | ||
| 73 | + subject.env_vars['SCRIPT_NAME'].should be_nil # blank ends up nil | ||
| 74 | + | ||
| 75 | + # XML representation: | ||
| 76 | + # <var key="rack.session.options"> | ||
| 77 | + # <var key="secure">false</var> | ||
| 78 | + # <var key="httponly">true</var> | ||
| 79 | + # <var key="path">/</var> | ||
| 80 | + # <var key="expire_after"/> | ||
| 81 | + # <var key="domain"/> | ||
| 82 | + # <var key="id"/> | ||
| 83 | + # </var> | ||
| 84 | + expected = { | ||
| 85 | + 'secure' => 'false', | ||
| 86 | + 'httponly' => 'true', | ||
| 87 | + 'path' => '/', | ||
| 88 | + 'expire_after' => nil, | ||
| 89 | + 'domain' => nil, | ||
| 90 | + 'id' => nil | ||
| 91 | + } | ||
| 92 | + subject.env_vars.should have_key('rack_session_options') | ||
| 93 | + subject.env_vars['rack_session_options'].should eql(expected) | ||
| 94 | + end | ||
| 95 | + end | ||
| 96 | + | ||
| 97 | + it 'save a notice assignes to err' do | ||
| 98 | + error_report.generate_notice! | ||
| 99 | + error_report.notice.err.should be_a(Err) | ||
| 100 | + end | ||
| 101 | + | ||
| 102 | + it 'memoize the notice' do | ||
| 103 | + expect { | ||
| 104 | + error_report.generate_notice! | ||
| 105 | + error_report.generate_notice! | ||
| 106 | + }.to change { | ||
| 107 | + Notice.count | ||
| 108 | + }.by(1) | ||
| 109 | + end | ||
| 110 | + | ||
| 111 | + it 'find the correct err for the notice' do | ||
| 112 | + Fabricate( | ||
| 113 | + :err, { | ||
| 114 | + :fingerprint => 'eeb6cc484167899c061e7859008c4b23bae0851c', | ||
| 115 | + :problem => Fabricate(:problem, :resolved => true) | ||
| 116 | + } | ||
| 117 | + ) | ||
| 118 | + expect { | ||
| 119 | + error_report.generate_notice! | ||
| 120 | + }.to change { | ||
| 121 | + error_report.error.resolved? | ||
| 122 | + }.from(true).to(false) | ||
| 123 | + end | ||
| 124 | + | ||
| 125 | + context "with notification service configured" do | ||
| 126 | + before do | ||
| 127 | + app.notify_on_errs = true | ||
| 128 | + app.watchers.build(:email => 'foo@example.com') | ||
| 129 | + app.save | ||
| 130 | + end | ||
| 131 | + it 'send email' do | ||
| 132 | + notice = error_report.generate_notice! | ||
| 133 | + email = ActionMailer::Base.deliveries.last | ||
| 134 | + email.to.should include(app.watchers.first.email) | ||
| 135 | + email.subject.should include(notice.message.truncate(50)) | ||
| 136 | + email.subject.should include("[#{app.name}]") | ||
| 137 | + email.subject.should include("[#{notice.environment_name}]") | ||
| 138 | + end | ||
| 139 | + end | ||
| 140 | + | ||
| 141 | + context "with xml without request section" do | ||
| 142 | + let(:xml){ | ||
| 143 | + Rails.root.join('spec','fixtures','hoptoad_test_notice_without_request_section.xml').read | ||
| 144 | + } | ||
| 145 | + it "save a notice" do | ||
| 146 | + expect { | ||
| 147 | + error_report.generate_notice! | ||
| 148 | + }.to change { | ||
| 149 | + app.reload.problems.count | ||
| 150 | + }.by(1) | ||
| 151 | + end | ||
| 152 | + end | ||
| 153 | + | ||
| 154 | + context "with xml with only a single line of backtrace" do | ||
| 155 | + let(:xml){ | ||
| 156 | + Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read | ||
| 157 | + } | ||
| 158 | + it "save a notice" do | ||
| 159 | + expect { | ||
| 160 | + error_report.generate_notice! | ||
| 161 | + }.to change { | ||
| 162 | + app.reload.problems.count | ||
| 163 | + }.by(1) | ||
| 164 | + end | ||
| 165 | + end | ||
| 166 | + end | ||
| 167 | + | ||
| 168 | + describe "#valid?" do | ||
| 169 | + context "with valid error report" do | ||
| 170 | + it "return true" do | ||
| 171 | + expect(error_report.valid?).to be true | ||
| 172 | + end | ||
| 173 | + end | ||
| 174 | + context "with not valid api_key" do | ||
| 175 | + before do | ||
| 176 | + App.where(:api_key => app.api_key).delete_all | ||
| 177 | + end | ||
| 178 | + it "return false" do | ||
| 179 | + expect(error_report.valid?).to be false | ||
| 180 | + end | ||
| 181 | + end | ||
| 182 | + end | ||
| 183 | + | ||
| 184 | + describe "#notice" do | ||
| 185 | + context "before generate_notice!" do | ||
| 186 | + it 'return nil' do | ||
| 187 | + expect(error_report.notice).to be nil | ||
| 188 | + end | ||
| 189 | + end | ||
| 190 | + | ||
| 191 | + context "after generate_notice!" do | ||
| 192 | + before do | ||
| 193 | + error_report.generate_notice! | ||
| 194 | + end | ||
| 195 | + | ||
| 196 | + it 'return the notice' do | ||
| 197 | + expect(error_report.notice).to be_a Notice | ||
| 198 | + end | ||
| 199 | + | ||
| 200 | + end | ||
| 201 | + end | ||
| 202 | + | ||
| 203 | + end | ||
| 204 | +end |
| @@ -0,0 +1,50 @@ | @@ -0,0 +1,50 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe "Notices management" do | ||
| 4 | + | ||
| 5 | + let(:errbit_app) { Fabricate(:app, | ||
| 6 | + :api_key => 'APIKEY') } | ||
| 7 | + | ||
| 8 | + describe "create a new notice" do | ||
| 9 | + context "with valide notice" do | ||
| 10 | + let(:xml) { Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read } | ||
| 11 | + it 'save a new notice' do | ||
| 12 | + expect { | ||
| 13 | + post '/notifier_api/v2/notices', :data => xml | ||
| 14 | + expect(response).to be_success | ||
| 15 | + }.to change { | ||
| 16 | + errbit_app.problems.count | ||
| 17 | + }.by(1) | ||
| 18 | + end | ||
| 19 | + end | ||
| 20 | + | ||
| 21 | + context "with notice with empty backtrace" do | ||
| 22 | + let(:xml) { Rails.root.join('spec','fixtures','hoptoad_test_notice_without_line_of_backtrace.xml').read } | ||
| 23 | + it 'save a new notice' do | ||
| 24 | + expect { | ||
| 25 | + post '/notifier_api/v2/notices', :data => xml | ||
| 26 | + expect(response).to be_success | ||
| 27 | + }.to change { | ||
| 28 | + errbit_app.problems.count | ||
| 29 | + }.by(1) | ||
| 30 | + end | ||
| 31 | + end | ||
| 32 | + | ||
| 33 | + context "with notice with bad api_key" do | ||
| 34 | + let(:errbit_app) { Fabricate(:app) } | ||
| 35 | + let(:xml) { Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read } | ||
| 36 | + it 'not save a new notice and return 422' do | ||
| 37 | + expect { | ||
| 38 | + post '/notifier_api/v2/notices', :data => xml | ||
| 39 | + expect(response.status).to eq 422 | ||
| 40 | + expect(response.body).to eq "Your API key is unknown" | ||
| 41 | + }.to_not change { | ||
| 42 | + errbit_app.problems.count | ||
| 43 | + }.by(1) | ||
| 44 | + end | ||
| 45 | + | ||
| 46 | + end | ||
| 47 | + | ||
| 48 | + end | ||
| 49 | + | ||
| 50 | +end |