From 40ffc17c85ebf217dd2cd6260bb073c047eb15d1 Mon Sep 17 00:00:00 2001 From: Noorul Islam K M Date: Sun, 23 Mar 2014 17:07:59 +0530 Subject: [PATCH] Fix client error handling Whenever server returns error, an exception is thrown. This is because pecan is returning json attributes that is not recongnized by apiclient. So fix apiclient to accept pecan response. Fixes-bug: #1293379 Change-Id: I5d8be574143c808a8e236d19c899358d784fc245 --- solumclient/builder/client.py | 6 ++- solumclient/client.py | 6 ++- solumclient/common/client.py | 65 +++++++++++++++++++++++ solumclient/common/exc.py | 44 ++++++++++++++++ solumclient/tests/common/test_auth.py | 2 +- solumclient/tests/common/test_client.py | 69 +++++++++++++++++++++++++ solumclient/tests/common/test_exc.py | 53 +++++++++++++++++++ 7 files changed, 240 insertions(+), 5 deletions(-) create mode 100644 solumclient/common/client.py create mode 100644 solumclient/tests/common/test_client.py create mode 100644 solumclient/tests/common/test_exc.py diff --git a/solumclient/builder/client.py b/solumclient/builder/client.py index 7b222a3..3b4f796 100644 --- a/solumclient/builder/client.py +++ b/solumclient/builder/client.py @@ -13,7 +13,8 @@ # under the License. from solumclient.common import auth -from solumclient.openstack.common.apiclient import client +from solumclient.common import client +from solumclient.openstack.common.apiclient import client as api_client API_NAME = 'builder' VERSION_MAP = { @@ -22,7 +23,8 @@ VERSION_MAP = { def Client(version, **kwargs): - client_class = client.BaseClient.get_class(API_NAME, version, VERSION_MAP) + client_class = api_client.BaseClient.get_class(API_NAME, version, + VERSION_MAP) keystone_auth = auth.KeystoneAuthPlugin( username=kwargs.get('username'), password=kwargs.get('password'), diff --git a/solumclient/client.py b/solumclient/client.py index 8b2bbbd..71610e1 100644 --- a/solumclient/client.py +++ b/solumclient/client.py @@ -13,7 +13,8 @@ # under the License. from solumclient.common import auth -from solumclient.openstack.common.apiclient import client +from solumclient.common import client +from solumclient.openstack.common.apiclient import client as api_client API_NAME = 'solum' VERSION_MAP = { @@ -22,7 +23,8 @@ VERSION_MAP = { def Client(version, **kwargs): - client_class = client.BaseClient.get_class(API_NAME, version, VERSION_MAP) + client_class = api_client.BaseClient.get_class(API_NAME, version, + VERSION_MAP) keystone_auth = auth.KeystoneAuthPlugin( username=kwargs.get('username'), password=kwargs.get('password'), diff --git a/solumclient/common/client.py b/solumclient/common/client.py new file mode 100644 index 0000000..c637e85 --- /dev/null +++ b/solumclient/common/client.py @@ -0,0 +1,65 @@ +# Copyright 2013 - Noorul Islam K M +# +# Licensed 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. + +import logging +import time + +from solumclient.common import exc +from solumclient.openstack.common.apiclient import client as api_client + + +_logger = logging.getLogger(__name__) + + +class HTTPClient(api_client.HTTPClient): + def request(self, method, url, **kwargs): + """Send an http request with the specified characteristics. + + Wrapper around `requests.Session.request` to handle tasks such as + setting headers, JSON encoding/decoding, and error handling. + + :param method: method of HTTP request + :param url: URL of HTTP request + :param kwargs: any other parameter that can be passed to +' requests.Session.request (such as `headers`) or `json` + that will be encoded as JSON and used as `data` argument + """ + kwargs.setdefault("headers", kwargs.get("headers", {})) + kwargs["headers"]["User-Agent"] = self.user_agent + if self.original_ip: + kwargs["headers"]["Forwarded"] = "for=%s;by=%s" % ( + self.original_ip, self.user_agent) + if self.timeout is not None: + kwargs.setdefault("timeout", self.timeout) + kwargs.setdefault("verify", self.verify) + if self.cert is not None: + kwargs.setdefault("cert", self.cert) + self.serialize(kwargs) + + self._http_log_req(method, url, kwargs) + if self.timings: + start_time = time.time() + resp = self.http.request(method, url, **kwargs) + if self.timings: + self.times.append(("%s %s" % (method, url), + start_time, time.time())) + self._http_log_resp(resp) + + if resp.status_code >= 400: + _logger.debug( + "Request returned failure status: %s", + resp.status_code) + raise exc.from_response(resp, method, url) + + return resp diff --git a/solumclient/common/exc.py b/solumclient/common/exc.py index e507eb6..42f93c5 100644 --- a/solumclient/common/exc.py +++ b/solumclient/common/exc.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +from solumclient.openstack.common.apiclient import exceptions + class BaseException(Exception): """An error occurred.""" @@ -24,3 +26,45 @@ class BaseException(Exception): class CommandError(BaseException): """Invalid usage of CLI.""" + + +def from_response(response, method, url): + """Returns an instance of :class:`HttpError` or subclass based on response. + + :param response: instance of `requests.Response` class + :param method: HTTP method used for request + :param url: URL used for request + """ + kwargs = { + "http_status": response.status_code, + "response": response, + "method": method, + "url": url, + "request_id": response.headers.get("x-compute-request-id"), + } + if "retry-after" in response.headers: + kwargs["retry_after"] = response.headers["retry-after"] + + content_type = response.headers.get("Content-Type", "") + if content_type.startswith("application/json"): + try: + body = response.json() + except ValueError: + pass + else: + if isinstance(body, dict): + kwargs["message"] = body.get("faultstring") + kwargs["details"] = body.get("debuginfo") + elif content_type.startswith("text/"): + kwargs["details"] = response.text + + try: + cls = exceptions._code_map[response.status_code] + except KeyError: + if 500 <= response.status_code < 600: + cls = exceptions.HttpServerError + elif 400 <= response.status_code < 500: + cls = exceptions.HTTPClientError + else: + cls = exceptions.HttpError + return cls(**kwargs) diff --git a/solumclient/tests/common/test_auth.py b/solumclient/tests/common/test_auth.py index 228da1e..c21ce35 100644 --- a/solumclient/tests/common/test_auth.py +++ b/solumclient/tests/common/test_auth.py @@ -16,7 +16,7 @@ from keystoneclient.v2_0 import client as ksclient import mock from solumclient.common import auth -from solumclient.openstack.common.apiclient import client +from solumclient.common import client from solumclient.tests import base diff --git a/solumclient/tests/common/test_client.py b/solumclient/tests/common/test_client.py new file mode 100644 index 0000000..ddf73fb --- /dev/null +++ b/solumclient/tests/common/test_client.py @@ -0,0 +1,69 @@ +# Copyright 2013 - Noorul Islam K M +# +# Licensed 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. + +import mock +import requests + +from solumclient.common import client +from solumclient.openstack.common.apiclient import auth +from solumclient.openstack.common.apiclient import client as api_client +from solumclient.openstack.common.apiclient import exceptions +from solumclient.tests import base + + +class TestClient(api_client.BaseClient): + service_type = "test" + + +class FakeAuthPlugin(auth.BaseAuthPlugin): + auth_system = "fake" + attempt = -1 + + def _do_authenticate(self, http_client): + pass + + def token_and_endpoint(self, endpoint_type, service_type): + self.attempt = self.attempt + 1 + return ("token-%s" % self.attempt, "/endpoint-%s" % self.attempt) + + +class ClientTest(base.TestCase): + def test_client_request(self): + http_client = client.HTTPClient(FakeAuthPlugin()) + mock_request = mock.Mock() + mock_request.return_value = requests.Response() + mock_request.return_value.status_code = 200 + with mock.patch("requests.Session.request", mock_request): + http_client.client_request( + TestClient(http_client), "GET", "/resource", json={"1": "2"}) + requests.Session.request.assert_called_with( + "GET", + "/endpoint-0/resource", + headers={ + "User-Agent": http_client.user_agent, + "Content-Type": "application/json", + "X-Auth-Token": "token-0" + }, + data='{"1": "2"}', + verify=True) + + def test_client_with_response_404_status_code(self): + http_client = client.HTTPClient(FakeAuthPlugin()) + mock_request = mock.Mock() + mock_request.return_value = requests.Response() + mock_request.return_value.status_code = 404 + with mock.patch("requests.Session.request", mock_request): + self.assertRaises( + exceptions.HttpError, http_client.client_request, + TestClient(http_client), "GET", "/resource") diff --git a/solumclient/tests/common/test_exc.py b/solumclient/tests/common/test_exc.py new file mode 100644 index 0000000..a1ca74d --- /dev/null +++ b/solumclient/tests/common/test_exc.py @@ -0,0 +1,53 @@ +# Copyright 2013 - Noorul Islam K M +# +# Licensed 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. + +import six + +from solumclient.common import exc +from solumclient.openstack.common.apiclient import exceptions +from solumclient.tests import base + + +class FakeResponse(object): + json_data = {} + + def __init__(self, **kwargs): + for key, value in six.iteritems(kwargs): + setattr(self, key, value) + + def json(self): + return self.json_data + + +class ExceptionTest(base.TestCase): + + def test_from_response_with_status_code_404(self): + json_data = {"faultstring": "fake message", + "debuginfo": "fake details"} + method = 'GET' + status_code = 404 + url = 'http://example.com:9777/v1/assemblies/fake-id' + ex = exc.from_response( + FakeResponse(status_code=status_code, + headers={"Content-Type": "application/json"}, + json_data=json_data), + method, + url + ) + self.assertTrue(isinstance(ex, exceptions.HttpError)) + self.assertEqual(json_data["faultstring"], ex.message) + self.assertEqual(json_data["debuginfo"], ex.details) + self.assertEqual(method, ex.method) + self.assertEqual(url, ex.url) + self.assertEqual(status_code, ex.http_status)