Add transaction id to errors

Add transaction id to errors to help troubleshoot, including error
when downloading object with truncted/missing segment, stat'ing or
listing non-existent container.

Change response headers returned by client.py Connection APIs
such as get_object() from lowercase dict to CaseInsensitiveDict,
as some error paths rely on this to retrieve the transaction id,
while there are existing code paths using response.headers which
is CaseInsensitiveDict from requests lib.

Change-Id: Ice9cc9fe68684563f18ee527996e5a4292230a96

swiftclient/client.py

@@ -19,10 +19,13 @@ OpenStack Swift client library used internally
 import socket
 import re
 import logging
+from urllib3.exceptions import HTTPError as urllib_http_error
+
 import warnings
 
 from requests.exceptions import RequestException, SSLError
 import http.client as http_client
+from requests.structures import CaseInsensitiveDict
 from urllib.parse import quote, unquote
 from urllib.parse import urljoin, urlparse, urlunparse
 from time import sleep, time
@@ -287,7 +290,7 @@ class _RetryBody(_ObjectBody):
         try:
             buf = self.resp.read(length)
             self.bytes_read += len(buf)
-        except (socket.error, RequestException):
+        except (socket.error, urllib_http_error, RequestException):
             if self.conn.attempts > self.conn.retries:
                 raise
         if (not buf and self.bytes_read < self.expected_length and
@@ -735,9 +738,9 @@ def get_auth(auth_url, user, key, **kwargs):
 
 
 def resp_header_dict(resp):
-    resp_headers = {}
+    resp_headers = CaseInsensitiveDict()
     for header, value in resp.getheaders():
-        header = parse_header_string(header).lower()
+        header = parse_header_string(header)
         resp_headers[header] = parse_header_string(value)
     return resp_headers
 
@@ -1926,6 +1929,7 @@ class Connection:
             is_not_range_request and resp_chunk_size and
             self.attempts <= self.retries and
             rheaders.get('transfer-encoding') is None)
+
         if retry_is_possible:
             body = _RetryBody(body.resp, self, container, obj,
                               resp_chunk_size=resp_chunk_size,

swiftclient/multithreading.py

@@ -89,6 +89,10 @@ class OutputManager:
             msg = msg % fmt_args
         self.error_print_pool.submit(self._print_error, msg)
 
+    def error_with_txn_id(self, swift_err):
+        self.error("{}\nFailed Transaction ID: {}".format(
+            swift_err.value, swift_err.transaction_id or 'unknown'))
+
     def get_error_count(self):
         return self.error_count
 

swiftclient/service.py

@@ -32,6 +32,9 @@ from time import time
 from threading import Thread
 from queue import Queue
 from queue import Empty as QueueEmpty
+from requests.exceptions import RequestException
+from socket import error as socket_error
+from urllib3.exceptions import HTTPError as urllib_http_error
 from urllib.parse import quote
 
 import json
@@ -68,12 +71,16 @@ class ResultsIterator:
 
 class SwiftError(Exception):
     def __init__(self, value, container=None, obj=None,
-                 segment=None, exc=None):
+                 segment=None, exc=None, transaction_id=None):
         self.value = value
         self.container = container
         self.obj = obj
         self.segment = segment
         self.exception = exc
+        if transaction_id is None:
+            self.transaction_id = getattr(exc, 'transaction_id', None)
+        else:
+            self.transaction_id = transaction_id
 
     def __str__(self):
         value = repr(self.value)
@@ -459,7 +466,9 @@ class _SwiftReader:
             try:
                 self._content_length = int(headers.get('content-length'))
             except ValueError:
-                raise SwiftError('content-length header must be an integer')
+                raise SwiftError(
+                    'content-length header must be an integer',
+                    transaction_id=self._txn_id)
 
     def __iter__(self):
         for chunk in self._body:
@@ -1306,9 +1315,15 @@ class SwiftService:
                             else:
                                 pseudodir = True
 
+                try:
                     for chunk in obj_body:
                         if fp is not None:
                             fp.write(chunk)
+                except (socket_error,
+                        urllib_http_error,
+                        RequestException) as err:
+                    raise ClientException(
+                        str(err), http_response_headers=headers)
 
                 finish_time = time()
 

swiftclient/shell.py

@@ -227,7 +227,7 @@ def st_delete(parser, args, output_manager, return_parser=False):
                         output_manager.error('Error Deleting: {0}: {1}'
                                              .format(p, r['error']))
         except SwiftError as err:
-            output_manager.error(err.value)
+            output_manager.error_with_txn_id(err)
 
 
 st_download_options = '''[--all] [--marker <marker>] [--prefix <prefix>]
@@ -484,11 +484,13 @@ def st_download(parser, args, output_manager, return_parser=False):
                                     "Object '%s/%s' not found", container, obj)
                                 continue
                         output_manager.error(
-                            "Error downloading object '%s/%s': %s",
+                            "Error downloading object '%s/%s': %s\n"
-                            container, obj, error)
+                            "Failed Transaction ID: %s",
+                            container, obj, error,
+                            getattr(error, 'transaction_id', 'unknown'))
 
         except SwiftError as e:
-            output_manager.error(e.value)
+            output_manager.error_with_txn_id(e)
         except Exception as e:
             output_manager.error(e)
 
@@ -670,7 +672,7 @@ def st_list(parser, args, output_manager, return_parser=False):
                         prt_bytes(totals['bytes'], human))
 
         except SwiftError as e:
-            output_manager.error(e.value)
+            output_manager.error_with_txn_id(e)
 
 
 st_stat_options = '''[--lh] [--header <header:value>]
@@ -761,7 +763,7 @@ def st_stat(parser, args, output_manager, return_parser=False):
                             st_stat_options, st_stat_help)
 
         except SwiftError as e:
-            output_manager.error(e.value)
+            output_manager.error_with_txn_id(e)
 
 
 st_post_options = '''[--read-acl <acl>] [--write-acl <acl>] [--sync-to <sync-to>]
@@ -867,7 +869,7 @@ def st_post(parser, args, output_manager, return_parser=False):
                 raise(result["error"])
 
         except SwiftError as e:
-            output_manager.error(e.value)
+            output_manager.error_with_txn_id(e)
 
 
 st_copy_options = '''[--destination </container/object>] [--fresh-metadata]
@@ -972,7 +974,7 @@ def st_copy(parser, args, output_manager, return_parser=False):
                 return
 
         except SwiftError as e:
-            output_manager.error(e.value)
+            output_manager.error_with_txn_id(e)
 
 
 st_upload_options = '''[--changed] [--skip-identical] [--segment-size <size>]
@@ -1270,7 +1272,7 @@ def st_upload(parser, args, output_manager, return_parser=False):
                                 "to chunk the object")
 
         except SwiftError as e:
-            output_manager.error(e.value)
+            output_manager.error_with_txn_id(e)
 
 
 st_capabilities_options = '''[--json] [<proxy_url>]
@@ -1332,7 +1334,7 @@ def st_capabilities(parser, args, output_manager, return_parser=False):
                 del capabilities['swift']
                 _print_compo_cap('Additional middleware', capabilities)
         except SwiftError as e:
-            output_manager.error(e.value)
+            output_manager.error_with_txn_id(e)
 
 
 st_info = st_capabilities

test/unit/test_service.py

@@ -27,6 +27,7 @@ from unittest import mock
 from concurrent.futures import Future
 from hashlib import md5
 from queue import Queue, Empty as QueueEmptyError
+from requests.structures import CaseInsensitiveDict
 from time import sleep
 
 import swiftclient
@@ -167,8 +168,10 @@ class TestSwiftReader(unittest.TestCase):
         self.assertIsNone(sr._actual_md5)
 
         # Check Contentlength raises error if it isn't an integer
-        self.assertRaises(SwiftError, self.sr, 'path', 'body',
+        with self.assertRaises(SwiftError) as cm:
-                          {'content-length': 'notanint'})
+            self.sr('path', 'body', {'content-length': 'notanint'})
+        self.assertEqual("'content-length header must be an integer'",
+                         str(cm.exception))
 
     def test_iterator_usage(self):
         def _consume(sr):
@@ -653,6 +656,7 @@ class TestSwiftError(unittest.TestCase):
         self.assertIsNone(se.exception)
 
         self.assertEqual(str(se), '5')
+        self.assertNotIn(str(se), 'Transaction ID')
 
     def test_swifterror_creation(self):
         test_exc = Exception('test exc')
@@ -665,6 +669,25 @@ class TestSwiftError(unittest.TestCase):
         self.assertEqual(se.exception, test_exc)
 
         self.assertEqual(str(se), '5 container:con object:obj segment:seg')
+        self.assertNotIn(str(se), 'Transaction ID')
+
+    def test_swifterror_clientexception_creation(self):
+        test_exc = ClientException(
+            Exception('test exc'),
+            http_response_headers=CaseInsensitiveDict({
+                'x-trans-id': 'someTransId'})
+        )
+        se = SwiftError(5, 'con', 'obj', 'seg', test_exc)
+
+        self.assertEqual(se.value, 5)
+        self.assertEqual(se.container, 'con')
+        self.assertEqual(se.obj, 'obj')
+        self.assertEqual(se.segment, 'seg')
+        self.assertEqual(se.exception, test_exc)
+
+        self.assertEqual('someTransId', se.transaction_id)
+        self.assertNotIn('someTransId', str(se))
+        self.assertIn('5 container:con object:obj segment:seg', str(se))
 
 
 class TestServiceUtils(unittest.TestCase):

test/unit/test_shell.py

@@ -15,17 +15,21 @@
 
 import io
 import contextlib
+import socket
 from genericpath import getmtime
 import getpass
 import hashlib
 import json
 import logging
 import os
+from requests.structures import CaseInsensitiveDict
 import tempfile
 import unittest
 from unittest import mock
 import textwrap
 from time import localtime, mktime, strftime, strptime
+from requests.exceptions import RequestException
+from urllib3.exceptions import HTTPError
 
 import swiftclient
 from swiftclient.service import SwiftError
@@ -207,6 +211,28 @@ class TestShell(unittest.TestCase):
                              '  Sync To: other\n'
                              ' Sync Key: secret\n')
 
+    @mock.patch('swiftclient.service.Connection')
+    def test_stat_container_not_found(self, connection):
+        connection.return_value.head_container.side_effect = \
+            swiftclient.ClientException(
+                'test',
+                http_status=404,
+                http_response_headers=CaseInsensitiveDict({
+                    'x-trans-id': 'someTransId'})
+            )
+        argv = ["", "stat", "container"]
+
+        with CaptureOutput() as output:
+            with self.assertRaises(SystemExit):
+                swiftclient.shell.main(argv)
+                connection.return_value.head_container.assert_called_with(
+                    'container', headers={}, resp_chunk_size=65536,
+                    response_dict={})
+
+            self.assertIn('Container \'container\' not found\n'
+                          'Failed Transaction ID: someTransId',
+                          output.err)
+
     @mock.patch('swiftclient.service.Connection')
     def test_stat_container_with_headers(self, connection):
         return_headers = {
@@ -285,6 +311,27 @@ class TestShell(unittest.TestCase):
                              '          ETag: md5\n'
                              '      Manifest: manifest\n')
 
+    @mock.patch('swiftclient.service.Connection')
+    def test_stat_object_not_found(self, connection):
+        connection.return_value.head_object.side_effect = \
+            swiftclient.ClientException(
+                'test', http_status=404,
+                http_response_headers=CaseInsensitiveDict({
+                    'x-trans-id': 'someTransId'})
+            )
+        argv = ["", "stat", "container", "object"]
+
+        with CaptureOutput() as output:
+            with self.assertRaises(SystemExit):
+                swiftclient.shell.main(argv)
+                connection.return_value.head_object.assert_called_with(
+                    'container', 'object', headers={}, resp_chunk_size=65536,
+                    response_dict={})
+
+            self.assertIn('test: 404\n'
+                          'Failed Transaction ID: someTransId',
+                          output.err)
+
     @mock.patch('swiftclient.service.Connection')
     def test_stat_object_with_headers(self, connection):
         return_headers = {
@@ -707,6 +754,117 @@ class TestShell(unittest.TestCase):
             swiftclient.shell.main(argv)
             self.assertEqual('objcontent', output.out)
 
+    def _do_test_download_clientexception(self, exc):
+        retry_calls = []
+
+        def _fake_retry(conn, *args, **kwargs):
+            retry_calls.append((args, kwargs))
+            conn.attempts += 1
+
+            body = mock.MagicMock()
+            body.resp.read.side_effect = RequestException('test_exc')
+            return (CaseInsensitiveDict({
+                'content-type': 'text/plain',
+                'etag': '2cbbfe139a744d6abbe695e17f3c1991',
+                'x-trans-id': 'someTransId'}),
+                body)
+
+        argv = ["", "download", "container", "object", "--retries", "1"]
+        with CaptureOutput() as output:
+            with mock.patch(BUILTIN_OPEN) as mock_open:
+                with mock.patch("swiftclient.service.Connection._retry",
+                                _fake_retry):
+                    with self.assertRaises(SystemExit):
+                        swiftclient.shell.main(argv)
+                    mock_open.assert_called_with('object', 'wb', 65536)
+            self.assertEqual([
+                ((None, swiftclient.client.get_object, 'container', 'object'),
+                 {'headers': {},
+                  'query_string': None,
+                  'resp_chunk_size': 65536,
+                  'response_dict': {}}),
+                ((None, swiftclient.client.get_object, 'container', 'object'),
+                 {'attempts': 1,
+                  'headers': {'If-Match': mock.ANY, 'Range': 'bytes=0-'},
+                  'query_string': None,
+                  'resp_chunk_size': 65536,
+                  'response_dict': {}})],
+                retry_calls)
+            self.assertIn('Error downloading object \'container/object\': '
+                          'test_exc',
+                          str(output.err))
+            self.assertIn('someTransId', str(output.err))
+
+    def test_download_request_exception(self):
+        self._do_test_download_clientexception(RequestException('text_exc'))
+
+    def test_download_socket_error(self):
+        self._do_test_download_clientexception(socket.error())
+
+    def test_download_http_error(self):
+        self._do_test_download_clientexception(HTTPError)
+
+    def test_download_request_exception_retries_0(self):
+        retry_calls = []
+
+        def _fake_retry(conn, *args, **kwargs):
+            retry_calls.append((args, kwargs))
+            conn.attempts += 1
+
+            body = mock.MagicMock()
+            body.__iter__.side_effect = RequestException('test_exc')
+            return (CaseInsensitiveDict({
+                'content-type': 'text/plain',
+                'etag': '2cbbfe139a744d6abbe695e17f3c1991',
+                'x-trans-id': 'someTransId'}),
+                body)
+
+        argv = ["", "download", "container", "object", "--retries", "0"]
+        with CaptureOutput() as output:
+            with mock.patch(BUILTIN_OPEN) as mock_open:
+                with mock.patch("swiftclient.service.Connection._retry",
+                                _fake_retry):
+                    with self.assertRaises(SystemExit):
+                        swiftclient.shell.main(argv)
+                    mock_open.assert_called_with('object', 'wb', 65536)
+            self.assertEqual([
+                ((None, swiftclient.client.get_object, 'container', 'object'),
+                 {'headers': {},
+                  'query_string': None,
+                  'resp_chunk_size': 65536,
+                  'response_dict': {}}), ],
+                retry_calls)
+            self.assertIn('Error downloading object \'container/object\': '
+                          'test_exc',
+                          str(output.err))
+            self.assertIn('someTransId', str(output.err))
+
+    @mock.patch('swiftclient.service.Connection')
+    def test_download_bad_content_length(self, connection):
+        objcontent = io.BytesIO(b'objcontent')
+        connection.return_value.get_object.side_effect = [
+            (CaseInsensitiveDict({
+                'content-type': 'text/plain',
+                'content-length': 'BAD',
+                'etag': '2cbbfe139a744d6abbe695e17f3c1991',
+                'x-trans-id': 'someTransId'}),
+             objcontent)
+        ]
+        with CaptureOutput() as output:
+            with self.assertRaises(SystemExit):
+                with mock.patch(BUILTIN_OPEN) as mock_open:
+                    argv = ["", "download", "container", "object"]
+                    swiftclient.shell.main(argv)
+                connection.return_value.get_object.assert_called_with(
+                    'container', 'object', headers={}, resp_chunk_size=65536,
+                    response_dict={})
+                mock_open.assert_called_with('object', 'wb', 65536)
+
+            self.assertIn("Error downloading object \'container/object\': "
+                          "'content-length header must be an integer'"
+                          "\nFailed Transaction ID: someTransId",
+                          str(output.err))
+
     @mock.patch('swiftclient.service.shuffle')
     @mock.patch('swiftclient.service.Connection')
     def test_download_shuffle(self, connection, mock_shuffle):
@@ -1901,7 +2059,9 @@ class TestShell(unittest.TestCase):
             with self.assertRaises(SystemExit):
                 swiftclient.shell.main(argv)
 
-            self.assertEqual(output.err, 'Account not found\n')
+            self.assertEqual(
+                output.err,
+                'Account not found\nFailed Transaction ID: unknown\n')
 
     @mock.patch('swiftclient.service.Connection')
     def test_post_container(self, connection):
@@ -2093,7 +2253,8 @@ class TestShell(unittest.TestCase):
             self.assertEqual(
                 output.err,
                 'Combination of multiple objects and destination '
-                'including object is invalid\n')
+                'including object is invalid\n'
+                'Failed Transaction ID: unknown\n')
 
     @mock.patch('swiftclient.service.Connection')
     def test_copy_object_bad_auth(self, connection):

test/unit/test_swiftclient.py

@@ -1118,6 +1118,14 @@ class TestGetObject(MockHttpTest):
         self.assertEqual('%ff', headers.get('x-non-utf-8-header', ''))
         self.assertEqual('%FF', headers.get('x-binary-header', ''))
 
+        self.assertEqual('t\xe9st', headers.get('X-Utf-8-Header', ''))
+        self.assertEqual('%ff', headers.get('X-Non-Utf-8-Header', ''))
+        self.assertEqual('%FF', headers.get('X-Binary-Header', ''))
+
+        self.assertEqual('t\xe9st', headers.get('X-UTF-8-HEADER', ''))
+        self.assertEqual('%ff', headers.get('X-NON-UTF-8-HEADER', ''))
+        self.assertEqual('%FF', headers.get('X-BINARY-HEADER', ''))
+
     def test_chunk_size_read_method(self):
         conn = c.Connection('http://auth.url/', 'some_user', 'some_key')
         with mock.patch('swiftclient.client.get_auth_1_0') as mock_get_auth: