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 2012/02/10 13:56:00 UTC
[PATCH core 1/2] Core: Added XML output for 504 and 502 errors
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/deltacloud/base_driver/exceptions.rb | 16 +++++++
server/lib/deltacloud/drivers/mock/mock_driver.rb | 32 +++++++++++++-
.../lib/deltacloud/helpers/application_helper.rb | 2 +-
server/lib/sinatra/lazy_auth.rb | 2 +-
server/views/errors/501.html.haml | 43 ++++++++++++++++++++
server/views/errors/501.xml.haml | 12 +++++
server/views/errors/502.xml.haml | 13 ++++--
server/views/errors/504.html.haml | 43 ++++++++++++++++++++
server/views/errors/504.xml.haml | 12 +++++
9 files changed, 166 insertions(+), 9 deletions(-)
create mode 100644 server/views/errors/501.html.haml
create mode 100644 server/views/errors/501.xml.haml
create mode 100644 server/views/errors/504.html.haml
create mode 100644 server/views/errors/504.xml.haml
diff --git a/server/lib/deltacloud/base_driver/exceptions.rb b/server/lib/deltacloud/base_driver/exceptions.rb
index e44c89b..9effe29 100644
--- a/server/lib/deltacloud/base_driver/exceptions.rb
+++ b/server/lib/deltacloud/base_driver/exceptions.rb
@@ -72,6 +72,20 @@ module Deltacloud
end
end
+ class ProviderTimeout < DeltacloudException
+ def initialize(e, message)
+ message ||= e.message
+ super(504, e.class.name, message, e.backtrace)
+ end
+ end
+
+ class NotImplemented < DeltacloudException
+ def initialize(e, message)
+ message ||= e.message
+ super(501, e.class.name, message, e.backtrace)
+ end
+ end
+
class ObjectNotFound < DeltacloudException
def initialize(e, message)
message ||= e.message
@@ -121,7 +135,9 @@ module Deltacloud
when 405 then Deltacloud::ExceptionHandler::MethodNotAllowed.new(e, @message)
when 400 then Deltacloud::ExceptionHandler::ValidationFailure.new(e, @message)
when 500 then Deltacloud::ExceptionHandler::BackendError.new(e, @message)
+ when 501 then Deltacloud::ExceptionHandler::NotImplemented.new(e, @message)
when 502 then Deltacloud::ExceptionHandler::ProviderError.new(e, @message)
+ when 504 then Deltacloud::ExceptionHandler::ProviderTimeout.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 1f36b36..c0dfd2e 100644
--- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
+++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
@@ -100,8 +100,18 @@ module Deltacloud::Drivers::Mock
end
def realms(credentials, opts=nil)
- return REALMS if ( opts.nil? )
- results = REALMS
+ check_credentials( credentials )
+ results = []
+ safely do
+ # This hack is used to test if client capture exceptions correctly
+ # To raise an exception do GET /api/realms/50[0-2]
+ raise "DeltacloudErrorTest" if opts and opts[:id] == "500"
+ raise "NotImplementedTest" if opts and opts[:id] == "501"
+ raise "ProviderErrorTest" if opts and opts[:id] == "502"
+ raise "ProviderTimeoutTest" if opts and opts[:id] == "504"
+ return REALMS if ( opts.nil? )
+ results = REALMS
+ end
results = filter_on( results, :id, opts )
results
end
@@ -520,8 +530,24 @@ module Deltacloud::Drivers::Mock
message "Could not delete a non existent blob"
end
- on /Err/ do
+ on /DeltacloudErrorTest/ do
status 500
+ message "DeltacloudErrorMessage"
+ end
+
+ on /NotImplementedTest/ do
+ status 501
+ message "NotImplementedMessage"
+ end
+
+ on /ProviderErrorTest/ do
+ status 502
+ message "ProviderErrorMessage"
+ end
+
+ on /ProviderTimeoutTest/ do
+ status 504
+ message "ProviderTimeoutMessage"
end
end
diff --git a/server/lib/deltacloud/helpers/application_helper.rb b/server/lib/deltacloud/helpers/application_helper.rb
index d44f108..4353c4d 100644
--- a/server/lib/deltacloud/helpers/application_helper.rb
+++ b/server/lib/deltacloud/helpers/application_helper.rb
@@ -101,7 +101,7 @@ module ApplicationHelper
format.json { convert_to_json(model, @element) }
end
else
- report_error(404)
+ report_error(404)
end
end
diff --git a/server/lib/sinatra/lazy_auth.rb b/server/lib/sinatra/lazy_auth.rb
index ac8f5c7..9556bbc 100644
--- a/server/lib/sinatra/lazy_auth.rb
+++ b/server/lib/sinatra/lazy_auth.rb
@@ -63,7 +63,7 @@ module Sinatra
def authorize!
r = "#{driver_symbol}-deltacloud@#{HOSTNAME}"
response['WWW-Authenticate'] = %(Basic realm="#{r}")
- throw(:halt, [401, "Not authorized\n"])
+ report_error(401)
end
# Request the current user's credentials. Actual credentials are only
diff --git a/server/views/errors/501.html.haml b/server/views/errors/501.html.haml
new file mode 100644
index 0000000..19cf090
--- /dev/null
+++ b/server/views/errors/501.html.haml
@@ -0,0 +1,43 @@
+%div{ :'data-role' => :content, :'data-theme' => 'b'}
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ %li{ :'data-role' => 'list-divider'} Server message
+ %li
+ %h3=[@error.class.name, @error.message].join(' - ')
+ %li{ :'data-role' => 'list-divider'} Original request URI
+ %li
+ %a{ :href => request.env['REQUEST_URI'], :'data-ajax' => 'false'}
+ %span=request.env['REQUEST_URI']
+ %span{ :class => 'ui-li-count'} Retry
+ %li{ :'data-role' => 'list-divider'} Error details
+ %li
+ - if @error.class.method_defined? :details
+ %p= @error.details
+ - else
+ %em No details
+
+ %div{ 'data-role' => :collapsible, 'data-collapsed' => "true"}
+ %h3 Backtrace
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ %li
+ %pre=@error.backtrace.join("\n")
+
+ %div{ 'data-role' => :collapsible, 'data-collapsed' => "true"}
+ %h3 Parameters
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ - if params.keys.empty?
+ %li{ :'data-role' => 'list-divider'} No parameters
+ - params.each do |key, value|
+ - next if value.inspect.to_s == '#'
+ %li{ :'data-role' => 'list-divider'}=key
+ %li
+ %span{:style => 'font-weight:normal;'}=value.inspect
+
+
+ %div{ 'data-role' => :collapsible, 'data-collapsed' => "true"}
+ %h3 Request details
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ - request.env.each do |key, value|
+ - next if value.inspect.to_s == '#'
+ %li{ :'data-role' => 'list-divider'}=key
+ %li
+ %span{:style => 'font-weight:normal;'}=value.inspect
diff --git a/server/views/errors/501.xml.haml b/server/views/errors/501.xml.haml
new file mode 100644
index 0000000..788fe4b
--- /dev/null
+++ b/server/views/errors/501.xml.haml
@@ -0,0 +1,12 @@
+%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
diff --git a/server/views/errors/502.xml.haml b/server/views/errors/502.xml.haml
index 6e7a7b8..788fe4b 100644
--- a/server/views/errors/502.xml.haml
+++ b/server/views/errors/502.xml.haml
@@ -1,7 +1,12 @@
%error{:url => "#{request.env['REQUEST_URI']}", :status => "#{response.status}"}
%kind backend_error
- %backend{ :driver => driver_symbol }
- %code= @error.code
- - if @error.respond_to?(:details) && @error.details
- %details< #{cdata @error.details.join("\n")}
+ %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
diff --git a/server/views/errors/504.html.haml b/server/views/errors/504.html.haml
new file mode 100644
index 0000000..19cf090
--- /dev/null
+++ b/server/views/errors/504.html.haml
@@ -0,0 +1,43 @@
+%div{ :'data-role' => :content, :'data-theme' => 'b'}
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ %li{ :'data-role' => 'list-divider'} Server message
+ %li
+ %h3=[@error.class.name, @error.message].join(' - ')
+ %li{ :'data-role' => 'list-divider'} Original request URI
+ %li
+ %a{ :href => request.env['REQUEST_URI'], :'data-ajax' => 'false'}
+ %span=request.env['REQUEST_URI']
+ %span{ :class => 'ui-li-count'} Retry
+ %li{ :'data-role' => 'list-divider'} Error details
+ %li
+ - if @error.class.method_defined? :details
+ %p= @error.details
+ - else
+ %em No details
+
+ %div{ 'data-role' => :collapsible, 'data-collapsed' => "true"}
+ %h3 Backtrace
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ %li
+ %pre=@error.backtrace.join("\n")
+
+ %div{ 'data-role' => :collapsible, 'data-collapsed' => "true"}
+ %h3 Parameters
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ - if params.keys.empty?
+ %li{ :'data-role' => 'list-divider'} No parameters
+ - params.each do |key, value|
+ - next if value.inspect.to_s == '#'
+ %li{ :'data-role' => 'list-divider'}=key
+ %li
+ %span{:style => 'font-weight:normal;'}=value.inspect
+
+
+ %div{ 'data-role' => :collapsible, 'data-collapsed' => "true"}
+ %h3 Request details
+ %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
+ - request.env.each do |key, value|
+ - next if value.inspect.to_s == '#'
+ %li{ :'data-role' => 'list-divider'}=key
+ %li
+ %span{:style => 'font-weight:normal;'}=value.inspect
diff --git a/server/views/errors/504.xml.haml b/server/views/errors/504.xml.haml
new file mode 100644
index 0000000..788fe4b
--- /dev/null
+++ b/server/views/errors/504.xml.haml
@@ -0,0 +1,12 @@
+%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
--
1.7.4.4
Re: [PATCH core 1/2] Core: Added XML output for 504 and 502
errors
Posted by Michal Fojtik <mf...@redhat.com>.
On Thursday, February 16, 2012 at 2:02 PM, marios@redhat.com wrote:
> ACK, couple thoughts:
>
> On 10/02/12 14:56, mfojtik@redhat.com (mailto:mfojtik@redhat.com) wrote:
> > From: Michal Fojtik <mfojtik@redhat.com (mailto:mfojtik@redhat.com)>
> >
> >
> > Signed-off-by: Michal fojtik <mfojtik@redhat.com (mailto:mfojtik@redhat.com)>
> > ---
> > server/lib/deltacloud/base_driver/exceptions.rb | 16 +++++++
> > server/lib/deltacloud/drivers/mock/mock_driver.rb | 32 +++++++++++++-
> > .../lib/deltacloud/helpers/application_helper.rb | 2 +-
> > server/lib/sinatra/lazy_auth.rb | 2 +-
> > server/views/errors/501.html.haml | 43 ++++++++++++++++++++
> > server/views/errors/501.xml.haml | 12 +++++
> > server/views/errors/502.xml.haml | 13 ++++--
> > server/views/errors/504.html.haml | 43 ++++++++++++++++++++
> > server/views/errors/504.xml.haml | 12 +++++
>
>
>
> we really need all of these? like 501.html.haml 504.html.haml are
> identical, as are 501.xml.haml 502.xml.haml and 504.xml.haml. If we
> really want seperate files for 'neatness' perhaps just include them in
> each other? just thinking in terms of maintaining if we want to change
> something its gonna be messy.
I agree. We should probably create one generic error template and then just render
them in this files (or change the error reporting code completely for this).
Will do it in next revision.
> One more thought below:
>
> > diff --git a/server/lib/deltacloud/base_driver/exceptions.rb b/server/lib/deltacloud/base_driver/exceptions.rb
> > index e44c89b..9effe29 100644
> > --- a/server/lib/deltacloud/base_driver/exceptions.rb
> >
> > def realms(credentials, opts=nil)
> > - return REALMS if ( opts.nil? )
> > - results = REALMS
> > + check_credentials( credentials )
> > + results = []
> > + safely do
> > + # This hack is used to test if client capture exceptions correctly
> > + # To raise an exception do GET /api/realms/50[0-2]
> > + raise "DeltacloudErrorTest" if opts and opts[:id] == "500"
> > + raise "NotImplementedTest" if opts and opts[:id] == "501"
> > + raise "ProviderErrorTest" if opts and opts[:id] == "502"
> > + raise "ProviderTimeoutTest" if opts and opts[:id] == "504"
> > + return REALMS if ( opts.nil? )
> > + results = REALMS
> > + end
> > results = filter_on( results, :id, opts )
> > results
>
>
>
> you want to keep this stuff here? I saw that you are using them in
> client/specs/error_spec but feels a bit too 'hacky' - maybe define some
> special method just for this?
Well, yes they are hacky :-) But on the other side this is the most easy way
how to test/trigger exception handling in server/client.
This could be also extra useful for conductor guys to test their exception handling
code.
I was thinking about special method but that would need to be integrated to Rabbit
which can be a mess. I think we should keep this code there for now (to make our
client tests happy) and properly document this as a feature (exception testing.)
-- michal
Re: [PATCH core 1/2] Core: Added XML output for 504 and 502 errors
Posted by "marios@redhat.com" <ma...@redhat.com>.
ACK, couple thoughts:
On 10/02/12 14:56, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
>
>
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
> server/lib/deltacloud/base_driver/exceptions.rb | 16 +++++++
> server/lib/deltacloud/drivers/mock/mock_driver.rb | 32 +++++++++++++-
> .../lib/deltacloud/helpers/application_helper.rb | 2 +-
> server/lib/sinatra/lazy_auth.rb | 2 +-
> server/views/errors/501.html.haml | 43 ++++++++++++++++++++
> server/views/errors/501.xml.haml | 12 +++++
> server/views/errors/502.xml.haml | 13 ++++--
> server/views/errors/504.html.haml | 43 ++++++++++++++++++++
> server/views/errors/504.xml.haml | 12 +++++
we really need all of these? like 501.html.haml 504.html.haml are
identical, as are 501.xml.haml 502.xml.haml and 504.xml.haml. If we
really want seperate files for 'neatness' perhaps just include them in
each other? just thinking in terms of maintaining if we want to change
something its gonna be messy.
One more thought below:
> diff --git a/server/lib/deltacloud/base_driver/exceptions.rb b/server/lib/deltacloud/base_driver/exceptions.rb
> index e44c89b..9effe29 100644
> --- a/server/lib/deltacloud/base_driver/exceptions.rb
>
> def realms(credentials, opts=nil)
> - return REALMS if ( opts.nil? )
> - results = REALMS
> + check_credentials( credentials )
> + results = []
> + safely do
> + # This hack is used to test if client capture exceptions correctly
> + # To raise an exception do GET /api/realms/50[0-2]
> + raise "DeltacloudErrorTest" if opts and opts[:id] == "500"
> + raise "NotImplementedTest" if opts and opts[:id] == "501"
> + raise "ProviderErrorTest" if opts and opts[:id] == "502"
> + raise "ProviderTimeoutTest" if opts and opts[:id] == "504"
> + return REALMS if ( opts.nil? )
> + results = REALMS
> + end
> results = filter_on( results, :id, opts )
> results
you want to keep this stuff here? I saw that you are using them in
client/specs/error_spec but feels a bit too 'hacky' - maybe define some
special method just for this?
marios