Skip to content

Commit

Permalink
Merge "Add transaction id to errors"
Browse files Browse the repository at this point in the history
  • Loading branch information
Zuul authored and openstack-gerrit committed Mar 1, 2024
2 parents 27171d9 + ac2a60d commit fa9718d
Show file tree
Hide file tree
Showing 7 changed files with 239 additions and 22 deletions.
10 changes: 7 additions & 3 deletions swiftclient/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
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
Expand Down Expand Up @@ -287,7 +290,7 @@ def read(self, length=None):
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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -1926,6 +1929,7 @@ def get_object(self, container, obj, resp_chunk_size=None,
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,
Expand Down
4 changes: 4 additions & 0 deletions swiftclient/multithreading.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ def error(self, msg, *fmt_args):
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

Expand Down
25 changes: 20 additions & 5 deletions swiftclient/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
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
Expand Down Expand Up @@ -68,12 +71,16 @@ def __next__(self):

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)
Expand Down Expand Up @@ -459,7 +466,9 @@ def __init__(self, path, body, headers, checksum=True):
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:
Expand Down Expand Up @@ -1306,9 +1315,15 @@ def _download_object_job(self, conn, container, obj, options):
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()

Expand Down
22 changes: 12 additions & 10 deletions swiftclient/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -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>]
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -670,7 +672,7 @@ def listing(stats_parts_gen=stats_parts_gen):
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>]
Expand Down Expand Up @@ -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>]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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>]
Expand Down Expand Up @@ -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>]
Expand Down Expand Up @@ -1332,7 +1334,7 @@ def _print_compo_cap(name, capabilities):
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
Expand Down
27 changes: 25 additions & 2 deletions test/unit/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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
Expand Down Expand Up @@ -167,8 +168,10 @@ def test_create_with_content_length(self):
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):
Expand Down Expand Up @@ -653,6 +656,7 @@ def test_empty_swifterror_creation(self):
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')
Expand All @@ -665,6 +669,25 @@ def test_swifterror_creation(self):
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):
Expand Down
Loading

0 comments on commit fa9718d

Please sign in to comment.