Handle faultstring when using SessionClient
When the magnum-api returns an error code 400-600, SessionClient faultstring did not return to users. It should return a detailed error message. This commit was ported from Ironic: https://review.openstack.org/#/c/142021/ Change-Id: I8bc15c0ddab10f0e117fb6acb2f3995929fa65e2 Closes-Bug: #1520363
This commit is contained in:
Hongbin Lu
parent
bab488ff1c
commit
14efefd01f
@ -36,6 +36,20 @@ CHUNKSIZE = 1024 * 64 # 64kB
|
||||
API_VERSION = '/v1'
|
||||
|
||||
|
||||
def _extract_error_json(body):
|
||||
"""Return error_message from the HTTP response body."""
|
||||
error_json = {}
|
||||
try:
|
||||
body_json = json.loads(body)
|
||||
if 'error_message' in body_json:
|
||||
raw_msg = body_json['error_message']
|
||||
error_json = json.loads(raw_msg)
|
||||
except ValueError:
|
||||
return {}
|
||||
|
||||
return error_json
|
||||
|
||||
|
||||
class HTTPClient(object):
|
||||
|
||||
def __init__(self, endpoint, **kwargs):
|
||||
@ -119,18 +133,6 @@ class HTTPClient(object):
|
||||
base_url = _args[2]
|
||||
return '%s/%s' % (base_url, url.lstrip('/'))
|
||||
|
||||
def _extract_error_json(self, body):
|
||||
error_json = {}
|
||||
try:
|
||||
body_json = json.loads(body)
|
||||
if 'error_message' in body_json:
|
||||
raw_msg = body_json['error_message']
|
||||
error_json = json.loads(raw_msg)
|
||||
except ValueError:
|
||||
return {}
|
||||
|
||||
return error_json
|
||||
|
||||
def _http_request(self, url, method, **kwargs):
|
||||
"""Send an http request with the specified characteristics.
|
||||
|
||||
@ -173,7 +175,7 @@ class HTTPClient(object):
|
||||
|
||||
if 400 <= resp.status < 600:
|
||||
LOG.warn("Request returned failure status.")
|
||||
error_json = self._extract_error_json(body_str)
|
||||
error_json = _extract_error_json(body_str)
|
||||
raise exceptions.from_response(
|
||||
resp, error_json.get('faultstring'),
|
||||
error_json.get('debuginfo'), method, url)
|
||||
@ -303,7 +305,10 @@ class SessionClient(adapter.LegacyJsonAdapter):
|
||||
raise_exc=False, **kwargs)
|
||||
|
||||
if 400 <= resp.status_code < 600:
|
||||
raise exceptions.from_response(resp)
|
||||
error_json = _extract_error_json(resp.content)
|
||||
raise exceptions.from_response(
|
||||
resp, error_json.get('faultstring'),
|
||||
error_json.get('debuginfo'), method, url)
|
||||
elif resp.status_code in (301, 302, 305):
|
||||
# Redirected. Reissue the request to the new location.
|
||||
location = resp.headers.get('location')
|
||||
|
||||
@ -56,6 +56,9 @@ def from_response(response, message=None, traceback=None, method=None,
|
||||
response.status_code = response.status
|
||||
response.headers = {
|
||||
'Content-Type': response.getheader('content-type', "")}
|
||||
|
||||
if hasattr(response, 'status_code'):
|
||||
# NOTE(hongbin): This allows SessionClient to handle faultstring.
|
||||
response.json = lambda: {'error': error_body}
|
||||
|
||||
if (response.headers['Content-Type'].startswith('text/') and
|
||||
|
||||
@ -22,6 +22,17 @@ from magnumclient import exceptions as exc
|
||||
from magnumclient.tests import utils
|
||||
|
||||
|
||||
def _get_error_body(faultstring=None, debuginfo=None):
|
||||
error_body = {
|
||||
'faultstring': faultstring,
|
||||
'debuginfo': debuginfo
|
||||
}
|
||||
raw_error_body = json.dumps(error_body)
|
||||
body = {'error_message': raw_error_body}
|
||||
raw_body = json.dumps(body)
|
||||
return raw_body
|
||||
|
||||
|
||||
HTTP_CLASS = six.moves.http_client.HTTPConnection
|
||||
HTTPS_CLASS = http.VerifiedHTTPSConnection
|
||||
DEFAULT_TIMEOUT = 600
|
||||
@ -49,19 +60,8 @@ class HttpClientTest(utils.BaseTestCase):
|
||||
url = client._make_connection_url('v1/resources')
|
||||
self.assertEqual('/v1/resources', url)
|
||||
|
||||
@staticmethod
|
||||
def _get_error_body(faultstring=None, debuginfo=None):
|
||||
error_body = {
|
||||
'faultstring': faultstring,
|
||||
'debuginfo': debuginfo
|
||||
}
|
||||
raw_error_body = json.dumps(error_body)
|
||||
body = {'error_message': raw_error_body}
|
||||
raw_body = json.dumps(body)
|
||||
return raw_body
|
||||
|
||||
def test_server_exception_empty_body(self):
|
||||
error_body = self._get_error_body()
|
||||
error_body = _get_error_body()
|
||||
fake_resp = utils.FakeResponse({'content-type': 'application/json'},
|
||||
six.StringIO(error_body),
|
||||
version=1,
|
||||
@ -77,7 +77,7 @@ class HttpClientTest(utils.BaseTestCase):
|
||||
|
||||
def test_server_exception_msg_only(self):
|
||||
error_msg = 'test error msg'
|
||||
error_body = self._get_error_body(error_msg)
|
||||
error_body = _get_error_body(error_msg)
|
||||
fake_resp = utils.FakeResponse({'content-type': 'application/json'},
|
||||
six.StringIO(error_body),
|
||||
version=1,
|
||||
@ -95,7 +95,7 @@ class HttpClientTest(utils.BaseTestCase):
|
||||
error_msg = 'another test error'
|
||||
error_trace = ("\"Traceback (most recent call last):\\n\\n "
|
||||
"File \\\"/usr/local/lib/python2.7/...")
|
||||
error_body = self._get_error_body(error_msg, error_trace)
|
||||
error_body = _get_error_body(error_msg, error_trace)
|
||||
fake_resp = utils.FakeResponse({'content-type': 'application/json'},
|
||||
six.StringIO(error_body),
|
||||
version=1,
|
||||
@ -218,7 +218,7 @@ class HttpClientTest(utils.BaseTestCase):
|
||||
self.assertEqual(expected, params)
|
||||
|
||||
def test_401_unauthorized_exception(self):
|
||||
error_body = self._get_error_body()
|
||||
error_body = _get_error_body()
|
||||
fake_resp = utils.FakeResponse({'content-type': 'text/plain'},
|
||||
six.StringIO(error_body),
|
||||
version=1,
|
||||
@ -229,3 +229,43 @@ class HttpClientTest(utils.BaseTestCase):
|
||||
|
||||
self.assertRaises(exc.Unauthorized, client.json_request,
|
||||
'GET', '/v1/resources')
|
||||
|
||||
|
||||
class SessionClientTest(utils.BaseTestCase):
|
||||
|
||||
def test_server_exception_msg_and_traceback(self):
|
||||
error_msg = 'another test error'
|
||||
error_trace = ("\"Traceback (most recent call last):\\n\\n "
|
||||
"File \\\"/usr/local/lib/python2.7/...")
|
||||
error_body = _get_error_body(error_msg, error_trace)
|
||||
|
||||
fake_session = utils.FakeSession({'Content-Type': 'application/json'},
|
||||
error_body,
|
||||
500)
|
||||
|
||||
client = http.SessionClient(session=fake_session)
|
||||
|
||||
error = self.assertRaises(exc.InternalServerError,
|
||||
client.json_request,
|
||||
'GET', '/v1/resources')
|
||||
|
||||
self.assertEqual(
|
||||
'%(error)s (HTTP 500)\n%(trace)s' % {'error': error_msg,
|
||||
'trace': error_trace},
|
||||
"%(error)s\n%(details)s" % {'error': str(error),
|
||||
'details': str(error.details)})
|
||||
|
||||
def test_server_exception_empty_body(self):
|
||||
error_body = _get_error_body()
|
||||
|
||||
fake_session = utils.FakeSession({'Content-Type': 'application/json'},
|
||||
error_body,
|
||||
500)
|
||||
|
||||
client = http.SessionClient(session=fake_session)
|
||||
|
||||
error = self.assertRaises(exc.InternalServerError,
|
||||
client.json_request,
|
||||
'GET', '/v1/resources')
|
||||
|
||||
self.assertEqual('Internal Server Error (HTTP 500)', str(error))
|
||||
|
||||
@ -158,3 +158,23 @@ class TestCase(testtools.TestCase):
|
||||
sys.stderr.close()
|
||||
sys.stderr = orig_stderr
|
||||
return (stdout, stderr)
|
||||
|
||||
|
||||
class FakeSessionResponse(object):
|
||||
|
||||
def __init__(self, headers, content=None, status_code=None):
|
||||
self.headers = headers
|
||||
self.content = content
|
||||
self.status_code = status_code
|
||||
|
||||
|
||||
class FakeSession(object):
|
||||
|
||||
def __init__(self, headers, content=None, status_code=None):
|
||||
self.headers = headers
|
||||
self.content = content
|
||||
self.status_code = status_code
|
||||
|
||||
def request(self, url, method, **kwargs):
|
||||
return FakeSessionResponse(self.headers, self.content,
|
||||
self.status_code)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user