From 4a3807b21a0c9dac3b4fd22833066b838649d5fe Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 13:51:47 -0400 Subject: [PATCH] Standardize error handling for RemoveLog service endpoints Return status 409 for errors --- lib/travis/api/app/endpoint/jobs.rb | 7 +++++-- lib/travis/api/app/endpoint/logs.rb | 5 ++++- spec/unit/endpoint/logs_spec.rb | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 26d662f2..2424e221 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -64,7 +64,7 @@ class Travis::Api::App end end - patch '/:id/log', scope: :private do + patch '/:id/log', scope: :private do |id| # PATCH method for `RemoveLog` service, since we are replacing log content service = self.service(:remove_log, params) begin @@ -72,7 +72,10 @@ class Travis::Api::App rescue Travis::AuthorizationDenied => e status 401 { error: { message: e.message } } - rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e + rescue Travis::JobUnfinished => jue + status 409 + { error: { message: "Job #{id} is not finished" } } + rescue Travis::LogAlreadyRemoved => e status 409 { error: { message: e.message } } end diff --git a/lib/travis/api/app/endpoint/logs.rb b/lib/travis/api/app/endpoint/logs.rb index 37d38be2..04b3753a 100644 --- a/lib/travis/api/app/endpoint/logs.rb +++ b/lib/travis/api/app/endpoint/logs.rb @@ -19,8 +19,11 @@ class Travis::Api::App status 401 { error: { message: ade.message } } rescue Travis::JobUnfinished => jue - status 422 + status 409 { error: { message: "Job #{id} is not finished" } } + rescue Travis::LogAlreadyRemoved => e + status 409 + { error: { message: e.message } } rescue => e status 500 { error: { message: "Unexpected error occurred: #{e.message}" } } diff --git a/spec/unit/endpoint/logs_spec.rb b/spec/unit/endpoint/logs_spec.rb index 3fa502aa..20b4e83b 100644 --- a/spec/unit/endpoint/logs_spec.rb +++ b/spec/unit/endpoint/logs_spec.rb @@ -25,12 +25,12 @@ describe Travis::Api::App::Endpoint::Logs do end context 'job is still running' do - it 'returns status 422' do + it 'returns status 409' do job.stubs(:finished?).returns false user.stubs(:permission?).with(:push, anything).returns true response = patch("/logs/#{job.id}") - response.status.should == 422 + response.status.should == 409 JSON.parse(response.body)['error']['message'].should =~ Regexp.new("Job .*is (not |un)finished") end end