Merge "Include response headers in ClientExceptions"

6 files changed, 158 insertions, 100 deletions

diff --git a/swiftclient/client.py b/swiftclient/client.py
index e5d564d..4dbbd49 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 370a8d0..da70379 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 aa95c11..4ba71b4 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, \
@@ -1519,7 +1520,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 59ed17d..36c1593 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 65830f5..f3bee3b 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 1bfa8da..3b043bc 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: