From 9b8ab67a780416508b995adafb07e96ea646d6f8 Mon Sep 17 00:00:00 2001 From: Tim Burke 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 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: