deltacloud-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mfoj...@apache.org
Subject [2/3] git commit: Client: Revamped client/server error handling
Date Wed, 29 Feb 2012 15:38:21 GMT
Client: Revamped client/server error handling

This patch will various HTTP error exceptions to
client library. The exception classes are then used
to properly report server/client exceptions and they
should help handle various HTTP errors.

Signed-off-by: Michal fojtik <mfojtik@redhat.com>


Project: http://git-wip-us.apache.org/repos/asf/deltacloud/repo
Commit: http://git-wip-us.apache.org/repos/asf/deltacloud/commit/d6b59298
Tree: http://git-wip-us.apache.org/repos/asf/deltacloud/tree/d6b59298
Diff: http://git-wip-us.apache.org/repos/asf/deltacloud/diff/d6b59298

Branch: refs/heads/master
Commit: d6b59298cc643009358b828b1d6593e086f2d1e1
Parents: 6562a1c
Author: Michal Fojtik <mfojtik@redhat.com>
Authored: Fri Feb 10 13:56:01 2012 +0100
Committer: Michal fojtik <mfojtik@redhat.com>
Committed: Wed Feb 29 16:39:13 2012 +0100

----------------------------------------------------------------------
 client/lib/deltacloud.rb                        |   72 ++++------
 client/lib/errors.rb                            |  140 ++++++++++++++++++
 client/specs/errors_spec.rb                     |   59 ++++++++
 server/lib/deltacloud/base_driver/exceptions.rb |    4 +-
 server/views/errors/500.xml.haml                |    2 +
 server/views/errors/501.xml.haml                |   13 +--
 server/views/errors/502.xml.haml                |   13 +--
 server/views/errors/504.xml.haml                |   13 +--
 8 files changed, 235 insertions(+), 81 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/client/lib/deltacloud.rb
----------------------------------------------------------------------
diff --git a/client/lib/deltacloud.rb b/client/lib/deltacloud.rb
index 614eab2..e0cb2e4 100644
--- a/client/lib/deltacloud.rb
+++ b/client/lib/deltacloud.rb
@@ -21,6 +21,7 @@ require 'hwp_properties'
 require 'instance_state'
 require 'documentation'
 require 'base_object'
+require 'errors'
 require 'client_bucket_methods'
 
 module DeltaCloud
@@ -314,9 +315,7 @@ module DeltaCloud
 
         request(:post, entry_points[:"#{$1}s"], {}, params) do |response|
           obj = base_object(:"#{$1}", response)
-          # All create calls must respond 201 HTTP code
-          # to indicate that resource was created.
-          handle_backend_error(response) if response.code!=201
+          response_error(response) unless response_successful?(response.code)
           yield obj if block_given?
         end
         return obj
@@ -349,6 +348,28 @@ module DeltaCloud
       headers
     end
 
+    def response_successful?(code)
+      return true if code.to_s =~ /^2(\d{2})$/
+      return true if code.to_s =~ /^3(\d{2})$/
+      return false
+    end
+
+    def response_error(response)
+      if response.code.to_s =~ /4(\d{2})/
+        DeltaCloud::HTTPError.client_error(response.code)
+      else
+        xml = Nokogiri::XML(response.to_s)
+        opts = {
+          :driver => (xml/'backend').first[:driver],
+          :provider => (xml/'backend').first[:provider],
+          :params => (xml/'request/param').inject({}) { |r,p| r[:"#{p[:name]}"] = p.text;
r }
+        }
+        backtrace = (xml/'backtrace').empty? ? nil : (xml/'backtrace').first.text.split("\n")[1..10].map
{ |l| l.strip }
+        DeltaCloud::HTTPError.server_error(xml.root[:status] || response.code,
+                                           (xml/'message').first.text, opts, backtrace)
+      end
+    end
+
     # Basic request method
     #
     def request(*args, &block)
@@ -367,55 +388,18 @@ module DeltaCloud
       if conf[:method].eql?(:post)
         resource = RestClient::Resource.new(conf[:path], :open_timeout => conf[:open_timeout],
:timeout => conf[:timeout])
         resource.send(:post, conf[:form_data], default_headers.merge(extended_headers)) do
|response, request, block|
-          handle_backend_error(response) if [500, 502, 501, 401, 504].include? response.code
-          if response.respond_to?('body')
-            yield response.body if block_given?
-          else
-            yield response.to_s if block_given?
-          end
+          response_error(response) unless response_successful? response.code
+          yield response.to_s
         end
       else
         resource = RestClient::Resource.new(conf[:path], :open_timeout => conf[:open_timeout],
:timeout => conf[:timeout])
         resource.send(conf[:method], default_headers.merge(extended_headers)) do |response,
request, block|
-          handle_backend_error(response) if [500, 502, 501, 504, 401].include? response.code
-          if conf[:method].eql?(:get) and [301, 302, 307].include? response.code
-            response.follow_redirection(request) do |response, request, block|
-              if response.respond_to?('body')
-                yield response.body if block_given?
-              else
-                yield response.to_s if block_given?
-              end
-            end
-          else
-            if response.respond_to?('body')
-              yield response.body if block_given?
-            else
-              yield response.to_s if block_given?
-            end
-          end
+          response_error(response) unless response_successful? response.code
+          yield response.to_s
         end
       end
     end
 
-    # Re-raise backend errors as on exception in client with message from
-    # backend
-    class BackendError < StandardError
-
-      def initialize(opts={})
-        opts[:message] = "Not authorized / Invalid credentials" if opts[:code] == 401
-        super("#{opts[:code]} : #{opts[:message]}")
-        set_backtrace(opts[:backtrace].split("\n").map { |l| l.strip }[0..10]) if opts[:backtrace]
-      end
-
-    end
-
-    def handle_backend_error(response)
-      response_xml = Nokogiri::XML(response)
-      backtrace = (response_xml/'error/backtrace').empty? ? nil : (response_xml/'error/backtrace').text
-      raise BackendError.new(:message => (response_xml/'error/message').text,
-                             :code => response.code,
-                             :backtrace => backtrace)
-    end
 
     # Check if specified collection have wanted feature
     def feature?(collection, name)

http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/client/lib/errors.rb
----------------------------------------------------------------------
diff --git a/client/lib/errors.rb b/client/lib/errors.rb
new file mode 100644
index 0000000..d0eff44
--- /dev/null
+++ b/client/lib/errors.rb
@@ -0,0 +1,140 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+module DeltaCloud
+  module HTTPError
+
+    class ClientError < StandardError
+
+      attr_reader :params, :driver, :provider
+
+      def initialize(code, message, opts={}, backtrace=nil)
+        @params, @driver, @provider = opts[:params], opts[:driver], opts[:provider]
+        if code.to_s =~ /^5(\d{2})/
+          message += "\nParameters: #{@params.inspect}\n"
+          message += "Driver: #{@driver}@#{@provider}"
+        end
+        super("#{code}\n\n#{self.class.superclass}: #{message}\n\n")
+        # If server provided us the backtrace, then replace client backtrace
+        # with the server one.
+        set_backtrace(backtrace) unless backtrace.nil?
+      end
+    end
+
+    class ServerError < ClientError; end
+    class UknownError < ClientError; end
+
+    # For sake of consistent documentation we need to create
+    # this exceptions manually, instead of using some meta-programming.
+    # Client will really appreciate this it will try to catch some
+    # specific exception.
+
+    # Client errors (4xx)
+    class BadRequest < ClientError; end
+    class Unauthorized < ClientError; end
+    class Forbidden < ClientError; end
+    class NotFound < ClientError; end
+    class MethodNotAllowed < ClientError; end
+    class NotAcceptable < ClientError; end
+    class RequestTimeout < ClientError; end
+    class Gone < ClientError; end
+    class ExpectationFailed < ClientError; end
+    class UnsupportedMediaType < ClientError; end
+
+    # Server errors (5xx)
+    class DeltacloudError < ServerError; end
+    class ProviderError < ServerError; end
+    class ProviderTimeout < ServerError; end
+    class ServiceUnavailable < ServerError; end
+    class NotImplemented < ServerError; end
+
+    class ExceptionHandler
+
+      attr_reader :http_status_code, :message, :trace
+
+      def initialize(status_code, message=nil, opts={}, backtrace=nil, &block)
+        @http_status_code = status_code.to_i
+        @trace = backtrace
+        @message = message || client_error_messages[status_code] || 'No error message received'
+        @options = opts
+        instance_eval(&block) if block_given?
+      end
+
+      def on(code, exception_class)
+        if code == @http_status_code
+          raise exception_class.new(code, @message, @options, @trace)
+        end
+      end
+
+      private
+
+      def client_error_messages
+        {
+          400 => 'The request could not be understood by the server due to malformed syntax.',
+          401 => 'Authentication required for this request or invalid credentials provided.',
+          403 => 'Requested operation is not allowed for this resource.',
+          404 => 'Not Found',
+          405 => 'Method not allowed for this resource.',
+          406 => 'Requested media type is not supported by server.',
+          408 => 'The client did not produce a request within the time that the server
was prepared to wait.',
+          410 => 'The resource is no longer available'
+        }
+      end
+
+    end
+
+    def self.parse_response_error(response)
+    
+    end
+
+    def self.client_error(code)
+      ExceptionHandler.new(code) do
+        # Client errors
+        on 400, BadRequest
+        on 401, Unauthorized
+        on 403, Forbidden
+        on 404, NotFound
+        on 405, MethodNotAllowed
+        on 406, NotAcceptable
+        on 408, RequestTimeout
+        on 410, Gone
+      end
+    end
+
+    def self.server_error(code, message, opts={}, backtrace=nil)
+      ExceptionHandler.new(code, message, opts, backtrace) do
+        # Client errors
+        on 400, BadRequest
+        on 401, Unauthorized
+        on 403, Forbidden
+        on 404, NotFound
+        on 405, MethodNotAllowed
+        on 406, NotAcceptable
+        on 408, RequestTimeout
+        on 410, Gone
+        on 415, UnsupportedMediaType
+        on 417, ExpectationFailed
+        # Server errors
+        on 500, DeltacloudError
+        on 501, NotImplemented
+        on 502, ProviderError
+        on 503, ServiceUnavailable
+        on 504, ProviderTimeout
+      end
+      raise Deltacloud::HTTPError::UnknownError.new(code, message, opts, backtrace)
+    end
+
+  end
+end

http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/client/specs/errors_spec.rb
----------------------------------------------------------------------
diff --git a/client/specs/errors_spec.rb b/client/specs/errors_spec.rb
new file mode 100644
index 0000000..031e3b7
--- /dev/null
+++ b/client/specs/errors_spec.rb
@@ -0,0 +1,59 @@
+#
+# Copyright (C) 2009-2011  Red Hat, Inc.
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+require 'specs/spec_helper'
+
+describe "server error handler" do
+
+  it_should_behave_like "all resources"
+
+  it 'should capture HTTP 500 error as DeltacloudError' do
+    DeltaCloud.new( API_NAME, API_PASSWORD, API_URL ) do |client|
+      expect { client.realm('500') }.should raise_error(DeltaCloud::HTTPError::DeltacloudError)
+    end
+  end
+
+  it 'should capture HTTP 502 error as ProviderError' do
+    DeltaCloud.new( API_NAME, API_PASSWORD, API_URL ) do |client|
+      expect { client.realm('502') }.should raise_error(DeltaCloud::HTTPError::ProviderError)
+    end
+  end
+
+  it 'should capture HTTP 501 error as NotImplemented' do
+    DeltaCloud.new( API_NAME, API_PASSWORD, API_URL ) do |client|
+      expect { client.realm('501') }.should raise_error(DeltaCloud::HTTPError::NotImplemented)
+    end
+  end
+
+  it 'should capture HTTP 504 error as ProviderTimeout' do
+    DeltaCloud.new( API_NAME, API_PASSWORD, API_URL ) do |client|
+      expect { client.realm('504') }.should raise_error(DeltaCloud::HTTPError::ProviderTimeout)
+    end
+  end
+
+end
+
+describe "client error handler" do
+
+  it 'should capture HTTP 404 error as NotFound' do
+    DeltaCloud.new( API_NAME, API_PASSWORD, API_URL ) do |client|
+      expect { client.realm('non-existing-realm') }.should raise_error(DeltaCloud::HTTPError::NotFound)
+    end
+  end
+
+end

http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/server/lib/deltacloud/base_driver/exceptions.rb
----------------------------------------------------------------------
diff --git a/server/lib/deltacloud/base_driver/exceptions.rb b/server/lib/deltacloud/base_driver/exceptions.rb
index e30f94c..ccb942d 100644
--- a/server/lib/deltacloud/base_driver/exceptions.rb
+++ b/server/lib/deltacloud/base_driver/exceptions.rb
@@ -177,7 +177,9 @@ module Deltacloud
           if exdef.match?($!)
             $stderr.send(report_method, "#{[$!.class.to_s, $!.message].join(':')}\n#{$!.backtrace.join("\n")}")
             new_exception = exdef.handler($!)
-            raise exdef.handler($!) if new_exception
+            m = new_exception.message.nil? ? $!.message : new_exception.message
+            $stderr.send(report_method, "#{[$!.class.to_s, m].join(':')}\n#{$!.backtrace[0..10].join("\n")}")
+            raise exdef.handler($!) unless new_exception.nil?
           end
         end
         $stderr.send(report_method, "[NO HANDLED] #{[$!.class.to_s, $!.message].join(': ')}\n#{$!.backtrace.join("\n")}")

http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/server/views/errors/500.xml.haml
----------------------------------------------------------------------
diff --git a/server/views/errors/500.xml.haml b/server/views/errors/500.xml.haml
index 5c111e8..0b2d14a 100644
--- a/server/views/errors/500.xml.haml
+++ b/server/views/errors/500.xml.haml
@@ -1,3 +1,5 @@
+- unless defined?(partial)
+  !!! XML
 %error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
   %kind backend_error
   %backend{ :driver => driver_symbol, :provider => "#{Thread::current[:provider] ||
ENV['API_PROVIDER'] || 'default'}" }

http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/server/views/errors/501.xml.haml
----------------------------------------------------------------------
diff --git a/server/views/errors/501.xml.haml b/server/views/errors/501.xml.haml
index 788fe4b..a4015a4 100644
--- a/server/views/errors/501.xml.haml
+++ b/server/views/errors/501.xml.haml
@@ -1,12 +1 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %kind backend_error
-  %backend{ :driver => driver_symbol, :provider => "#{Thread::current[:provider] ||
ENV['API_PROVIDER'] || 'default'}" }
-  - if @error.respond_to?(:details) && @error.details
-    %details< #{cdata @error.details.join("\n")}
-  %message< #{cdata @error.message}
-  - if @error.respond_to? :backtrace
-    %backtrace=cdata @error.backtrace.join("\n")
-  - if params
-    %request
-      - params.each do |k, v|
-        %param{ :name => k}=v
+= haml :'errors/500', :locals => { :@error => @error, :partial => true }

http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/server/views/errors/502.xml.haml
----------------------------------------------------------------------
diff --git a/server/views/errors/502.xml.haml b/server/views/errors/502.xml.haml
index 788fe4b..a4015a4 100644
--- a/server/views/errors/502.xml.haml
+++ b/server/views/errors/502.xml.haml
@@ -1,12 +1 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %kind backend_error
-  %backend{ :driver => driver_symbol, :provider => "#{Thread::current[:provider] ||
ENV['API_PROVIDER'] || 'default'}" }
-  - if @error.respond_to?(:details) && @error.details
-    %details< #{cdata @error.details.join("\n")}
-  %message< #{cdata @error.message}
-  - if @error.respond_to? :backtrace
-    %backtrace=cdata @error.backtrace.join("\n")
-  - if params
-    %request
-      - params.each do |k, v|
-        %param{ :name => k}=v
+= haml :'errors/500', :locals => { :@error => @error, :partial => true }

http://git-wip-us.apache.org/repos/asf/deltacloud/blob/d6b59298/server/views/errors/504.xml.haml
----------------------------------------------------------------------
diff --git a/server/views/errors/504.xml.haml b/server/views/errors/504.xml.haml
index 788fe4b..a4015a4 100644
--- a/server/views/errors/504.xml.haml
+++ b/server/views/errors/504.xml.haml
@@ -1,12 +1 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %kind backend_error
-  %backend{ :driver => driver_symbol, :provider => "#{Thread::current[:provider] ||
ENV['API_PROVIDER'] || 'default'}" }
-  - if @error.respond_to?(:details) && @error.details
-    %details< #{cdata @error.details.join("\n")}
-  %message< #{cdata @error.message}
-  - if @error.respond_to? :backtrace
-    %backtrace=cdata @error.backtrace.join("\n")
-  - if params
-    %request
-      - params.each do |k, v|
-        %param{ :name => k}=v
+= haml :'errors/500', :locals => { :@error => @error, :partial => true }


Mime
View raw message