Mailing-List: contact deltacloud-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: deltacloud-dev@incubator.apache.org
Received-SPF: pass (athena.apache.org: domain of fvollero@redhat.com
 designates 209.132.183.28 as permitted sender)
From: fvollero@redhat.com
To: deltacloud-dev@incubator.apache.org
Subject: [PATCH core] Cleanup and minor improvements in json error reporting
Date: Mon, 20 Jun 2011 12:18:34 +0200
Message-Id: <1308565114-25523-1-git-send-email-fvollero@redhat.com>

From: Francesco Vollero <fvollero@redhat.com>

---
 server/lib/deltacloud/base_driver/exceptions.rb    |   37 +++++++++----------
 server/lib/deltacloud/drivers/mock/mock_driver.rb  |   29 +++++++++++++---
 .../lib/deltacloud/helpers/application_helper.rb   |    3 +-
 server/lib/deltacloud/helpers/json_helper.rb       |    8 ++++
 server/lib/sinatra/rabbit.rb                       |    3 +-
 server/views/errors/500.xml.haml                   |    2 -
 6 files changed, 53 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..ebfcf35 100644
--- a/server/lib/deltacloud/helpers/application_helper.rb
+++ b/server/lib/deltacloud/helpers/application_helper.rb
@@ -109,7 +109,8 @@ module ApplicationHelper
     @code = 500 if not @code and not @error.class.method_defined? :code
     response.status = @code || @error.code
     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