From 9b8ab67a780416508b995adafb07e96ea646d6f8 Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Mon, 18 Jan 2016 17:05:28 -0800
Subject: [PATCH] Include response headers in ClientExceptions
Now, client applications can get to things like transaction IDs for
failures without needing to turn on all of logging.
While we're at it, add a from_response factory method for
ClientException.
Co-Authored-By: Alexander Corwin <ancorwin@gmail.com>
Change-Id: Ib46d5f8fc7f36f651f5908bb9d900316fdaebce3
---
swiftclient/client.py | 90 +++++++---------------------------
swiftclient/exceptions.py | 15 +++++-
swiftclient/shell.py | 11 ++++-
tests/unit/test_shell.py | 33 ++++++++++++-
tests/unit/test_swiftclient.py | 88 +++++++++++++++++++++++++++------
tests/unit/utils.py | 21 +++++---
6 files changed, 158 insertions(+), 100 deletions(-)
diff --git a/swiftclient/client.py b/swiftclient/client.py
index e5d564d5..4dbbd49c 100644
--- a/swiftclient/client.py
+++ b/swiftclient/client.py
@@ -463,9 +463,7 @@ def get_auth_1_0(url, user, key, snet, **kwargs):
# bad URL would get you that document page and a 200. We error out
# if we don't have a x-storage-url header and if we get a body.
if resp.status < 200 or resp.status >= 300 or (body and not url):
- raise ClientException('Auth GET failed', http_scheme=parsed.scheme,
- http_host=conn.host, http_path=parsed.path,
- http_status=resp.status, http_reason=resp.reason)
+ raise ClientException.from_response(resp, 'Auth GET failed', body)
if snet:
parsed = list(urlparse(url))
# Second item in the list is the netloc
@@ -705,11 +703,7 @@ def get_account(url, token, marker=None, limit=None, prefix=None,
resp_headers = resp_header_dict(resp)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Account GET failed', http_scheme=parsed.scheme,
- http_host=conn.host, http_path=parsed.path,
- http_query=qs, http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Account GET failed', body)
if resp.status == 204:
return resp_headers, []
return resp_headers, parse_api_response(resp_headers, body)
@@ -741,10 +735,7 @@ def head_account(url, token, http_conn=None, service_token=None):
body = resp.read()
http_log((url, method,), {'headers': headers}, resp, body)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Account HEAD failed', http_scheme=parsed.scheme,
- http_host=conn.host, http_path=parsed.path,
- http_status=resp.status, http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Account HEAD failed', body)
resp_headers = resp_header_dict(resp)
return resp_headers
@@ -786,13 +777,7 @@ def post_account(url, token, headers, http_conn=None, response_dict=None,
store_response(resp, response_dict)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Account POST failed',
- http_scheme=parsed.scheme,
- http_host=conn.host,
- http_path=parsed.path,
- http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Account POST failed', body)
resp_headers = {}
for header, value in resp.getheaders():
resp_headers[header.lower()] = value
@@ -877,11 +862,7 @@ def get_container(url, token, container, marker=None, limit=None,
{'headers': headers}, resp, body)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Container GET failed',
- http_scheme=parsed.scheme, http_host=conn.host,
- http_path=cont_path, http_query=qs,
- http_status=resp.status, http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Container GET failed', body)
resp_headers = resp_header_dict(resp)
if resp.status == 204:
return resp_headers, []
@@ -922,11 +903,8 @@ def head_container(url, token, container, http_conn=None, headers=None,
{'headers': req_headers}, resp, body)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Container HEAD failed',
- http_scheme=parsed.scheme, http_host=conn.host,
- http_path=path, http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(
+ resp, 'Container HEAD failed', body)
resp_headers = resp_header_dict(resp)
return resp_headers
@@ -969,11 +947,7 @@ def put_container(url, token, container, headers=None, http_conn=None,
http_log(('%s%s' % (url.replace(parsed.path, ''), path), method,),
{'headers': headers}, resp, body)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Container PUT failed',
- http_scheme=parsed.scheme, http_host=conn.host,
- http_path=path, http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Container PUT failed', body)
def post_container(url, token, container, headers, http_conn=None,
@@ -1012,11 +986,8 @@ def post_container(url, token, container, headers, http_conn=None,
store_response(resp, response_dict)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Container POST failed',
- http_scheme=parsed.scheme, http_host=conn.host,
- http_path=path, http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(
+ resp, 'Container POST failed', body)
def delete_container(url, token, container, http_conn=None,
@@ -1052,11 +1023,8 @@ def delete_container(url, token, container, http_conn=None,
store_response(resp, response_dict)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Container DELETE failed',
- http_scheme=parsed.scheme, http_host=conn.host,
- http_path=path, http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(
+ resp, 'Container DELETE failed', body)
def get_object(url, token, container, name, http_conn=None,
@@ -1109,11 +1077,7 @@ def get_object(url, token, container, name, http_conn=None,
body = resp.read()
http_log(('%s%s' % (url.replace(parsed.path, ''), path), method,),
{'headers': headers}, resp, body)
- raise ClientException('Object GET failed', http_scheme=parsed.scheme,
- http_host=conn.host, http_path=path,
- http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Object GET failed', body)
if resp_chunk_size:
object_body = _ObjectBody(resp, resp_chunk_size)
else:
@@ -1160,10 +1124,7 @@ def head_object(url, token, container, name, http_conn=None,
http_log(('%s%s' % (url.replace(parsed.path, ''), path), method,),
{'headers': headers}, resp, body)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Object HEAD failed', http_scheme=parsed.scheme,
- http_host=conn.host, http_path=path,
- http_status=resp.status, http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Object HEAD failed', body)
resp_headers = resp_header_dict(resp)
return resp_headers
@@ -1275,10 +1236,7 @@ def put_object(url, token=None, container=None, name=None, contents=None,
store_response(resp, response_dict)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Object PUT failed', http_scheme=parsed.scheme,
- http_host=conn.host, http_path=path,
- http_status=resp.status, http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Object PUT failed', body)
etag = resp.getheader('etag', '').strip('"')
return etag
@@ -1318,10 +1276,7 @@ def post_object(url, token, container, name, headers, http_conn=None,
store_response(resp, response_dict)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Object POST failed', http_scheme=parsed.scheme,
- http_host=conn.host, http_path=path,
- http_status=resp.status, http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Object POST failed', body)
def delete_object(url, token=None, container=None, name=None, http_conn=None,
@@ -1375,11 +1330,7 @@ def delete_object(url, token=None, container=None, name=None, http_conn=None,
store_response(resp, response_dict)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Object DELETE failed',
- http_scheme=parsed.scheme, http_host=conn.host,
- http_path=path, http_status=resp.status,
- http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(resp, 'Object DELETE failed', body)
def get_capabilities(http_conn):
@@ -1396,11 +1347,8 @@ def get_capabilities(http_conn):
body = resp.read()
http_log((parsed.geturl(), 'GET',), {'headers': {}}, resp, body)
if resp.status < 200 or resp.status >= 300:
- raise ClientException('Capabilities GET failed',
- http_scheme=parsed.scheme,
- http_host=conn.host, http_path=parsed.path,
- http_status=resp.status, http_reason=resp.reason,
- http_response_content=body)
+ raise ClientException.from_response(
+ resp, 'Capabilities GET failed', body)
resp_headers = resp_header_dict(resp)
return parse_api_response(resp_headers, body)
diff --git a/swiftclient/exceptions.py b/swiftclient/exceptions.py
index 370a8d0f..da70379e 100644
--- a/swiftclient/exceptions.py
+++ b/swiftclient/exceptions.py
@@ -13,12 +13,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+from six.moves import urllib
+
class ClientException(Exception):
def __init__(self, msg, http_scheme='', http_host='', http_port='',
http_path='', http_query='', http_status=None, http_reason='',
- http_device='', http_response_content=''):
+ http_device='', http_response_content='',
+ http_response_headers=None):
super(ClientException, self).__init__(msg)
self.msg = msg
self.http_scheme = http_scheme
@@ -30,6 +33,16 @@ class ClientException(Exception):
self.http_reason = http_reason
self.http_device = http_device
self.http_response_content = http_response_content
+ self.http_response_headers = http_response_headers
+
+ @classmethod
+ def from_response(cls, resp, msg=None, body=None):
+ msg = msg or '%s %s' % (resp.status_code, resp.reason)
+ body = body or resp.content
+ parsed_url = urllib.parse.urlparse(resp.request.url)
+ return cls(msg, parsed_url.scheme, parsed_url.hostname,
+ parsed_url.port, parsed_url.path, parsed_url.query,
+ resp.status_code, resp.reason, '', body, resp.headers)
def __str__(self):
a = self.msg
diff --git a/swiftclient/shell.py b/swiftclient/shell.py
index 15be20ae..68b93445 100755
--- a/swiftclient/shell.py
+++ b/swiftclient/shell.py
@@ -33,7 +33,8 @@ from swiftclient.utils import config_true_value, generate_temp_url, prt_bytes
from swiftclient.multithreading import OutputManager
from swiftclient.exceptions import ClientException
from swiftclient import __version__ as client_version
-from swiftclient.client import logger_settings as client_logger_settings
+from swiftclient.client import logger_settings as client_logger_settings, \
+ parse_header_string
from swiftclient.service import SwiftService, SwiftError, \
SwiftUploadObject, get_conn
from swiftclient.command_helpers import print_account_stats, \
@@ -1475,7 +1476,13 @@ Examples:
parser.usage = globals()['st_%s_help' % args[0]]
try:
globals()['st_%s' % args[0]](parser, argv[1:], output)
- except (ClientException, RequestException, socket.error) as err:
+ except ClientException as err:
+ output.error(str(err))
+ trans_id = (err.http_response_headers or {}).get('X-Trans-Id')
+ if trans_id:
+ output.error("Failed Transaction ID: %s",
+ parse_header_string(trans_id))
+ except (RequestException, socket.error) as err:
output.error(str(err))
if output.get_error_count() > 0:
diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py
index e3ab0bf2..3ea93361 100644
--- a/tests/unit/test_shell.py
+++ b/tests/unit/test_shell.py
@@ -1073,13 +1073,42 @@ class TestShell(unittest.TestCase):
def test_post_account_bad_auth(self, connection):
argv = ["", "post"]
connection.return_value.post_account.side_effect = \
- swiftclient.ClientException('bad auth')
+ swiftclient.ClientException(
+ 'bad auth', http_response_headers={'X-Trans-Id': 'trans_id'})
with CaptureOutput() as output:
with self.assertRaises(SystemExit):
swiftclient.shell.main(argv)
- self.assertEqual(output.err, 'bad auth\n')
+ self.assertEqual(output.err,
+ 'bad auth\nFailed Transaction ID: trans_id\n')
+
+ # do it again with a unicode token
+ connection.return_value.post_account.side_effect = \
+ swiftclient.ClientException(
+ 'bad auth', http_response_headers={
+ 'X-Trans-Id': 'non\u2011utf8'})
+
+ with CaptureOutput() as output:
+ with self.assertRaises(SystemExit):
+ swiftclient.shell.main(argv)
+
+ self.assertEqual(output.err,
+ 'bad auth\n'
+ 'Failed Transaction ID: non\u2011utf8\n')
+
+ # do it again with a wonky token
+ connection.return_value.post_account.side_effect = \
+ swiftclient.ClientException(
+ 'bad auth', http_response_headers={
+ 'X-Trans-Id': b'non\xffutf8'})
+
+ with CaptureOutput() as output:
+ with self.assertRaises(SystemExit):
+ swiftclient.shell.main(argv)
+
+ self.assertEqual(output.err,
+ 'bad auth\nFailed Transaction ID: non%FFutf8\n')
@mock.patch('swiftclient.service.Connection')
def test_post_account_not_found(self, connection):
diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py
index 65830f58..f3bee3bf 100644
--- a/tests/unit/test_swiftclient.py
+++ b/tests/unit/test_swiftclient.py
@@ -51,6 +51,7 @@ class TestClientException(unittest.TestCase):
'status',
'reason',
'device',
+ 'response_content',
)
for value in test_kwargs:
kwargs = {
@@ -59,6 +60,26 @@ class TestClientException(unittest.TestCase):
exc = c.ClientException('test', **kwargs)
self.assertIn(value, str(exc))
+ def test_attrs(self):
+ test_kwargs = (
+ 'scheme',
+ 'host',
+ 'port',
+ 'path',
+ 'query',
+ 'status',
+ 'reason',
+ 'device',
+ 'response_content',
+ 'response_headers',
+ )
+ for value in test_kwargs:
+ key = 'http_%s' % value
+ kwargs = {key: value}
+ exc = c.ClientException('test', **kwargs)
+ self.assertIs(True, hasattr(exc, key))
+ self.assertEqual(getattr(exc, key), value)
+
class MockHttpResponse(object):
def __init__(self, status=0, headers=None, verify=False):
@@ -582,7 +603,9 @@ class TestHeadAccount(MockHttpTest):
def test_server_error(self):
body = 'c' * 65
- c.http_connection = self.fake_http_connection(500, body=body)
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
with self.assertRaises(c.ClientException) as exc_context:
c.head_account('http://www.tests.com', 'asdf')
e = exc_context.exception
@@ -741,7 +764,9 @@ class TestHeadContainer(MockHttpTest):
def test_server_error(self):
body = 'c' * 60
- c.http_connection = self.fake_http_connection(500, body=body)
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
with self.assertRaises(c.ClientException) as exc_context:
c.head_container('http://www.test.com', 'asdf', 'container')
e = exc_context.exception
@@ -750,6 +775,7 @@ class TestHeadContainer(MockHttpTest):
])
self.assertEqual(e.http_status, 500)
self.assertEqual(e.http_response_content, body)
+ self.assertEqual(e.http_response_headers, headers)
class TestPutContainer(MockHttpTest):
@@ -766,10 +792,13 @@ class TestPutContainer(MockHttpTest):
def test_server_error(self):
body = 'c' * 60
- c.http_connection = self.fake_http_connection(500, body=body)
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
with self.assertRaises(c.ClientException) as exc_context:
c.put_container('http://www.test.com', 'token', 'container')
self.assertEqual(exc_context.exception.http_response_content, body)
+ self.assertEqual(exc_context.exception.http_response_headers, headers)
self.assertRequests([
('PUT', '/container', '', {
'x-auth-token': 'token',
@@ -792,9 +821,14 @@ class TestDeleteContainer(MockHttpTest):
class TestGetObject(MockHttpTest):
def test_server_error(self):
- c.http_connection = self.fake_http_connection(500)
- self.assertRaises(c.ClientException, c.get_object,
- 'http://www.test.com', 'asdf', 'asdf', 'asdf')
+ body = 'c' * 60
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
+ with self.assertRaises(c.ClientException) as exc_context:
+ c.get_object('http://www.test.com', 'asdf', 'asdf', 'asdf')
+ self.assertEqual(exc_context.exception.http_response_content, body)
+ self.assertEqual(exc_context.exception.http_response_headers, headers)
def test_query_string(self):
c.http_connection = self.fake_http_connection(200,
@@ -945,9 +979,14 @@ class TestGetObject(MockHttpTest):
class TestHeadObject(MockHttpTest):
def test_server_error(self):
- c.http_connection = self.fake_http_connection(500)
- self.assertRaises(c.ClientException, c.head_object,
- 'http://www.test.com', 'asdf', 'asdf', 'asdf')
+ body = 'c' * 60
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
+ with self.assertRaises(c.ClientException) as exc_context:
+ c.head_object('http://www.test.com', 'asdf', 'asdf', 'asdf')
+ self.assertEqual(exc_context.exception.http_response_content, body)
+ self.assertEqual(exc_context.exception.http_response_headers, headers)
def test_request_headers(self):
c.http_connection = self.fake_http_connection(204)
@@ -1024,12 +1063,15 @@ class TestPutObject(MockHttpTest):
def test_server_error(self):
body = 'c' * 60
- c.http_connection = self.fake_http_connection(500, body=body)
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
args = ('http://www.test.com', 'asdf', 'asdf', 'asdf', 'asdf')
with self.assertRaises(c.ClientException) as exc_context:
c.put_object(*args)
e = exc_context.exception
self.assertEqual(e.http_response_content, body)
+ self.assertEqual(e.http_response_headers, headers)
self.assertEqual(e.http_status, 500)
self.assertRequests([
('PUT', '/asdf/asdf', 'asdf', {
@@ -1249,11 +1291,14 @@ class TestPostObject(MockHttpTest):
def test_server_error(self):
body = 'c' * 60
- c.http_connection = self.fake_http_connection(500, body=body)
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
args = ('http://www.test.com', 'token', 'container', 'obj', {})
with self.assertRaises(c.ClientException) as exc_context:
c.post_object(*args)
self.assertEqual(exc_context.exception.http_response_content, body)
+ self.assertEqual(exc_context.exception.http_response_headers, headers)
self.assertRequests([
('POST', 'http://www.test.com/container/obj', '', {
'x-auth-token': 'token',
@@ -1273,9 +1318,14 @@ class TestDeleteObject(MockHttpTest):
])
def test_server_error(self):
- c.http_connection = self.fake_http_connection(500)
- self.assertRaises(c.ClientException, c.delete_object,
- 'http://www.test.com', 'asdf', 'asdf', 'asdf')
+ body = 'c' * 60
+ headers = {'foo': 'bar'}
+ c.http_connection = self.fake_http_connection(
+ StubResponse(500, body, headers))
+ with self.assertRaises(c.ClientException) as exc_context:
+ c.delete_object('http://www.test.com', 'asdf', 'asdf', 'asdf')
+ self.assertEqual(exc_context.exception.http_response_content, body)
+ self.assertEqual(exc_context.exception.http_response_headers, headers)
def test_query_string(self):
c.http_connection = self.fake_http_connection(200,
@@ -1302,9 +1352,15 @@ class TestGetCapabilities(MockHttpTest):
self.assertTrue(http_conn[1].resp.has_been_read)
def test_server_error(self):
- conn = self.fake_http_connection(500)
+ body = 'c' * 60
+ headers = {'foo': 'bar'}
+ conn = self.fake_http_connection(
+ StubResponse(500, body, headers))
http_conn = conn('http://www.test.com/info')
- self.assertRaises(c.ClientException, c.get_capabilities, http_conn)
+ with self.assertRaises(c.ClientException) as exc_context:
+ c.get_capabilities(http_conn)
+ self.assertEqual(exc_context.exception.http_response_content, body)
+ self.assertEqual(exc_context.exception.http_response_headers, headers)
def test_conn_get_capabilities_with_auth(self):
auth_headers = {
diff --git a/tests/unit/utils.py b/tests/unit/utils.py
index 1bfa8da8..3b043bc7 100644
--- a/tests/unit/utils.py
+++ b/tests/unit/utils.py
@@ -87,17 +87,19 @@ def fake_http_connect(*code_iter, **kwargs):
def __init__(self, status, etag=None, body='', timestamp='1',
headers=None):
- self.status = status
+ self.status_code = self.status = status
self.reason = 'Fake'
+ self.scheme = 'http'
self.host = '1.2.3.4'
self.port = '1234'
self.sent = 0
self.received = 0
self.etag = etag
- self.body = body
+ self.content = self.body = body
self.timestamp = timestamp
self._is_closed = True
self.headers = headers or {}
+ self.request = None
def getresponse(self):
if kwargs.get('raise_exc'):
@@ -223,15 +225,18 @@ class MockHttpTest(unittest.TestCase):
pass
conn = RequestsWrapper()
- def request(method, url, *args, **kwargs):
+ def request(method, path, *args, **kwargs):
try:
conn.resp = self.fake_connect()
except StopIteration:
self.fail('Unexpected %s request for %s' % (
- method, url))
- self.request_log.append((parsed, method, url, args,
+ method, path))
+ self.request_log.append((parsed, method, path, args,
kwargs, conn.resp))
conn.host = conn.resp.host
+ conn.resp.request = RequestsWrapper()
+ conn.resp.request.url = '%s://%s%s' % (
+ conn.resp.scheme, conn.resp.host, path)
conn.resp.has_been_read = False
_orig_read = conn.resp.read
@@ -240,15 +245,15 @@ class MockHttpTest(unittest.TestCase):
return _orig_read(*args, **kwargs)
conn.resp.read = read
if on_request:
- status = on_request(method, url, *args, **kwargs)
+ status = on_request(method, path, *args, **kwargs)
conn.resp.status = status
if auth_token:
headers = args[1]
self.assertEqual(auth_token,
headers.get('X-Auth-Token'))
if query_string:
- self.assertTrue(url.endswith('?' + query_string))
- if url.endswith('invalid_cert') and not insecure:
+ self.assertTrue(path.endswith('?' + query_string))
+ if path.endswith('invalid_cert') and not insecure:
from swiftclient import client as c
raise c.ClientException("invalid_certificate")
if exc: