On Jun 17, 2011, at 4:36 PM, [email protected] wrote: Hi,
This looks good. Good work with JSON error reporting. We should also cover this stuff with some basic unit tests in future. Please remove unnecessary 'puts' below an then send me this patch again, I'll push it. ACK -- Michal > From: Francesco Vollero <[email protected]> > > --- > server/lib/deltacloud/base_driver/exceptions.rb | 37 +++++++++---------- > server/lib/deltacloud/drivers/mock/mock_driver.rb | 29 +++++++++++++--- > .../lib/deltacloud/helpers/application_helper.rb | 4 ++- > server/lib/deltacloud/helpers/json_helper.rb | 8 ++++ > server/lib/sinatra/rabbit.rb | 3 +- > server/views/errors/500.xml.haml | 2 - > 6 files changed, 54 insertions(+), 29 deletions(-) > > diff --git a/server/lib/deltacloud/base_driver/exceptions.rb > b/server/lib/deltacloud/base_driver/exceptions.rb > index de9223e..391910f 100644 > --- a/server/lib/deltacloud/base_driver/exceptions.rb > +++ b/server/lib/deltacloud/base_driver/exceptions.rb > @@ -3,11 +3,10 @@ module Deltacloud > > class DeltacloudException < StandardError > > - attr_accessor :code, :name, :message, :backtrace, :request, :details > + attr_accessor :code, :name, :message, :backtrace, :request > > - def initialize(code, name, message, backtrace, details, request=nil) > + def initialize(code, name, message, backtrace, request=nil) > @code, @name, @message = code, name, message > - @details = details > @backtrace = backtrace > @request = request > self > @@ -16,26 +15,30 @@ module Deltacloud > end > > class AuthenticationFailure < DeltacloudException > - def initialize(e, details) > - super(401, e.class.name, e.message, e.backtrace, details) > + def initialize(e, message=nil) > + message ||= e.message > + super(401, e.class.name, message, e.backtrace) > end > end > > class ValidationFailure < DeltacloudException > - def initialize(e, details) > - super(400, e.class.name, e.message, e.backtrace, details) > + def initialize(e, message=nil) > + message ||= e.message > + super(400, e.class.name, message, e.backtrace) > end > end > > class BackendError < DeltacloudException > - def initialize(e, details) > - super(500, e.class.name, e.message, e.backtrace, details) > + def initialize(e, message=nil) > + message ||= e.message > + super(500, e.class.name, message, e.backtrace, message) > end > end > > class ProviderError < DeltacloudException > - def initialize(e, details) > - super(502, e.class.name, e.message, e.backtrace, details) > + def initialize(e, message) > + message ||= e.message > + super(502, e.class.name, message, e.backtrace) > end > end > > @@ -58,10 +61,6 @@ module Deltacloud > self.message = message > end > > - def details(details) > - self.details = details > - end > - > def exception(handler) > self.handler = handler > end > @@ -79,10 +78,10 @@ module Deltacloud > def handler(e) > return @handler if @handler > case @status > - when 401 then > Deltacloud::ExceptionHandler::AuthenticationFailure.new(e, @details) > - when 400 then > Deltacloud::ExceptionHandler::ValidationFailure.new(e, @details) > - when 500 then Deltacloud::ExceptionHandler::BackendError.new(e, > @details) > - when 502 then Deltacloud::ExceptionHandler::ProviderError.new(e, > @details) > + when 401 then > Deltacloud::ExceptionHandler::AuthenticationFailure.new(e, @message) > + when 400 then > Deltacloud::ExceptionHandler::ValidationFailure.new(e, @message) > + when 500 then Deltacloud::ExceptionHandler::BackendError.new(e, > @message) > + when 502 then Deltacloud::ExceptionHandler::ProviderError.new(e, > @message) > end > end > > diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb > b/server/lib/deltacloud/drivers/mock/mock_driver.rb > index dd237a1..bc1e9ff 100644 > --- a/server/lib/deltacloud/drivers/mock/mock_driver.rb > +++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb > @@ -137,7 +137,7 @@ class MockDriver < Deltacloud::BaseDriver > def create_image(credentials, opts={}) > check_credentials(credentials) > instance = instance(credentials, :id => opts[:instance_id]) > - raise BackendError::new(500, 'CreateImageNotSupported', '', '') unless > instance.can_create_image? > + raise 'CreateImageNotSupported' unless instance.can_create_image? > ids = Dir[ "#{@storage_root}/images/*.yml" ].collect{|e| File.basename( > e, ".yml" )} > count = 0 > while true > @@ -324,8 +324,7 @@ class MockDriver < Deltacloud::BaseDriver > } > key_dir = File.join(@storage_root, 'keys') > if File.exists?(key_dir + "/#{key_hash[:id]}.yml") > - raise Deltacloud::BackendError.new(403, self.class.to_s, "key-exists", > - ["Key with same name already > exists"]) > + raise "KeyExist" > end > FileUtils.mkdir_p(key_dir) unless File.directory?(key_dir) > File.open(key_dir + "/#{key_hash[:id]}.yml", 'w') do |f| > @@ -387,7 +386,7 @@ class MockDriver < Deltacloud::BaseDriver > check_credentials(credentials) > bucket = bucket(credentials, {:id => name}) > unless (bucket.size == "0") > - raise Deltacloud::BackendError.new(403, self.class.to_s, > "bucket-not-empty", "delete operation not valid for non-empty bucket") > + raise "BucketNotEmpty" > end > safely do > File.delete(File::join(@storage_root, 'buckets', "#{name}.yml")) > @@ -451,7 +450,7 @@ class MockDriver < Deltacloud::BaseDriver > blobfile = File::join("#{@storage_root}", "buckets", "blobs", > "#{blob_id}.yml") > safely do > unless File.exists?(blobfile) > - raise Deltacloud::BackendError.new(500, self.class.to_s, "blob > #{blob_id} doesn't exist", "cannot delete non existant blob") > + raise "NotExistentBlob" > end > File.delete(blobfile) > end > @@ -509,6 +508,26 @@ class MockDriver < Deltacloud::BaseDriver > > on /AuthFailure/ do > status 401 > + message "Authentication Failure" > + end > + > + on /BucketNotEmpty/ do > + status 403 > + message "Delete operation not valid for non-empty bucket" > + end > + > + on /KeyExist/ do > + status 403 > + message "Key with same name already exists" > + end > + > + on /CreateImageNotSupported/ do > + status 500 > + end > + > + on /NotExistentBlob/ do > + status 500 > + message "Could not delete a non existent blob" > end > > on /Err/ do > diff --git a/server/lib/deltacloud/helpers/application_helper.rb > b/server/lib/deltacloud/helpers/application_helper.rb > index e431e11..9166c00 100644 > --- a/server/lib/deltacloud/helpers/application_helper.rb > +++ b/server/lib/deltacloud/helpers/application_helper.rb > @@ -108,8 +108,10 @@ module ApplicationHelper > @error, @code = request.env['sinatra.error'], code > @code = 500 if not @code and not @error.class.method_defined? :code > response.status = @code || @error.code > + puts @error.inspect Please remove this puts :-) > respond_to do |format| > - format.xml { haml :"errors/#{@code || @error.code}", :layout => false } > + format.xml { haml :"errors/#{@code || @error.code}", :layout => false > } > + format.json { json_return_error(@error) } > format.html { haml :"errors/#{@code || @error.code}", :layout => :error > } > end > end > diff --git a/server/lib/deltacloud/helpers/json_helper.rb > b/server/lib/deltacloud/helpers/json_helper.rb > index 170b118..aea16b6 100644 > --- a/server/lib/deltacloud/helpers/json_helper.rb > +++ b/server/lib/deltacloud/helpers/json_helper.rb > @@ -20,4 +20,12 @@ module JSONHelper > features.empty? ? {} : { :features => features } > end > > + def json_return_error(error) > + error_output=Hash.new > + error_output[:url] =request.env['REQUEST_URI'] > + error_output[:status] =response.status > + error_output[:message]=error.message if error > + error_output.to_json > + end > + > end > diff --git a/server/lib/sinatra/rabbit.rb b/server/lib/sinatra/rabbit.rb > index 39ac227..5c1d757 100644 > --- a/server/lib/sinatra/rabbit.rb > +++ b/server/lib/sinatra/rabbit.rb > @@ -28,8 +28,7 @@ module Sinatra > class DuplicateCollectionException < > Deltacloud::ExceptionHandler::DeltacloudException; end > class UnsupportedCollectionException < > Deltacloud::ExceptionHandler::DeltacloudException > def initialize > - @details = "This collection is not supported for this provider." > - @message = @details > + @message = "This collection is not supported for this provider." > # The server understood the request, but is refusing to fulfill it. > Authorization will not help and the request > # SHOULD NOT be repeated. If the request method was not HEAD and the > server wishes to make public why the request > # has not been fulfilled, it SHOULD describe the reason for the > refusal in the entity. If the server does not wish > diff --git a/server/views/errors/500.xml.haml > b/server/views/errors/500.xml.haml > index 39416b6..69c242e 100644 > --- a/server/views/errors/500.xml.haml > +++ b/server/views/errors/500.xml.haml > @@ -2,6 +2,4 @@ > %kind backend_error > %backend{ :driver => driver_symbol } > %code=response.status > - - if @error.class.method_defined? :details > - %details< #{cdata @error.details.join("\n")} > %message< #{cdata @error.message} > -- > 1.7.4.4 > ------------------------------------------------------ Michal Fojtik, [email protected] Deltacloud API: http://deltacloud.org
