diff --git a/swiftclient/client.py b/swiftclient/client.py index 06350901..074e3062 100644 --- a/swiftclient/client.py +++ b/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, diff --git a/swiftclient/multithreading.py b/swiftclient/multithreading.py index eaccec68..9659997c 100644 --- a/swiftclient/multithreading.py +++ b/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 diff --git a/swiftclient/service.py b/swiftclient/service.py index 3d1b4764..d0e93075 100644 --- a/swiftclient/service.py +++ b/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() diff --git a/swiftclient/shell.py b/swiftclient/shell.py index faa20001..1dc8d2f4 100755 --- a/swiftclient/shell.py +++ b/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 ] [--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 ] @@ -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 ] [--write-acl ] [--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 ] [--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 ] @@ -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] [] @@ -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 diff --git a/test/unit/test_service.py b/test/unit/test_service.py index 6bc50ca7..9325d646 100644 --- a/test/unit/test_service.py +++ b/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): diff --git a/test/unit/test_shell.py b/test/unit/test_shell.py index 9d1d802d..2caf05a9 100644 --- a/test/unit/test_shell.py +++ b/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): diff --git a/test/unit/test_swiftclient.py b/test/unit/test_swiftclient.py index 3506a11d..4c79fe2d 100644 --- a/test/unit/test_swiftclient.py +++ b/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: