You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2011/04/04 16:02:03 UTC

New DSL for handling Exceptions and error reporting

Hi,

This set of patches will add 'better' error/exceptions reporting. I tried to make this
thing very simple and avoid voodoo we had in 'catched_exceptions' list.

No you can use simple DSL (like features) to catching and describing exceptions from
our beloved 'safely' blocks.

Example usage:

exceptions do

  on /AuthFailure/ do
    status 401 # Authentication failed...
  end

  on /No offering found/ do
    status 400  # Wrong combination of hardware_profile / image
  end

  on /CloudServers::Exception::(\w+)/ do
    status 502  # Provider error
  end

  on /Error/
    status 500
    message "Hello, this is a nice message to client."
    exception CustomException
  end

end

Let me know what do you think. All ideas how to improve it are welcome.

  -- Michal


[PATCH core 2/3] Renamed views to use HTTP status codes. Adapted server, rabbit and validations

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>

---
 server/lib/deltacloud/base_driver/base_driver.rb   |   53 ++-----------------
 .../lib/deltacloud/helpers/application_helper.rb   |   12 ++--
 server/lib/deltacloud/validation.rb                |    2 +-
 server/lib/sinatra/rabbit.rb                       |   21 ++++++--
 server/server.rb                                   |   30 +++---------
 server/views/errors/400.html.haml                  |   11 ++++
 server/views/errors/400.xml.haml                   |    4 ++
 server/views/errors/401.html.haml                  |    8 +++
 server/views/errors/401.xml.haml                   |    3 +
 server/views/errors/403.html.haml                  |    6 ++
 server/views/errors/403.xml.haml                   |    2 +
 server/views/errors/404.html.haml                  |    6 ++
 server/views/errors/404.xml.haml                   |    2 +
 server/views/errors/500.xml.haml                   |    8 +++
 server/views/errors/502.html.haml                  |   14 +++++
 server/views/errors/502.xml.haml                   |    8 +++
 server/views/errors/auth_exception.html.haml       |    8 ---
 server/views/errors/auth_exception.xml.haml        |    3 -
 server/views/errors/backend_error.html.haml        |   22 --------
 server/views/errors/backend_error.xml.haml         |    8 ---
 server/views/errors/not_allowed.html.haml          |    6 --
 server/views/errors/not_allowed.xml.haml           |    2 -
 server/views/errors/not_found.html.haml            |    6 --
 server/views/errors/not_found.xml.haml             |    2 -
 server/views/errors/validation_failure.html.haml   |   11 ----
 server/views/errors/validation_failure.xml.haml    |    8 ---
 26 files changed, 107 insertions(+), 159 deletions(-)
 create mode 100644 server/views/errors/400.html.haml
 create mode 100644 server/views/errors/400.xml.haml
 create mode 100644 server/views/errors/401.html.haml
 create mode 100644 server/views/errors/401.xml.haml
 create mode 100644 server/views/errors/403.html.haml
 create mode 100644 server/views/errors/403.xml.haml
 create mode 100644 server/views/errors/404.html.haml
 create mode 100644 server/views/errors/404.xml.haml
 create mode 100644 server/views/errors/500.xml.haml
 create mode 100644 server/views/errors/502.html.haml
 create mode 100644 server/views/errors/502.xml.haml
 delete mode 100644 server/views/errors/auth_exception.html.haml
 delete mode 100644 server/views/errors/auth_exception.xml.haml
 delete mode 100644 server/views/errors/backend_error.html.haml
 delete mode 100644 server/views/errors/backend_error.xml.haml
 delete mode 100644 server/views/errors/not_allowed.html.haml
 delete mode 100644 server/views/errors/not_allowed.xml.haml
 delete mode 100644 server/views/errors/not_found.html.haml
 delete mode 100644 server/views/errors/not_found.xml.haml
 delete mode 100644 server/views/errors/validation_failure.html.haml
 delete mode 100644 server/views/errors/validation_failure.xml.haml

diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb
index 38383d8..2514659 100644
--- a/server/lib/deltacloud/base_driver/base_driver.rb
+++ b/server/lib/deltacloud/base_driver/base_driver.rb
@@ -16,32 +16,17 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+require 'lib/deltacloud/base_driver/exceptions'
+
 module Deltacloud
 
-  class AuthException < Exception
-  end
+  class BaseDriver
 
-  class BackendError < StandardError
-    attr_reader :code, :cause, :details
-    def initialize(code, cause, message, details)
-      super(message)
-      @code = code
-      @cause = cause
-      @details = details
-    end
-  end
+    include ExceptionHandler
 
-  class BackendFeatureUnsupported < StandardError
-    attr_reader :code, :cause, :details
-    def initialize(code, cause, message, details)
-      super(message)
-      @code = code
-      @cause = cause
-      @details = details
+    def self.exceptions(&block)
+      ExceptionHandler::exceptions(&block)
     end
-  end
-
-  class BaseDriver
     
     def self.define_hardware_profile(name,&block)
       @hardware_profiles ||= []
@@ -235,32 +220,6 @@ module Deltacloud
       { :error => [], :auth => [], :glob => [] }
     end
 
-    def safely(&block)
-      begin
-        block.call
-      rescue => e
-        catched_exceptions_list[:auth].each do |ex|
-          if e.class == ex or e.class.name =~ ex or e.message =~ ex
-            raise Deltacloud::AuthException.new
-          end
-        end
-        catched_exceptions_list[:error].each do |ex|
-          if e.class == ex or e.class.name =~ ex or e.message =~ ex
-            raise Deltacloud::BackendError.new(502, e.class.to_s, e.message, e.backtrace) if e.class.name =~ ex
-          end
-        end
-        catched_exceptions_list[:glob].each do |ex|
-          if e.class == ex or e.class.name =~ ex or e.message =~ ex
-            raise Deltacloud::BackendError.new(500, e.class.to_s, e.message, e.backtrace)
-          end
-        end
-        $stderr.puts "# UNCAUGHT EXCEPTION # -> '#{e.class}' - '#{e.message}'"
-        $stderr.puts "# #{e.backtrace.join("\n")}"
-        $stderr.puts "##############"
-        raise Deltacloud::BackendError.new(500, e.class.to_s, e.message, e.backtrace)
-      end
-    end
-
   end
 
 end
diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb
index 345aaff..0113126 100644
--- a/server/lib/deltacloud/helpers/application_helper.rb
+++ b/server/lib/deltacloud/helpers/application_helper.rb
@@ -102,16 +102,16 @@ module ApplicationHelper
         format.json { convert_to_json(model, @element) }
       end
     else
-        report_error(404, 'not_found')
+        report_error(404)
     end
   end
 
-  def report_error(status, template)
+  def report_error(code=nil)
     @error = request.env['sinatra.error']
-    response.status = status
+    response.status = code || @error.code
     respond_to do |format|
-      format.xml { haml :"errors/#{template}", :layout => false }
-      format.html { haml :"errors/#{template}", :layout => :error }
+      format.xml { haml :"errors/#{code || @error.code}", :layout => false }
+      format.html { haml :"errors/#{code || @error.code}", :layout => :error }
     end
   end
 
@@ -121,7 +121,7 @@ module ApplicationHelper
     # If original instance doesn't include called action
     # return with 405 error (Method is not Allowed)
     unless driver.instance_actions_for(original_instance.state).include?(name.to_sym)
-      return report_error(405, 'not_allowed')
+      return report_error(405)
     end
 
     @instance = driver.send(:"#{name}_instance", credentials, params["id"])
diff --git a/server/lib/deltacloud/validation.rb b/server/lib/deltacloud/validation.rb
index cfac4ce..93444b7 100644
--- a/server/lib/deltacloud/validation.rb
+++ b/server/lib/deltacloud/validation.rb
@@ -26,7 +26,7 @@ module Deltacloud::Validation
     end
 
     def name
-      param.name
+      param.name if @param
     end
   end
 
diff --git a/server/lib/sinatra/rabbit.rb b/server/lib/sinatra/rabbit.rb
index 8be2b8b..730f4d7 100644
--- a/server/lib/sinatra/rabbit.rb
+++ b/server/lib/sinatra/rabbit.rb
@@ -25,10 +25,20 @@ module Sinatra
 
   module Rabbit
 
-    class DuplicateParamException < Exception; end
-    class DuplicateOperationException < Exception; end
-    class DuplicateCollectionException < Exception; end
-    class UnsupportedCollectionException < Exception; end
+    class DuplicateParamException < Deltacloud::ExceptionHandler::DeltacloudException; end
+    class DuplicateOperationException < Deltacloud::ExceptionHandler::DeltacloudException; end
+    class DuplicateCollectionException < Deltacloud::ExceptionHandler::DeltacloudException; end
+    class UnsupportedCollectionException < Deltacloud::ExceptionHandler::DeltacloudException
+      def initialize
+        @details = "This collection is not supported for this provider."
+        @message = @details
+        # 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
+        # to make this information available to the client, the status code 404 (Not Found) can be used instead.
+        @code = 403 # 
+      end
+    end
 
     class Operation
       attr_reader :name, :method, :collection
@@ -275,8 +285,7 @@ module Sinatra
 
       def check_supported(driver)
         unless global? || driver.has_collection?(@name)
-          raise UnsupportedCollectionException,
-            "Collection #{@name} not supported by this driver"
+          raise UnsupportedCollectionException
         end
       end
     end
diff --git a/server/server.rb b/server/server.rb
index 586a9af..ebaf3f1 100644
--- a/server/server.rb
+++ b/server/server.rb
@@ -59,24 +59,8 @@ end
 # whatever you want (eg. if you running API behind NAT)
 HOSTNAME=ENV['API_HOST'] ? ENV['API_HOST'] : nil
 
-error Deltacloud::Validation::Failure do
-  report_error(400, "validation_failure")
-end
-
-error Deltacloud::BackendCapability::Failure do
-  report_error(405, "backend_capability_failure")
-end
-
-error Deltacloud::AuthException do
-  report_error(401, "auth_exception")
-end
-
-error Deltacloud::BackendError do
-  report_error(500, "backend_error")
-end
-
-error Sinatra::Rabbit::UnsupportedCollectionException do
-  report_error(404, "not_found")
+error do
+  report_error
 end
 
 Sinatra::Application.register Sinatra::RespondTo
@@ -495,7 +479,7 @@ END
           format.json { convert_to_json(:hardware_profile, @profile) }
         end
       else
-        report_error(404, 'not_found')
+        report_error(404)
       end
     end
   end
@@ -743,7 +727,7 @@ head '/api/buckets/:bucket/:blob' do
         headers["X-Deltacloud-Blobmeta-#{k}"] = v
       end
    else
-    report_error(404, 'not_found')
+    report_error(404)
   end
 end
 
@@ -757,7 +741,7 @@ post '/api/buckets/:bucket/:blob' do
       headers["X-Deltacloud-Blobmeta-#{k}"] = v
     end
   else
-    report_error(404, 'not_found') #FIXME is this the right error code?
+    report_error(404) #FIXME is this the right error code?
   end
 end
 
@@ -771,7 +755,7 @@ get '/api/buckets/:bucket/:blob' do
       format.json { convert_to_json(blobs, @blob) }
       end
   else
-      report_error(404, 'not_found')
+      report_error(404)
   end
 end
 
@@ -784,7 +768,7 @@ get '/api/buckets/:bucket/:blob/content' do
     params['content_disposition'] = "attachment; filename=#{@blob.id}"
     BlobStream.call(env, credentials, params)
   else
-    report_error(404, 'not_found')
+    report_error(404)
   end
 end
 
diff --git a/server/views/errors/400.html.haml b/server/views/errors/400.html.haml
new file mode 100644
index 0000000..1d3ee48
--- /dev/null
+++ b/server/views/errors/400.html.haml
@@ -0,0 +1,11 @@
+%h1 Validation Failure
+
+%p= @error.message
+
+%dl
+  %di
+    %dt Request URL
+    %dd= request.env['REQUEST_URI']
+  %di
+    %dt Parameter
+    %dd= @error.name
diff --git a/server/views/errors/400.xml.haml b/server/views/errors/400.xml.haml
new file mode 100644
index 0000000..4f2723f
--- /dev/null
+++ b/server/views/errors/400.xml.haml
@@ -0,0 +1,4 @@
+%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
+  %parameter #{@error.name}
+  %message< #{cdata @error.message}
+
diff --git a/server/views/errors/401.html.haml b/server/views/errors/401.html.haml
new file mode 100644
index 0000000..1226587
--- /dev/null
+++ b/server/views/errors/401.html.haml
@@ -0,0 +1,8 @@
+%h1 Authentication Failure
+
+%p= @error.message
+
+%dl
+  %di
+    %dt Request URL
+    %dd= request.env['REQUEST_URI']
diff --git a/server/views/errors/401.xml.haml b/server/views/errors/401.xml.haml
new file mode 100644
index 0000000..bfa9111
--- /dev/null
+++ b/server/views/errors/401.xml.haml
@@ -0,0 +1,3 @@
+%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
+  %message< #{cdata @error.message}
+
diff --git a/server/views/errors/403.html.haml b/server/views/errors/403.html.haml
new file mode 100644
index 0000000..59e9fe9
--- /dev/null
+++ b/server/views/errors/403.html.haml
@@ -0,0 +1,6 @@
+%h1 Method Not Allowed
+
+%dl
+  %di
+    %dt Request URL
+    %dd= request.env['REQUEST_URI']
diff --git a/server/views/errors/403.xml.haml b/server/views/errors/403.xml.haml
new file mode 100644
index 0000000..3bad335
--- /dev/null
+++ b/server/views/errors/403.xml.haml
@@ -0,0 +1,2 @@
+%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
+  %message Method not allowed for this resource
diff --git a/server/views/errors/404.html.haml b/server/views/errors/404.html.haml
new file mode 100644
index 0000000..dfbb116
--- /dev/null
+++ b/server/views/errors/404.html.haml
@@ -0,0 +1,6 @@
+%h1 Not Found
+
+%dl
+  %di
+    %dt Request URL
+    %dd= request.env['REQUEST_URI']
diff --git a/server/views/errors/404.xml.haml b/server/views/errors/404.xml.haml
new file mode 100644
index 0000000..38840f3
--- /dev/null
+++ b/server/views/errors/404.xml.haml
@@ -0,0 +1,2 @@
+%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
+  %message Not Found
diff --git a/server/views/errors/500.xml.haml b/server/views/errors/500.xml.haml
new file mode 100644
index 0000000..87c8e19
--- /dev/null
+++ b/server/views/errors/500.xml.haml
@@ -0,0 +1,8 @@
+%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
+  %kind backend_error
+  %backend{ :driver => driver_symbol }
+    %code= @error.code
+    %cause= @error.cause
+    - if @error.details
+      %details< #{cdata @error.details.join("\n")}
+  %message< #{cdata @error.message}
diff --git a/server/views/errors/502.html.haml b/server/views/errors/502.html.haml
new file mode 100644
index 0000000..b6b3e1b
--- /dev/null
+++ b/server/views/errors/502.html.haml
@@ -0,0 +1,14 @@
+%h1 Backend Error
+
+%p= @error.message
+
+%dl
+  %di
+    %dt Request URL
+    %dd= request.env['REQUEST_URI']
+  %di
+    %dt Parameters
+    %dd= params.inspect
+  %di
+    %dt Code
+    %dd= @error.code
diff --git a/server/views/errors/502.xml.haml b/server/views/errors/502.xml.haml
new file mode 100644
index 0000000..87c8e19
--- /dev/null
+++ b/server/views/errors/502.xml.haml
@@ -0,0 +1,8 @@
+%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
+  %kind backend_error
+  %backend{ :driver => driver_symbol }
+    %code= @error.code
+    %cause= @error.cause
+    - if @error.details
+      %details< #{cdata @error.details.join("\n")}
+  %message< #{cdata @error.message}
diff --git a/server/views/errors/auth_exception.html.haml b/server/views/errors/auth_exception.html.haml
deleted file mode 100644
index 1226587..0000000
--- a/server/views/errors/auth_exception.html.haml
+++ /dev/null
@@ -1,8 +0,0 @@
-%h1 Authentication Failure
-
-%p= @error.message
-
-%dl
-  %di
-    %dt Request URL
-    %dd= request.env['REQUEST_URI']
diff --git a/server/views/errors/auth_exception.xml.haml b/server/views/errors/auth_exception.xml.haml
deleted file mode 100644
index bfa9111..0000000
--- a/server/views/errors/auth_exception.xml.haml
+++ /dev/null
@@ -1,3 +0,0 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %message< #{cdata @error.message}
-
diff --git a/server/views/errors/backend_error.html.haml b/server/views/errors/backend_error.html.haml
deleted file mode 100644
index fb73386..0000000
--- a/server/views/errors/backend_error.html.haml
+++ /dev/null
@@ -1,22 +0,0 @@
-%h1 Backend Error
-
-%p= @error.message
-
-%dl
-  %di
-    %dt Request URL
-    %dd= request.env['REQUEST_URI']
-  %di
-    %dt Parameters
-    %dd= params.inspect
-  %di
-    %dt Code
-    %dd= @error.code
-  %di
-    %dt Cause
-    %dd= @error.cause
-  %di
-    %dt Details
-    %dd
-      %pre
-        =@error.details.join("\n")
diff --git a/server/views/errors/backend_error.xml.haml b/server/views/errors/backend_error.xml.haml
deleted file mode 100644
index 87c8e19..0000000
--- a/server/views/errors/backend_error.xml.haml
+++ /dev/null
@@ -1,8 +0,0 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %kind backend_error
-  %backend{ :driver => driver_symbol }
-    %code= @error.code
-    %cause= @error.cause
-    - if @error.details
-      %details< #{cdata @error.details.join("\n")}
-  %message< #{cdata @error.message}
diff --git a/server/views/errors/not_allowed.html.haml b/server/views/errors/not_allowed.html.haml
deleted file mode 100644
index 59e9fe9..0000000
--- a/server/views/errors/not_allowed.html.haml
+++ /dev/null
@@ -1,6 +0,0 @@
-%h1 Method Not Allowed
-
-%dl
-  %di
-    %dt Request URL
-    %dd= request.env['REQUEST_URI']
diff --git a/server/views/errors/not_allowed.xml.haml b/server/views/errors/not_allowed.xml.haml
deleted file mode 100644
index 3bad335..0000000
--- a/server/views/errors/not_allowed.xml.haml
+++ /dev/null
@@ -1,2 +0,0 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %message Method not allowed for this resource
diff --git a/server/views/errors/not_found.html.haml b/server/views/errors/not_found.html.haml
deleted file mode 100644
index dfbb116..0000000
--- a/server/views/errors/not_found.html.haml
+++ /dev/null
@@ -1,6 +0,0 @@
-%h1 Not Found
-
-%dl
-  %di
-    %dt Request URL
-    %dd= request.env['REQUEST_URI']
diff --git a/server/views/errors/not_found.xml.haml b/server/views/errors/not_found.xml.haml
deleted file mode 100644
index 38840f3..0000000
--- a/server/views/errors/not_found.xml.haml
+++ /dev/null
@@ -1,2 +0,0 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %message Not Found
diff --git a/server/views/errors/validation_failure.html.haml b/server/views/errors/validation_failure.html.haml
deleted file mode 100644
index 1d3ee48..0000000
--- a/server/views/errors/validation_failure.html.haml
+++ /dev/null
@@ -1,11 +0,0 @@
-%h1 Validation Failure
-
-%p= @error.message
-
-%dl
-  %di
-    %dt Request URL
-    %dd= request.env['REQUEST_URI']
-  %di
-    %dt Parameter
-    %dd= @error.name
diff --git a/server/views/errors/validation_failure.xml.haml b/server/views/errors/validation_failure.xml.haml
deleted file mode 100644
index f18d6a2..0000000
--- a/server/views/errors/validation_failure.xml.haml
+++ /dev/null
@@ -1,8 +0,0 @@
-%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
-  %parameter #{@error.name}
-  %message< #{cdata @error.message}
-  - unless @error.param.options.empty?
-    %valid_options
-      - @error.param.options.each do |v|
-        %value< #{cdata v}
-
-- 
1.7.4.1


Re: New DSL for handling Exceptions and error reporting

Posted by Michal Fojtik <mf...@redhat.com>.
On Apr 5, 2011, at 1:49 AM, David Lutterkort wrote:

> Hi Michal,
> 
> On Mon, 2011-04-04 at 16:02 +0200, mfojtik@redhat.com wrote:
>> This set of patches will add 'better' error/exceptions reporting. I tried to make this
>> thing very simple and avoid voodoo we had in 'catched_exceptions' list.
>> 
>> No you can use simple DSL (like features) to catching and describing exceptions from
>> our beloved 'safely' blocks.
>> 
>> Example usage:
>> 
>> exceptions do
>> 
>>  on /AuthFailure/ do
>>    status 401 # Authentication failed...
>>  end
>> 
>>  on /No offering found/ do
>>    status 400  # Wrong combination of hardware_profile / image
>>  end
>> 
>>  on /CloudServers::Exception::(\w+)/ do
>>    status 502  # Provider error
>>  end
>> 
>>  on /Error/
>>    status 500
>>    message "Hello, this is a nice message to client."
>>    exception CustomException
>>  end
>> 
>> end
>> 
>> Let me know what do you think. All ideas how to improve it are welcome.
> 
> I still find this overengineered, and it doesn't address the main
> shortcoming of the current error handling scheme: that it's not possible
> to dispatch on criteria that seem more reliable than matching on
> specific words in the error message.

This DSL is not supposed to work in way that it only sticks to error messages
(exception messages). You can also match exception class like:

/CloudServers::Exception::(\w+)/

Or you can combine them.

> For example, AWS only throws AwsError, but the HTTP status code in the
> exception is useful in some situations to cleanly and reliably generate
> the appropriate response; for example to handle authentication failures,
> you can check whether the AwsError has a status of 401.

I think this error code is 'rethrowned' from Net::HTTP class and it's available
in message. So we can do: '/AwsError.*401.*/' to achieve same effect.

> Having the DSL is nice, but I'd prefer a more flexible way to handle all
> that even if it requires more manual work; to me something like Rails'
> rescue_from is as far as I'd want to take this.

I though that rescue_from helper works just with classes, like:

rescue_from NoMethodError, :with => :show_error

In some cases this can be not applicable to our code, because some gems throwing
errors as part of Exception message (AwsError).

From AwsError you can get both 401 code with message and also InvalidAPICredentials
in case you enter wrong credentials.

I can easily add 'class' matching to this DSL and it will work in same way as rescue_from:

on NoMethodError do
  status xxx
  render :no_method_error
end

  -- Michal

------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org


Re: New DSL for handling Exceptions and error reporting

Posted by David Lutterkort <lu...@redhat.com>.
Hi Michal,

On Mon, 2011-04-04 at 16:02 +0200, mfojtik@redhat.com wrote:
> This set of patches will add 'better' error/exceptions reporting. I tried to make this
> thing very simple and avoid voodoo we had in 'catched_exceptions' list.
> 
> No you can use simple DSL (like features) to catching and describing exceptions from
> our beloved 'safely' blocks.
> 
> Example usage:
> 
> exceptions do
> 
>   on /AuthFailure/ do
>     status 401 # Authentication failed...
>   end
> 
>   on /No offering found/ do
>     status 400  # Wrong combination of hardware_profile / image
>   end
> 
>   on /CloudServers::Exception::(\w+)/ do
>     status 502  # Provider error
>   end
> 
>   on /Error/
>     status 500
>     message "Hello, this is a nice message to client."
>     exception CustomException
>   end
> 
> end
> 
> Let me know what do you think. All ideas how to improve it are welcome.

I still find this overengineered, and it doesn't address the main
shortcoming of the current error handling scheme: that it's not possible
to dispatch on criteria that seem more reliable than matching on
specific words in the error message.

For example, AWS only throws AwsError, but the HTTP status code in the
exception is useful in some situations to cleanly and reliably generate
the appropriate response; for example to handle authentication failures,
you can check whether the AwsError has a status of 401.

Having the DSL is nice, but I'd prefer a more flexible way to handle all
that even if it requires more manual work; to me something like Rails'
rescue_from is as far as I'd want to take this.

David



[PATCH core 3/3] Adapted drivers to use new exception handling DSL

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>

---
 server/lib/deltacloud/drivers/ec2/ec2_driver.rb    |   18 ++++++++++-----
 .../lib/deltacloud/drivers/ec2/ec2_mock_driver.rb  |    2 +-
 server/lib/deltacloud/drivers/mock/mock_driver.rb  |   20 ++++++++++------
 .../drivers/rackspace/rackspace_driver.rb          |   24 +++++++++++++++-----
 .../drivers/rimuhosting/rimuhosting_client.rb      |    3 +-
 .../drivers/rimuhosting/rimuhosting_driver.rb      |    9 ++-----
 server/lib/deltacloud/drivers/sbc/sbc_client.rb    |    4 +-
 server/lib/deltacloud/drivers/sbc/sbc_driver.rb    |   12 ++++++++++
 .../drivers/terremark/terremark_driver.rb          |   13 ++++------
 9 files changed, 67 insertions(+), 38 deletions(-)

diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index 302aac2..04074c0 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -713,12 +713,18 @@ module Deltacloud
           end
         end
 
-        def catched_exceptions_list
-          {
-            :auth => [ /AuthFailure/ ],
-            :error => [ /Aws::AwsError/, /Error/ ],
-            :glob => [ /AWS::(\w+)/, /Deltacloud::Runner::(\w+)/ ]
-          }
+        exceptions do
+          on /(AuthFailure|InvalidAccessKeyId)/ do
+            status 401
+          end
+
+          on /Error/ do
+            status 502
+          end
+
+          on /Deltacloud::Runner::(\w+)/ do
+            status 500
+          end
         end
 
       end
diff --git a/server/lib/deltacloud/drivers/ec2/ec2_mock_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_mock_driver.rb
index 6a9c051..01fccea 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_mock_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_mock_driver.rb
@@ -180,7 +180,7 @@ Deltacloud::Drivers::EC2::EC2Driver.class_eval do
 
   def new_client(credentials, opts={})
     if credentials.user != 'mockuser' and credentials.password != 'mockpassword'
-      raise Deltacloud::AuthException.new
+      raise "AuthFailure"
     end
     RightAws::MockEc2.new
   end
diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb b/server/lib/deltacloud/drivers/mock/mock_driver.rb
index a4f9e11..16ac76e 100644
--- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
+++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
@@ -492,7 +492,7 @@ class MockDriver < Deltacloud::BaseDriver
     begin
       check_credentials(credentials)
       return true
-    rescue Deltacloud::AuthException
+    rescue
     end
     return false
   end
@@ -501,16 +501,20 @@ class MockDriver < Deltacloud::BaseDriver
 
   def check_credentials(credentials)
     if ( credentials.user != 'mockuser' ) or ( credentials.password != 'mockpassword' )
-      raise Deltacloud::AuthException.new
+      raise 'AuthFailure'
     end
   end
 
-  def catched_exceptions_list
-    {
-      :auth => [],
-      :error => [ /Deltacloud::BackendError/, /Errno::ENOENT/ ],
-      :glob => [ /Error/ ]
-    }
+  exceptions do
+
+    on /AuthFailure/ do
+      status 401
+    end
+
+    on /Err/ do
+      status 500
+    end
+
   end
 
 end
diff --git a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
index b5f0c27..3089398 100644
--- a/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
+++ b/server/lib/deltacloud/drivers/rackspace/rackspace_driver.rb
@@ -390,12 +390,24 @@ private
     end
   end
 
-  def catched_exceptions_list
-    {
-      :auth => [ /Authentication failed/ ],
-      :error => [ /Error/ ],
-      :glob => [ /CloudServers::Exception::(\w+)/, /Deltacloud::Runner::(\w+)/ ]
-    }
+  exceptions do
+
+    on /No offering found/ do
+      status 400 
+    end
+
+    on /Authentication failed/ do
+      status 401
+    end
+
+    on /Error/ do
+      status 500
+    end
+
+    on /CloudServers::Exception::(\w+)/ do
+      status 500
+    end
+
   end
 
   private
diff --git a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_client.rb b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_client.rb
index c1f26ff..9d1d821 100755
--- a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_client.rb
+++ b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_client.rb
@@ -40,6 +40,7 @@ class RimuHostingClient
     if(!@auth.nil?)
       headers["Authorization"] = @auth
     end
+    safely do
     r = @service.send_request(method, @uri.path + resource, data, headers)
          puts r.body
     res = JSON.parse(r.body)
@@ -47,7 +48,7 @@ class RimuHostingClient
 
     if(res['response_type'] == "ERROR" and ( (res['error_info']['error_class'] == "PermissionException") or
 					     (res['error_info']['error_class'] == "LoginRequired") )) 
-      raise Deltacloud::AuthException.new
+      raise "AuthFailure"
     end
     res
   end
diff --git a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
index ce5dc5f..6d37644 100755
--- a/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
+++ b/server/lib/deltacloud/drivers/rimuhosting/rimuhosting_driver.rb
@@ -149,15 +149,12 @@ class RimuHostingDriver < Deltacloud::BaseDriver
     stopped.to( :finish )         .automatically
   end
 
-  def safely(&block)
-    begin
-      block.call
-    rescue Exception => e
-      raise Deltacloud::BackendError.new(500, e.class.to_s, e.message, e.backtrace)
+  exceptions do
+    on /AuthFailure/ do
+      status 401
     end
   end
 
-
 end
 
     end
diff --git a/server/lib/deltacloud/drivers/sbc/sbc_client.rb b/server/lib/deltacloud/drivers/sbc/sbc_client.rb
index 7c6012a..23da4ff 100644
--- a/server/lib/deltacloud/drivers/sbc/sbc_client.rb
+++ b/server/lib/deltacloud/drivers/sbc/sbc_client.rb
@@ -182,9 +182,9 @@ class SBCClient
   #
   def backend_error!(resp)
     if resp.is_a?(Net::HTTPUnauthorized)
-      raise Deltacloud::AuthException, resp.message
+      raise "AuthFailure"
     else
-      raise Deltacloud::BackendError.new(resp.code, resp.body, resp.message, '')
+      raise "BackendError"
     end
   end
 
diff --git a/server/lib/deltacloud/drivers/sbc/sbc_driver.rb b/server/lib/deltacloud/drivers/sbc/sbc_driver.rb
index 1ef3d63..4ad20b5 100644
--- a/server/lib/deltacloud/drivers/sbc/sbc_driver.rb
+++ b/server/lib/deltacloud/drivers/sbc/sbc_driver.rb
@@ -120,6 +120,18 @@ class SBCDriver < Deltacloud::BaseDriver
     instance(credentials, instance_id)
   end
 
+  exceptions do
+
+    on /AuthFailure/ do
+      status 401
+    end
+
+    on /BackendError/ do
+      status 502
+    end
+
+  end
+
   #
   # --------------------- Private helpers ---------------------
   #
diff --git a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
index 6e876d4..83a9cf3 100644
--- a/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
+++ b/server/lib/deltacloud/drivers/terremark/terremark_driver.rb
@@ -194,7 +194,7 @@ end
 def valid_credentials?(credentials)
   begin
     new_client(credentials)
-  rescue Deltacloud::AuthException
+  rescue
     return false
   end
   true
@@ -274,20 +274,17 @@ end
       vdc_id = terremark_client.default_vdc_id
     end
     if (vdc_id.nil?)
-       raise DeltaCloud::AuthException.new
+       raise "AuthFailure"
     end
     terremark_client
   end
 
-  def safely(&block)
-    begin
-      block.call
-    rescue Exception => e
-      raise Deltacloud::BackendError.new(500, e.class.to_s, e.message, e.backtrace)
+  exceptions do
+    on /AuthFailure/ do
+      status 401
     end
   end
 
-
 end
 
     end
-- 
1.7.4.1


[PATCH core 1/3] Added DSL for handling exceptions

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>

---
 server/lib/deltacloud/base_driver/exceptions.rb |  127 +++++++++++++++++++++++
 1 files changed, 127 insertions(+), 0 deletions(-)
 create mode 100644 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
new file mode 100644
index 0000000..e7b6ac8
--- /dev/null
+++ b/server/lib/deltacloud/base_driver/exceptions.rb
@@ -0,0 +1,127 @@
+module Deltacloud
+  module ExceptionHandler
+
+    class DeltacloudException < StandardError
+
+      attr_accessor :code, :name, :message, :backtrace, :request
+
+      def initialize(code, name, message, backtrace, details, request=nil)
+        @code, @name, @message = code, name, message
+        @details = details
+        @backtrace = backtrace
+        @request = request
+        self
+      end
+
+    end
+
+    class AuthenticationFailure < DeltacloudException
+      def initialize(e, details)
+        super(401, e.class.name, e.message, e.backtrace, details)
+      end
+    end
+
+    class ValidationFailure < DeltacloudException
+      def initialize(e, details)
+        super(400, e.class.name, e.message, e.backtrace, details)
+      end
+    end
+
+    class BackendError < DeltacloudException
+      def initialize(e, details)
+        super(500, e.class.name, e.message, e.backtrace, details)
+      end
+    end
+
+    class ProviderError < DeltacloudException
+      def initialize(e, details)
+        super(502, e.class.name, e.message, e.backtrace, details)
+      end
+    end
+
+    class ExceptionDef
+      attr_accessor :status
+      attr_accessor :message
+      attr_reader   :conditions
+      attr_reader   :handler
+
+      def initialize(conditions, &block)
+        @conditions = conditions
+        instance_eval(&block) if block_given?
+      end
+
+      def status(code)
+        self.status = code
+      end
+
+      def message(message)
+        self.message = message
+      end
+
+      def details(details)
+        self.details = details
+      end
+
+      def exception(handler)
+        self.handler = handler
+      end
+
+      # Condition can be class or regexp
+      #
+      def match?(e)
+        @conditions.each do |c|
+          return true if c.class == Class && e.class == c
+          return true if c.class == Regexp && (e.class.name =~ c or e.message =~ c)
+        end
+        return false
+      end
+
+      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)
+        end
+      end
+
+    end
+
+    class Exceptions
+      attr_reader :exception_definitions
+
+      def initialize(&block)
+        @exception_definitions = []
+        instance_eval(&block) if block_given?
+        self
+      end
+
+      def on(*conditions, &block)
+        @exception_definitions << ExceptionDef::new(conditions, &block) if block_given?
+      end
+    end
+
+    def self.exceptions(&block)
+      @definitions = Exceptions.new(&block).exception_definitions if block_given?
+      @definitions 
+    end
+
+    def safely(&block)
+      begin
+        block.call
+      rescue => e
+        Deltacloud::ExceptionHandler::exceptions.each do |exdef|
+          raise exdef.handler(e) if exdef.match?(e)
+        end
+        $stderr.puts "# UNCAUGHT EXCEPTION  ~> '#{e.class}' - "
+        $stderr.puts "# #{e.message}"
+        $stderr.puts "# #{e.backtrace.join("\n")}"
+        $stderr.puts "##############"
+        raise BackendError.new(e)
+      end
+    end
+
+  end
+
+end
-- 
1.7.4.1