deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Fojtik <mfoj...@redhat.com>
Subject Re: [PATCH core 2/2] Client: Added a better way how to handle client/server errors
Date Thu, 16 Feb 2012 13:19:34 GMT
On Thursday, February 16, 2012 at 2:07 PM, marios@redhat.com wrote:
> ACK.
>  
> Can you add 's.has_rdoc' to the deltacloud-client.gemspec (couldn't
> build it without)
>  
> I played with the client errors, like client.realms("foo") - looks
> good. Can we do something about 401? I tried like:
>  
> client = DeltaCloud.new( "foo", "bar", "http://localhost:3001/api" )
> client.images
>  
>   
> but instead of 401 Auth Exception I got NoMethodError: undefined method
> `[]' for nil:NilClass


Yes, I hit this error too and I was not sure how or from where it come. Seems like
we're using something weird to report 401 error (need to check the server code)…

Anyway if you want to look into this issue I will be more than happy :)
  
>  
> ( I can take a look at this if you like - I think we might need to
> change something server side too... like with valid_credentials? )
>  
> marios
>  
>  
> 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)>
> > ---
> > client/lib/deltacloud.rb | 67 +++++++++-----------------
> > client/lib/errors.rb | 111 +++++++++++++++++++++++++++++++++++++++++++
> > client/specs/errors_spec.rb | 59 +++++++++++++++++++++++
> > 3 files changed, 193 insertions(+), 44 deletions(-)
> > create mode 100644 client/lib/errors.rb
> > create mode 100644 client/specs/errors_spec.rb
> >  
> > diff --git a/client/lib/deltacloud.rb b/client/lib/deltacloud.rb
> > index 614eab2..97722de 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_successfull?(response.code)
> > yield obj if block_given?
> > end
> > return obj
> > @@ -349,6 +348,23 @@ module DeltaCloud
> > headers
> > end
> >  
> > + def response_successfull?(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)
> > + xml = Nokogiri::XML(response.to_s)
> > + opts = response.code.to_s =~ /^5(\d{2})$/ ? {
> > + :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.raise_error(xml.root[:status] || response.code, (xml/'message').first.text,
opts, backtrace)
> > + end
> > +
> > # Basic request method
> > #
> > def request(*args, &block)
> > @@ -367,55 +383,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_successfull? 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_successfull? 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)
> > diff --git a/client/lib/errors.rb b/client/lib/errors.rb
> > new file mode 100644
> > index 0000000..60d59e1
> > --- /dev/null
> > +++ b/client/lib/errors.rb
> > @@ -0,0 +1,111 @@
> > +# 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, opts={}, backtrace=nil, &block)
> > + @http_status_code = status_code.to_i
> > + @trace = backtrace
> > + @message = message
> > + @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
> > +
> > + end
> > +
> > + def self.parse_response_error(response)
> > +  
> > + end
> > +
> > + def self.raise_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
> > 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
>  




Mime
View raw message