Merge "Add transaction id to errors"

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
 
-                for chunk in obj_body:
-                    if fp is not None:
-                        fp.write(chunk)
+                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",
-                            container, obj, error)
+                            "Error downloading object '%s/%s': %s\n"
+                            "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',
-                          {'content-length': 'notanint'})
+        with self.assertRaises(SwiftError) as cm:
+            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
@@ -235,6 +239,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 = {
@@ -313,6 +339,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 = {
@@ -735,6 +782,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):
@@ -1930,7 +2088,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):
@@ -2122,7 +2282,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: