Commit 1e6b4cf2816368ff827f542789a05e8d3a429968
1 parent
308aca74
Exists in
master
and in
1 other branch
Return 400 status and message if no args pass when you do a notice
The API received notice return a 500 if no all params are pass. Now return a 400 and information about args needed Fix #393
Showing
2 changed files
with
58 additions
and
30 deletions
Show diff stats
app/controllers/notices_controller.rb
| 1 | class NoticesController < ApplicationController | 1 | class NoticesController < ApplicationController |
| 2 | - respond_to :xml | 2 | + |
| 3 | + class ParamsError < StandardError; end | ||
| 3 | 4 | ||
| 4 | skip_before_filter :authenticate_user!, :only => :create | 5 | skip_before_filter :authenticate_user!, :only => :create |
| 5 | 6 | ||
| 7 | + rescue_from ParamsError, :with => :bad_params | ||
| 8 | + | ||
| 6 | def create | 9 | def create |
| 7 | # params[:data] if the notice came from a GET request, raw_post if it came via POST | 10 | # params[:data] if the notice came from a GET request, raw_post if it came via POST |
| 8 | - report = ErrorReport.new(params[:data] || request.raw_post) | 11 | + report = ErrorReport.new(notice_params) |
| 9 | 12 | ||
| 10 | if report.valid? | 13 | if report.valid? |
| 11 | report.generate_notice! | 14 | report.generate_notice! |
| @@ -24,4 +27,19 @@ class NoticesController < ApplicationController | @@ -24,4 +27,19 @@ class NoticesController < ApplicationController | ||
| 24 | redirect_to app_problem_path(problem.app, problem) | 27 | redirect_to app_problem_path(problem.app, problem) |
| 25 | end | 28 | end |
| 26 | 29 | ||
| 30 | + private | ||
| 31 | + | ||
| 32 | + def notice_params | ||
| 33 | + return @notice_params if @notice_params | ||
| 34 | + @notice_params = params[:data] || request.raw_post | ||
| 35 | + if @notice_params.blank? | ||
| 36 | + raise ParamsError.new('Need a data params in GET or raw post data') | ||
| 37 | + end | ||
| 38 | + @notice_params | ||
| 39 | + end | ||
| 40 | + | ||
| 41 | + def bad_params(exception) | ||
| 42 | + render :text => exception.message, :status => :bad_request | ||
| 43 | + end | ||
| 44 | + | ||
| 27 | end | 45 | end |
spec/controllers/notices_controller_spec.rb
| @@ -9,17 +9,29 @@ describe NoticesController do | @@ -9,17 +9,29 @@ describe NoticesController do | ||
| 9 | let(:error_report) { mock(:valid? => true, :generate_notice! => true, :notice => notice) } | 9 | let(:error_report) { mock(:valid? => true, :generate_notice! => true, :notice => notice) } |
| 10 | 10 | ||
| 11 | context 'notices API' do | 11 | context 'notices API' do |
| 12 | - before do | ||
| 13 | - ErrorReport.should_receive(:new).with(xml).and_return(error_report) | ||
| 14 | - end | ||
| 15 | - | ||
| 16 | - context "with xml pass in raw_port" do | 12 | + context "with all params" do |
| 17 | before do | 13 | before do |
| 18 | - request.should_receive(:raw_post).and_return(xml) | ||
| 19 | - post :create, :format => :xml | 14 | + ErrorReport.should_receive(:new).with(xml).and_return(error_report) |
| 15 | + end | ||
| 16 | + | ||
| 17 | + context "with xml pass in raw_port" do | ||
| 18 | + before do | ||
| 19 | + request.should_receive(:raw_post).and_return(xml) | ||
| 20 | + post :create, :format => :xml | ||
| 21 | + end | ||
| 22 | + | ||
| 23 | + it "generates a notice from raw xml [POST]" do | ||
| 24 | + response.should be_success | ||
| 25 | + # Same RegExp from Airbrake::Sender#send_to_airbrake (https://github.com/airbrake/airbrake/blob/master/lib/airbrake/sender.rb#L53) | ||
| 26 | + # Inspired by https://github.com/airbrake/airbrake/blob/master/test/sender_test.rb | ||
| 27 | + response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) | ||
| 28 | + response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 29 | + end | ||
| 30 | + | ||
| 20 | end | 31 | end |
| 21 | 32 | ||
| 22 | - it "generates a notice from raw xml [POST]" do | 33 | + it "generates a notice from xml in a data param [POST]" do |
| 34 | + post :create, :data => xml, :format => :xml | ||
| 23 | response.should be_success | 35 | 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) | 36 | # 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 | 37 | # Inspired by https://github.com/airbrake/airbrake/blob/master/test/sender_test.rb |
| @@ -27,28 +39,26 @@ describe NoticesController do | @@ -27,28 +39,26 @@ describe NoticesController do | ||
| 27 | response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | 39 | response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) |
| 28 | end | 40 | end |
| 29 | 41 | ||
| 42 | + it "generates a notice from xml [GET]" do | ||
| 43 | + get :create, :data => xml, :format => :xml | ||
| 44 | + response.should be_success | ||
| 45 | + response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) | ||
| 46 | + response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 47 | + end | ||
| 48 | + context "with an invalid API_KEY" do | ||
| 49 | + let(:error_report) { mock(:valid? => false) } | ||
| 50 | + it 'return 422' do | ||
| 51 | + post :create, :format => :xml, :data => xml | ||
| 52 | + expect(response.status).to eq 422 | ||
| 53 | + end | ||
| 54 | + end | ||
| 30 | end | 55 | end |
| 31 | 56 | ||
| 32 | - it "generates a notice from xml in a data param [POST]" do | ||
| 33 | - post :create, :data => xml, :format => :xml | ||
| 34 | - response.should be_success | ||
| 35 | - # Same RegExp from Airbrake::Sender#send_to_airbrake (https://github.com/airbrake/airbrake/blob/master/lib/airbrake/sender.rb#L53) | ||
| 36 | - # Inspired by https://github.com/airbrake/airbrake/blob/master/test/sender_test.rb | ||
| 37 | - response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) | ||
| 38 | - response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 39 | - end | ||
| 40 | - | ||
| 41 | - it "generates a notice from xml [GET]" do | ||
| 42 | - get :create, :data => xml, :format => :xml | ||
| 43 | - response.should be_success | ||
| 44 | - response.body.should match(%r{<id[^>]*>#{notice.id}</id>}) | ||
| 45 | - response.body.should match(%r{<url[^>]*>(.+)#{locate_path(notice.id)}</url>}) | ||
| 46 | - end | ||
| 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 | 57 | + context "without params needed" do |
| 58 | + it 'return 400' do | ||
| 59 | + post :create, :format => :xml | ||
| 60 | + expect(response.status).to eq 400 | ||
| 61 | + expect(response.body).to eq 'Need a data params in GET or raw post data' | ||
| 52 | end | 62 | end |
| 53 | end | 63 | end |
| 54 | end | 64 | end |